Re: [PATCH] make config --add behave correctly for empty and NULL values

From: Jeff King <peff@peff.net>
Date: Aug 19, 2014, 05:17 UTC
Message-ID: <20140819051732.GA13765@peff.net>
In-Reply-To: <xmqqvbppwtir.fsf@gitster.dls.corp.google.com>

On Mon, Aug 18, 2014 at 11:18:52AM -0700, Junio C Hamano wrote:

> Are we sure that "a^", which cannot be true for any string, will not
> be caught by anybody's regcomp() as an error?  I know regcomp()
> accepts the expression and regexec() fails to match with GNU libc,
> but that is not the whole of the world.

We do support negation (ourselves) in the regexp, so "!$foo" would work, where "$foo" is some regexp that always matches. But that may be digging ourselves the opposite hole, trying to find a pattern that reliably matches everything.

> To be honest, I'd rather see this done "right", by giving an option
> to the caller to tell the function not to call regcomp/regexec in
> matches().

Yeah, that was my first thought, too on seeing the patch. I even worked up an example before reading your message, but:

Show 14 quoted lines

>  * Define a global exported via cache.h and defined in config.c
> 
> 	extern const char CONFIG_SET_MULTIVAR_NO_REPLACE[];
> 
>    and pass it from this calling site, instead of an arbitrary
>    literal string e.g. "a^"
> 
>  * Add a bit to the "store" struct, e.g. "unsigned value_never_matches:1";
> 
>  * In git_config_set_multivar_in_file() implementation, check for
>    this constant address and set store.value_never_matches to true;
> 
>  * in matches(), check this bit and always return "No, this existing
>    value do not match" when it is set.

I just used

  #define CONFIG_REGEX_NONE ((void *)1)

as my magic sentinel value, both for the string and compiled regex versions. Adding a bit to the store struct is a lot less disgusting and error-prone. So I won't share mine here. :)

-Peff