Re: git config set --file, key-value pair without '= value', gives Segmentation fault

From: Taylor Blau <me@ttaylorr.com>
Date: Jul 31, 2024, 23:36 UTC
Message-ID: <ZqrKeG+alFldB7OY@nand.local>
In-Reply-To: <xmqq5xslyyaf.fsf@gitster.g>

On Wed, Jul 31, 2024 at 03:47:04PM -0700, Junio C Hamano wrote:

Show 19 quoted lines

> I would have actually expected the fix that follow your analysis (by
> the way, I found it really well done) would say something more like
>
> 	if (strcmp(key, store->key))
> 		return 0; /* not ours */
> +	if (!value)
> +		value = "true";
> 	if (store->fixed_value)
> 		return !strcmp(store->fixed_value, value);
>
> but I am not sure exactly how we want to handle synonymous Boolean
> values here.  Regardless of how "true" value is spelled in the
> configuration file, e.g.
>
>         [section]               [section]
>                 key                     key = true
>
> I wonder if "git config set --value=yes [--fixed-value] section.key
> false" should replace either of them to false.

I think it gets tricky when we talk about the implicit true value here for exactly that reason. Do we take true to mean just "true"? Or "true" and "yes", "1", "", and so on?

I tried to match the behavior a few lines down in that function, where we only call regexec() if value is non-NULL.

> It would especially true if the command is invoked with --type=bool
> but it seems that the --type option does not participate in the
> matching with the current value.

I think I could be convinced that --type=bool should treat "", "true", "1", "on", and so on as matching here, but I'm not sure.

> > I'd like to hear from Stolee (CC'd), who is the author of c90702a1f6
> > before submitting this as a standalone patch.

Thanks, Taylor