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

[Jeff King <peff@peff.net>]

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:

>  * 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