All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Taylor Blau <me@ttaylorr.com>
Cc: Han Jiang <jhcarl0814@gmail.com>,
	 git@vger.kernel.org,  Patrick Steinhardt <ps@pks.im>,
	 Derrick Stolee <stolee@gmail.com>
Subject: Re: git config set --file, key-value pair without '= value', gives Segmentation fault
Date: Wed, 31 Jul 2024 15:47:04 -0700	[thread overview]
Message-ID: <xmqq5xslyyaf.fsf@gitster.g> (raw)
In-Reply-To: <ZqqucpNgqSgZDPtA@nand.local> (Taylor Blau's message of "Wed, 31 Jul 2024 17:36:50 -0400")

> So I think that the behavior dates back to c90702a1f6 (config: plumb
> --fixed-value into config API, 2020-11-25). I think that the fix looks
> something like:
>
> --- 8< ---
> diff --git a/config.c b/config.c
> index 6421894614..05f369ec0d 100644
> --- a/config.c
> +++ b/config.c
> @@ -2914,7 +2914,7 @@ static int matches(const char *key, const char *value,
>  {
>  	if (strcmp(key, store->key))
>  		return 0; /* not ours */
> -	if (store->fixed_value)
> +	if (store->fixed_value && value)
>  		return !strcmp(store->fixed_value, value);
>  	if (!store->value_pattern)
>  		return 1; /* always matches */

Hmph. fixed_value is about the string given on the command line
being not a pattern.  And value that is NULL is the valueless "true"
in this case.

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.

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'd like to hear from Stolee (CC'd), who is the author of c90702a1f6
> before submitting this as a standalone patch.


  reply	other threads:[~2024-07-31 22:47 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-07-31 11:55 git config set --file, key-value pair without '= value', gives Segmentation fault Han Jiang
2024-07-31 21:36 ` Taylor Blau
2024-07-31 22:47   ` Junio C Hamano [this message]
2024-07-31 23:36     ` Taylor Blau
2024-07-31 23:46       ` Han Jiang
2024-08-01  1:28       ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqq5xslyyaf.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=jhcarl0814@gmail.com \
    --cc=me@ttaylorr.com \
    --cc=ps@pks.im \
    --cc=stolee@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.