Re: [PATCH v2 0/2] fix read past end of array in alternates files

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 20, 2017, 02:44 UTC
Message-ID: <xmqqmv5qozq7.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20170919194044.3prgaxd4sqandy75@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 9 quoted lines

> On Mon, Sep 18, 2017 at 11:51:00AM -0400, Jeff King wrote:
>
>> This series fixes a regression in v2.11.1 where we might read past the
>> end of an mmap'd buffer. It was introduced in cf3c635210, but I didn't
>> base the patch on there, for a few reasons:
>
> Here's a v2 that fixes a minor leak in the first patch (I carefully
> remembered to free() the path buffer on the error path, but forgot to do
> it in the normal one. Oops).

Thanks.

> I also tried to address Jonathan's "should this be in the commit
> message" comment. The information above _is_ in there, but maybe putting
> it at the top as a sort of tl;dr makes it easier to find?

Probably.

Show 7 quoted lines

> The second patch is unchanged.
>
> Junio, I see you ended up back-porting it to v2.11. Would you prefer me
> to have done it that way in the first place? I was trying to reduce your
> work by basing it on "maint" (figuring that we wouldn't bother making a
> v2.11.x release anyway, and that skips you having to apply the second
> patch separately after the merge).

Upon seeing that this dated back to 2.11, because I am lazy and do not assess how much the fix needs to go to older tracks when I am queuing (remember: my attention span during patch queueing is measured in minutes, as people send changes to different areas), I tend to first try to see what's the oldest maintenance track we can practically apply the patch to. It turned out that the conflict resolution to apply on maint-2.11 wasn't that painful, so I took the lazy route all the way---the real fix on the oldest, even though I do not know (because I refused to think and decide due to laziness) if a next v2.11.x release is necessary, followed by a nice-to-have warning that uses newer features on the maintenance track. That way, when we decide that the fix won't be a big deal to require a new v2.11.x, but it is nice to have in v2.13.x, we could merge the first one, without having to cherry-pick.

All of the above is part of how the daily maintainer workflow goes, and there is no strong preference on my side, if the original is on the theoretically oldest (i.e. maint-2.11) or on the oldest practical (i.e. maint), as long as the conflicts are not too painful.