Re: [PATCH v3 00/15] Fix segfaults on settings given without a value

[Phillip Wood <phillip.wood123@gmail.com>]

Hi Thomas

On 11/11/2026 22:49, Thomas Bachem via GitGitGadget wrote:
> Changes since v2:
> 
>   * Where an invalid value is ignored, a missing one now is too, with a
>     warning: remote.*.tagOpt (patch 6) and color.blame.repeatedLines (patch
>     7). v2 rejected them (Phillip).
> 
>   * The test in patch 1 rebases onto HEAD~3 and checks the result with
>     test_commit_message (Phillip).

That sounds good, and the range-diff below looks as I'd expect. Thanks 
for working on this, as far as I'm concerned it is ready for next.

Phillip
> Range-diff vs v2:
> 
>    1:  47224a9893 !  1:  1262222ccd rebase -i: handle NULL value for rebase.missingCommitsCheck
>       @@ t/t3404-rebase-interactive.sh: test_expect_success 'rebase -i respects rebase.mi
>        +	rebase_setup_and_clean missing-commit &&
>        +	(
>        +		set_fake_editor &&
>       -+		FAKE_LINES="1 2 3 4" \
>       -+			git -c rebase.missingCommitsCheck rebase -i --root 2>actual
>       ++		FAKE_LINES=1 \
>       ++			git -c rebase.missingCommitsCheck rebase -i HEAD~3 2>actual
>        +	) &&
>       -+	test D = $(git cat-file commit HEAD | sed -ne \$p) &&
>       ++	test_commit_message HEAD -m C &&
>        +	test_grep "missing value for option rebase.missingCommitsCheck" actual &&
>        +	test_grep ! "commits may have been dropped" actual
>        +'
>    2:  9170c4162f =  2:  ec9d7fd183 odb: handle NULL values for core.alternateRefs* config
>    3:  d3fd6290e0 =  3:  c071e79d11 add-patch: handle NULL value for color slots
>    4:  a822cbb844 =  4:  8773aaef0b bundle-uri: handle NULL value for fetch.bundleCreationToken
>    5:  6c2a671a92 =  5:  123f03f024 grep: handle NULL value for grep.patternType
>    6:  d4b49d1363 !  6:  339590cd4b remote: handle NULL value for remote.<name>.tagOpt
>       @@ Commit message
>            "git remote" and "git status" segfault: handle_config() passes the
>            NULL value to strcmp().
>        
>       -    Reject a missing value with config_error_nonbool(), as handle_config()
>       -    already does for remote.<name>.url and remote.<name>.pushurl.
>       +    The setting takes "--no-tags" or "--tags", and any other value is
>       +    ignored. Ignore a missing value too, but warn about it.
>        
>       +    Helped-by: Phillip Wood <phillip.wood@dunelm.org.uk>
>            Assisted-by: Claude Fable 5.1
>       +    Assisted-by: Claude Opus 5.5
>            Signed-off-by: Thomas Bachem <mail@thomasbachem.com>
>        
>         ## remote.c ##
>       @@ remote.c: static int handle_config(const char *key, const char *value,
>         		else
>         			error(_("more than one uploadpack given, using the first"));
>         	} else if (!strcmp(subkey, "tagopt")) {
>       -+		if (!value)
>       -+			return config_error_nonbool(key);
>       ++		if (!value) {
>       ++			warning(_("missing value for '%s'"), key);
>       ++			return 0;
>       ++		}
>         		if (!strcmp(value, "--no-tags"))
>         			remote->fetch_tags = -1;
>         		else if (!strcmp(value, "--tags"))
>       @@ t/t5505-remote.sh: test_expect_success 'remote information for the origin' '
>         	)
>         '
>         
>       -+test_expect_success 'tagOpt needs a value' '
>       -+	test_must_fail git -C test -c remote.origin.tagOpt remote 2>err &&
>       ++test_expect_success 'tagOpt without a value only warns' '
>       ++	git -C test -c remote.origin.tagOpt remote >actual 2>err &&
>       ++	echo origin >expect &&
>       ++	test_cmp expect actual &&
>        +	test_grep "missing value for .remote.origin.tagopt." err
>        +'
>        +
>    7:  25843b43bf !  7:  ad50c4b35b blame: handle NULL values for color.blame.* config
>       @@ Commit message
>            value makes "git blame" segfault: git_blame_config() passes the NULL
>            value to strlen() or to parse_color_fields().
>        
>       -    Reject a missing value with config_error_nonbool(), as
>       +    An invalid color.blame.repeatedLines gets a warning and is ignored.
>       +    Treat a missing value the same way.
>       +
>       +    An invalid color.blame.highlightRecent makes "git blame" die, so
>       +    reject a missing value there with config_error_nonbool(), as
>            git_blame_config() already does for blame.date and blame.coloring.
>        
>       +    Helped-by: Phillip Wood <phillip.wood@dunelm.org.uk>
>            Assisted-by: Claude Fable 5.1
>       +    Assisted-by: Claude Opus 5.5
>            Signed-off-by: Thomas Bachem <mail@thomasbachem.com>
>        
>         ## builtin/blame.c ##
>       @@ builtin/blame.c: static int git_blame_config(const char *var, const char *value,
>         		return 0;
>         	}
>         	if (!strcmp(var, "color.blame.repeatedlines")) {
>       -+		if (!value)
>       -+			return config_error_nonbool(var);
>       ++		if (!value) {
>       ++			warning(_("missing value for '%s'"),
>       ++				"color.blame.repeatedLines");
>       ++			return 0;
>       ++		}
>         		if (color_parse_mem(value, strlen(value), repeated_meta_color))
>         			warning(_("invalid value for '%s': '%s'"),
>         				"color.blame.repeatedLines", value);
>       @@ t/t8012-blame-colors.sh: test_expect_success 'blame color by age: new code is di
>         	test_grep qfunc colored
>         '
>         
>       -+test_expect_success 'color.blame.repeatedLines and highlightRecent need a value' '
>       -+	test_must_fail git -c color.blame.repeatedLines blame hello.c 2>err &&
>       -+	test_grep "missing value for .color.blame.repeatedlines." err &&
>       ++test_expect_success 'color.blame.repeatedLines without a value only warns' '
>       ++	git blame --color-lines hello.c >expect &&
>       ++	git -c color.blame.repeatedLines blame --color-lines hello.c >actual 2>err &&
>       ++	test_cmp expect actual &&
>       ++	test_grep "missing value for .color.blame.repeatedLines." err
>       ++'
>       ++
>       ++test_expect_success 'color.blame.highlightRecent needs a value' '
>        +	test_must_fail git -c color.blame.highlightRecent blame hello.c 2>err &&
>        +	test_grep "missing value for .color.blame.highlightrecent." err
>        +'
>    8:  d8681a8560 =  8:  90209dc754 merge: handle NULL value for branch.<name>.mergeOptions
>    9:  51f8eba073 =  9:  f30ecf07b4 remote: handle NULL value for remotes.<group>
>   10:  a5b6436943 = 10:  bb6eecd20c setup: handle NULL value for safe.bareRepository
>   11:  1533ef9cc6 = 11:  510dc56c9b pseudo-merge: handle NULL value for bitmapPseudoMerge.<name>.pattern
>   12:  6219e9fd7f = 12:  751fae34b2 scalar: handle NULL value for scalar.repo
>   13:  080106d5ea = 13:  4d5d2ce04e config: handle NULL value for remote URLs in hasconfig includes
>   14:  fb9e63541b = 14:  474aebd572 bundle-uri: handle NULL values in bundle lists
>   15:  12f841e5ff = 15:  f6f3295ff5 config: handle NULL values when listing colors and expiry dates
>