From: Elijah Newren <newren@gmail.com>
Date: Wed, 03 Jul 2024 15:48:59 GMT
Subject: Re: [PATCH v2 7/7] merge-ort: convert more error() cases to path_msg()
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:
>
> 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.