Re: [PATCH] show-branch: fix crash with long ref name

From: Jeff King <peff@peff.net>
Date: Feb 14, 2017, 19:55 UTC
Message-ID: <20170214195513.7zae6x22advkrms6@sigill.intra.peff.net>
In-Reply-To: <xmqqlgt88t0r.fsf@gitster.mtv.corp.google.com>

On Tue, Feb 14, 2017 at 11:35:48AM -0800, Junio C Hamano wrote:

Show 18 quoted lines

> Jeff King <peff@peff.net> writes:
> 
> > This fixes the problem, but I think we can simplify it quite a bit by
> > using resolve_refdup(). Here's the patch series I ended up with:
> >
> >   [1/3]: show-branch: drop head_len variable
> >   [2/3]: show-branch: store resolved head in heap buffer
> >   [3/3]: show-branch: use skip_prefix to drop magic numbers
> >
> >  builtin/show-branch.c | 39 ++++++++++++---------------------------
> >  1 file changed, 12 insertions(+), 27 deletions(-)
> 
> Yes, the whole thing is my fault ;-) and I agree with what these
> patches do.
> 
> The second one lacks free(head) but I think that is OK; it is
> something we allocate in cmd_*() and use pretty much thruout the
> rest of the program.

Yes, I actually tested the whole thing under ASAN (which was necessary to notice the problem), which complained about the leak. I don't mind adding a free(head), but there are a bunch of similar "leaks" in that function, so I didn't bother.

I notice Christian's patch added a few tests. I don't know if we'd want to squash them in (I didn't mean to override his patch at all; I was about to send mine out when I noticed his, and I wondered if we wanted to combine the two efforts).

-Peff