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

2017-02-16 Thread Jeff King
On Thu, Feb 16, 2017 at 01:40:00PM +0100, Christian Couder wrote: > > I started to add some tests, but I had second thoughts. It _is_ nice > > to show off the fix, but as far as regressions go, this specific case is > > unlikely to come up again. What would be more valuable, I think, is a > >

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

2017-02-16 Thread Christian Couder
On Wed, Feb 15, 2017 at 10:40 PM, Jeff King wrote: > On Tue, Feb 14, 2017 at 10:29:46PM +0100, Christian Couder wrote: > >> > 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

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

2017-02-15 Thread Jeff King
On Wed, Feb 15, 2017 at 01:50:07PM -0800, Junio C Hamano wrote: > Jeff King writes: > > > I see the patches are marked for 'next' in the latest What's Cooking. > > If it is not too late in today's integration cycle, here is a re-roll of > > patch 3 that squashes in Pranit's

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

2017-02-15 Thread Junio C Hamano
Jeff King writes: > I see the patches are marked for 'next' in the latest What's Cooking. > If it is not too late in today's integration cycle, here is a re-roll of > patch 3 that squashes in Pranit's suggestion (if it is too late, then > Pranit, you may want to re-send it as a

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

2017-02-15 Thread Jeff King
On Tue, Feb 14, 2017 at 10:29:46PM +0100, Christian Couder wrote: > > 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

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

2017-02-14 Thread Christian Couder
On Tue, Feb 14, 2017 at 8:55 PM, Jeff King wrote: > On Tue, Feb 14, 2017 at 11:35:48AM -0800, Junio C Hamano wrote: > >> Jeff King writes: >> >> > This fixes the problem, but I think we can simplify it quite a bit by >> > using resolve_refdup(). Here's the patch

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

2017-02-14 Thread Jeff King
On Tue, Feb 14, 2017 at 11:35:48AM -0800, Junio C Hamano wrote: > Jeff King 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

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

2017-02-14 Thread Junio C Hamano
Jeff King 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]:

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

2017-02-14 Thread Jeff King
On Tue, Feb 14, 2017 at 04:48:16PM +0100, Christian Couder wrote: > @@ -791,11 +791,11 @@ int cmd_show_branch(int ac, const char **av, const char > *prefix) > head_oid.hash, NULL); > if (head_p) { > head_len = strlen(head_p); > -

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

2017-02-14 Thread Christian Couder
This is a minimum fix for a crash with a long ref name. Note that if the refname is too long (for example if you use 100 instead of 50 in the test script) you could get an error like: fatal: cannot lock ref 'refs/heads/1_2_3_4_ ... _98_99_100_': Unable to create '... /.git/refs/heads/1_2_3_4_