Re: [PATCH] show-branch: fix segfault when showbranch.default exists
- From
- Pierre Habouzit <madcoder@madism.org>
- Date
- Jun 9, 2009, 17:23 UTC
- 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:
Show 29 quoted lines
> 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