Re: [PATCH v3 05/11] bundle-uri: parse bundle.<id>.creationToken values

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 31, 2023, 21:22 UTC
Message-ID: <xmqq4js6xuu2.fsf@gitster.g>
In-Reply-To: <ff629bc119b466dcd827f758b3d892fefd6a9703.1675171760.git.gitgitgadget@gmail.com>

"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:

> +	if (info->creationToken)
> +		fprintf(fp, "\tcreationToken = %"PRIu64"\n", info->creationToken);

So, a bundle info with .creationToken set to 0 signals that the token is missing for that bundle. REMOTE_BUNDLE_INFO_INIT clears all members, so unless we find a concrete value and assign to the member, we'll have 0 in the member (and never a random value).

Very good.

We used to avoid camelCased variable names and member names and instead prefer snake_case; I wonder if it is still the case. Looking at bundle.h it still seems to be the case, and we may want to match that.

Show 10 quoted lines

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

Should this be a warning, or a hard error? I _think_ it depends on how much we trust creationToken supplied by the bundle providers for correctness. If we only use the values as hint to gain performance, and downloading the bundles in a wrong order (due to bogus creationToken values assigned to them) does not lead to correctness error, then warning is fine.

Looking good.