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

Elijah Newren
2024-07-03T15:49:12Z

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.