Re: [PATCH] refs: parse_hide_refs_config to use parse_config_key

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 24, 2017, 21:18 UTC
Message-ID: <xmqq4lzj1e4n.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20170224210643.max6z2ykm3gbg7lw@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 25 quoted lines

> On Fri, Feb 24, 2017 at 03:39:40PM -0500, Jeff King wrote:
>
>> This will start parsing "receive.foobar.hiderefs", which we don't want.
>> I think you need:
>> 
>>   !parse_config_key(var, section, &subsection, &subsection_len, &key) &&
>>   !subsection &&
>>   !strcmp(key, "hiderefs")
>> 
>> Perhaps passing NULL for the subsection variable should cause
>> parse_config_key to return failure when there is a non-empty subsection.
>> 
>> -Peff
>> 
>> PS Outside of parse_config_key, this code would be nicer if it used
>>    skip_prefix() instead of starts_with(). Since it's going away, I
>>    don't think it matters, but I note that parse_config_key could
>>    probably benefit from the same.
>
> While I'm thinking about it, here are patches to do that. The third one
> I'd probably squash into yours (after ordering it to the end).
>
>   [1/3]: parse_config_key: use skip_prefix instead of starts_with
>   [2/3]: parse_config_key: allow matching single-level config
>   [3/3]: parse_hide_refs_config: tell parse_config_key we don't want a subsection

While you were doing that, I was grepping the call sites for parse_config_key() and made sure that all of them are OK when fed two level names. Most of them follow this pattern:

	if (parse_config_key(k, "diff", &name, &namelen, &type) || !name)
		return -1;

and ones that do not immediately check !name does either eventually do so or have separate codepaths for handlihng two- and three-level names.