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

[Pierre Habouzit <madcoder@madism.org>]

On Tue, Jun 09, 2009 at 09:28:28AM -0700, Junio C Hamano wrote:
> Pierre Habouzit <madcoder@madism.org> writes:
> 
> > 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).

it does because it supposes that the command is there, and that when I
ported the old parsing stuff it's what it was doing.

FWIW I believe it's a misfeature.

-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org