Re: [PATCH] show-branch: fix segfault when showbranch.default exists

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 9, 2009, 16:28 UTC
Message-ID: <7viqj5nzgz.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20090609080612.GG9993@laphroaig.corp>

Pierre Habouzit <madcoder@madism.org> writes:

Show 13 quoted lines

> On Tue, Jun 09, 2009 at 12:17:28AM -0700, Stephen Boyd wrote:
>> Junio C Hamano wrote:
>> > I am not sure if this is a bug in parse_options(), or a bug in the caller,
>> > and tonight I do not have enough concentration to figure out which.  In
>> > any case, this patch works the issue around.
>> 
>> I am low on concentration tonight as well, but this looks right to me.
>> Parse options is expecting the regular old argv and argc. I overlooked
>> this code path during the conversion (though I remember figuring out
>> what this path was doing). Faking the argv and argc a little more
>> accurately, like you do, should work fine.
>
> yes, that's it.

Wait a minute, please.

Why is parse_options() allowed to clobber argv[0] in parse_options_end() in the first place?

I think the memmove() is there to allow the caller to find the remaining arguments after the library parsed out the options in argv[], but wouldn't the caller be expecting to inspect argv[] starting from position 1?

In other words, anything moved to the position of original argv[0] would be lost, and the problem I stumbled upon was exactly that (it triggered because the location of argv[0] was invalid and parse_options_end() wrote into it).