Re: [PATCH v2 7/8] alias_lookup(): optionally return top-level directory

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

Hi Junio,

On Tue, 13 Jun 2017, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > On Tue, Jun 13, 2017 at 01:42:02PM +0200, Johannes Schindelin wrote:
> >
> >> > As you probably guessed, I had tried that first and then figured that if
> >> > I needed to keep the config_key_is_valid() test anyway, I could just as
> >> > well keep the strbuf around for later use.
> >> > 
> >> > Will change the code,
> >> 
> >> Alas, I won't change the code after all.
> >> 
> >> It is really tempting to avoid the extra strbuf, but then the error
> >> message would change from
> >> 
> >> 	error: missing value for 'alias.br'
> >> 
> >> to
> >> 
> >> 	error: missing value for 'br'
> >> 
> >> which is of course no good at all.
> >> 
> >> And since I already have to keep that strbuf, I'll simply keep the
> >> config_key_is_valid() guard, too (because why not).
> >
> > Oof, yeah, that is definitely worse. I'm fine with keeping both parts.
> 
> When you replace Dscho's "compare 'var' with 'alias.br' that is in
> strbuf naively with the "skip-prefix and compare with br" without
> changing anything else, i.e.
> 
>     if (skip_prefix(var, "alias.", &key) && !strcmp(key, data->key))
> 	return git_config_string((const char **)&data->v, key, value);
> 
> it would cause the "br" to be fed to git_config_string() and result
> in problem reported for "br", not "alias.br".  
> 
> But this can be trivially fixed by passing "var" instead of "key" to
> git_config_string(), no?  Am I mistaken?

Oh my. You are right! The `git_config_string()` function uses the `key`
parameter only for reporting, not for anything else.

So all I had to do was to introduce another variable so that the `key`
pointer would not advance beyond the "alias.".

Will fix,
Dscho