From: Pierre Habouzit Date: Tue, 09 Jun 2009 17:23:02 GMT Subject: Re: [PATCH] show-branch: fix segfault when showbranch.default exists Message-ID: <20090609172302.GH9993@laphroaig.corp> In-Reply-To: <7viqj5nzgz.fsf@alter.siamese.dyndns.org> On Tue, Jun 09, 2009 at 09:28:28AM -0700, Junio C Hamano wrote: > Pierre Habouzit 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