Re: [PATCH v2 14/15] bundle-uri: handle NULL values in bundle lists

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

Hi Thomas

I'll defer to Stolee on what's the best behavior here, but it seems that 
in a couple of places we're erroring out on a NULL value when we would 
ignore an invalid value.

On 10/11/2026 07:36, Thomas Bachem via GitGitGadget wrote:
> From: Thomas Bachem <mail@thomasbachem.com>
> 
> diff --git a/bundle-uri.c b/bundle-uri.c
> index 67a717e857..41c817eefb 100644
> --- a/bundle-uri.c
> +++ b/bundle-uri.c> @@ -175,6 +177,9 @@ static int bundle_list_update(const char *key, 
const char *value,
>   
>   		if (!strcmp(subkey, "heuristic")) {
>   			int i;
> +
> +			if (!value)
> +				return config_error_nonbool(key);

Adding a bit more context below shows we ignore unknown heuristics, so 
do we really want to return an error here?
>   			for (i = 0; i < BUNDLE_HEURISTIC__COUNT; i++) {
>   				if (heuristics[i].heuristic &&
>   				    heuristics[i].name &&
                                      !strcmp(value, heuristics[i].name)) {
                                          list->heuristic = 
heuristics[i].heuristic;
                                          return 0;
                                  }
                          }

                          /* Ignore unknown heuristics. */
                          return 0;

> @@ -216,6 +223,8 @@ static int bundle_list_update(const char *key, const char *value,
>   	}
>   
>   	if (!strcmp(subkey, "creationtoken")) {
> +		if (!value)
> +			return config_error_nonbool(key);
>   		if (sscanf(value, "%"PRIu64, &bundle->creationToken) != 1)
>   			warning(_("could not parse bundle list key %s with value '%s'"),
>   				"creationToken", value);
		return 0;
	}

Here we ignore parse errors, so should we be returning an error on a 
NULL value?

Thanks

Phillip

> diff --git a/t/t5750-bundle-uri-parse.sh b/t/t5750-bundle-uri-parse.sh
> index 70ae94c4d0..a91c1b428d 100755
> --- a/t/t5750-bundle-uri-parse.sh
> +++ b/t/t5750-bundle-uri-parse.sh
> @@ -312,4 +312,20 @@ test_expect_success 'parse config format: bundle with url instead of uri' '
>   	test_grep "bundle '\''typo'\'' has no uri" err
>   '
>   
> +test_expect_success 'parse config format edge cases: keys without a value' '
> +	for key in mode heuristic
> +	do
> +		printf "[bundle]\n\tversion = 1\n\t%s\n" "$key" >input &&
> +		test_must_fail test-tool bundle-uri parse-config input 2>err &&
> +		test_grep "missing value for .bundle.$key." err || return 1
> +	done &&
> +	for key in uri creationtoken
> +	do
> +		printf "[bundle]\n\tversion = 1\n\tmode = all\n[bundle \"one\"]\n\t%s\n" \
> +			"$key" >input &&
> +		test_must_fail test-tool bundle-uri parse-config input 2>err &&
> +		test_grep "missing value for .bundle.one.$key." err || return 1
> +	done
> +'
> +
>   test_done