Re: [PATCH v4 1/2] pack-bitmap: fix memory leak if load_bitmap() failed

From: Taylor Blau <me@ttaylorr.com>
Date: May 29, 2025, 19:57 UTC
Message-ID: <aDi8OD08I6+6BLja@nand.local>
In-Reply-To: <xmqqjz5zmnxy.fsf@gitster.g>

On Thu, May 29, 2025 at 08:33:29AM -0700, Junio C Hamano wrote:

Show 16 quoted lines

> "Taylor Blau via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > From: Taylor Blau <me@ttaylorr.com>
> >
> > After going through the "failed" label, load_bitmap() will return -1,
> > and its caller (either prepare_bitmap_walk() or prepare_bitmap_git())
> > will then call free_bitmap_index().
> > ...
> > The solution is to remove the error handling code in load_bitmap(), because
> > its caller will always call free_bitmap_index() in case of an error.
> >
> > Signed-off-by: Taylor Blau <me@ttaylorr.com>
> > ---
>
> As this is Lidong relaying <aCOFqYdnPp1Lne4Y@nand.local> that Taylor
> sent to the list, shouldn't Lidong's sign-off be after Taylor's?

I've always assumed the answer here was "yes", but I don't know that our documentation suggests the same.

In c11c3b5681 (Documentation/SubmittingPatches: What's Acked-by and Tested-by?, 2008-02-03) you added:

    Notice that you can place your own Signed-off-by: line when
    forwarding somebody else's patch [...]. Indeed you are encouraged
    to do so.  [...]

and that text survives into the current version of SubmittingPatches. So I think that while our documentation encourages people to add their own S-o-b to others' patches sent on their behalf, it doesn't explicitly require it.

Thanks, Taylor