Hi Junio, On Tue, 13 Jun 2017, Junio C Hamano wrote: > Jeff King <peff@xxxxxxxx> 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