git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v2 7/7] merge-ort: convert more error() cases to path_msg()

From
Elijah Newren <newren@gmail.com>
Date
Jul 3, 2024, 15:48 UTC
Message-ID
<CABPp-BEkh4K1156FWUdB9wSjSQcdeUF0dcTJ3CS_b8yvz5Vffg@mail.gmail.com>
In-Reply-To
<20240702213326.GA591768@coredump.intra.peff.net>
On Tue, Jul 2, 2024 at 2:33 PM Jeff King <peff@peff.net> wrote:
Show 35 quoted lines
>
> On Wed, Jun 19, 2024 at 03:00:19AM +0000, Elijah Newren via GitGitGadget wrote:
>
> > +static int read_oid_strbuf(struct merge_options *opt,
> > +                        const struct object_id *oid,
> > +                        struct strbuf *dst,
> > +                        const char *path)
> >  {
> >       void *buf;
> >       enum object_type type;
> >       unsigned long size;
> >       buf = repo_read_object_file(the_repository, oid, &type, &size);
> > -     if (!buf)
> > -             return error(_("cannot read object %s"), oid_to_hex(oid));
> > +     if (!buf) {
> > +             path_msg(opt, ERROR_OBJECT_READ_FAILED, 0,
> > +                      path, NULL, NULL, NULL,
> > +                      _("error: cannot read object %s"), oid_to_hex(oid));
> > +             return -1;
> > +     }
> >       if (type != OBJ_BLOB) {
> >               free(buf);
> > -             return error(_("object %s is not a blob"), oid_to_hex(oid));
> > +             path_msg(opt, ERROR_OBJECT_NOT_A_BLOB, 0,
> > +                      path, NULL, NULL, NULL,
> > +                      _("error: object %s is not a blob"), oid_to_hex(oid));
> >       }
> >       strbuf_attach(dst, buf, size, size + 1);
> >       return 0;
>
> This loses the early return in the "type != OBJ_BLOB" code path. So we
> free(buf), but then continue on to the strbuf_attach() call on the
> dangling pointer. Should it "return -1" like the earlier conditional?
>
> -Peff

Oops! That's embarrassing. Thanks for catching; I'll send in a patch on top since this is already in next.

Previous: Jeff KingNext: Junio C Hamano
Message 30 of 34 in “Fix and improve some error codepaths in merge-ort”
  1. 0/7 Fix and improve some error codepaths in merge-ortElijah Newren via GitGitGadget, Jun 13, 2024
  2. 1/7 merge-ort: extract handling of priv member into reusable functionElijah Newren via GitGitGadget, Jun 13, 2024
  3. Junio C HamanoJun 13, 2024
  4. Taylor BlauJun 13, 2024
  5. 2/7 merge-ort: maintain expected invariant for priv memberElijah Newren via GitGitGadget, Jun 13, 2024
  6. Taylor BlauJun 13, 2024
  7. Elijah NewrenJun 19, 2024
  8. 3/7 merge-ort: fix type of local 'clean' var in handle_content_merge()Elijah Newren via GitGitGadget, Jun 13, 2024
  9. 4/7 merge-ort: clearer propagation of failure-to-function from merge_submoduleElijah Newren via GitGitGadget, Jun 13, 2024
  10. 5/7 merge-ort: loosen commented requirementsElijah Newren via GitGitGadget, Jun 13, 2024
  11. 6/7 merge-ort: upon merge abort, only show messages causing the abortElijah Newren via GitGitGadget, Jun 13, 2024
  12. Eric SunshineJun 14, 2024
  13. Elijah NewrenJun 19, 2024
  14. 7/7 merge-ort: convert more error() cases to path_msg()Elijah Newren via GitGitGadget, Jun 13, 2024
  15. Taylor BlauJun 13, 2024
  16. 0/7 Fix and improve some error codepaths in merge-ortElijah Newren via GitGitGadget, Jun 19, 2024
  17. 1/7 merge-ort: extract handling of priv member into reusable functionElijah Newren via GitGitGadget, Jun 19, 2024
  18. 2/7 merge-ort: maintain expected invariant for priv memberElijah Newren via GitGitGadget, Jun 19, 2024
  19. Derrick StoleeJun 28, 2024
  20. 3/7 merge-ort: fix type of local 'clean' var in handle_content_merge()Elijah Newren via GitGitGadget, Jun 19, 2024
  21. Derrick StoleeJun 28, 2024
  22. 4/7 merge-ort: clearer propagation of failure-to-function from merge_submoduleElijah Newren via GitGitGadget, Jun 19, 2024
  23. Derrick StoleeJun 28, 2024
  24. Elijah NewrenJun 28, 2024
  25. Derrick StoleeJun 28, 2024
  26. 5/7 merge-ort: loosen commented requirementsElijah Newren via GitGitGadget, Jun 19, 2024
  27. 6/7 merge-ort: upon merge abort, only show messages causing the abortElijah Newren via GitGitGadget, Jun 19, 2024
  28. 7/7 merge-ort: convert more error() cases to path_msg()Elijah Newren via GitGitGadget, Jun 19, 2024
  29. Jeff KingJul 2, 2024
  30. Elijah NewrenJul 3, 2024
  31. Junio C HamanoJul 3, 2024
  32. Jeff KingJul 6, 2024
  33. Derrick StoleeJun 28, 2024
  34. Junio C HamanoJun 28, 2024

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.