On Wed, Jul 03, 2024 at 08:48:59AM -0700, Elijah Newren wrote:
> > > 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?
>
> Oops! That's embarrassing. Thanks for catching; I'll send in a patch
> on top since this is already in next.
Yeah, I only caught it because Coverity flagged it when it hit 'next'.
It is quite subtle. :)
-Peff
