Re: [PATCH 1/2] upload-pack: fix ambiguous error message

From: Patrick Steinhardt <ps@pks.im>
Date: Nov 5, 2024, 06:27 UTC
Message-ID: <Zym65pRWPqu9VIvq@pks.im>
In-Reply-To: <bb8dbc5c7086ce5c09f8980c923ec85da38c9147.1730746964.git.gitgitgadget@gmail.com>

On Mon, Nov 04, 2024 at 07:02:43PM +0000, Elijah Newren via GitGitGadget wrote:

Show 17 quoted lines

> diff --git a/upload-pack.c b/upload-pack.c
> index 6d6e0f9f980..640d45295e1 100644
> --- a/upload-pack.c
> +++ b/upload-pack.c
> @@ -1025,10 +1025,14 @@ static int process_deepen_not(const char *line, struct oidset *deepen_not, int *
>  {
>  	const char *arg;
>  	if (skip_prefix(line, "deepen-not ", &arg)) {
> +		int cnt;
>  		char *ref = NULL;
>  		struct object_id oid;
> -		if (expand_ref(the_repository, arg, strlen(arg), &oid, &ref) != 1)
> +		cnt = expand_ref(the_repository, arg, strlen(arg), &oid, &ref);
> +		if (cnt > 1)
>  			die("git upload-pack: ambiguous deepen-not: %s", line);
> +		if (cnt < 1)
> +			die("git upload-pack: deepen-not is not a ref: %s", line);

I was wondering whether `expand_ref()` could ever return negative, but after reading through its implementation that doesn't seem to be the case. It's somewhat misleading that it returns `int`, as `size_t` would convey the return value in a better spirit.

Anyway, that is not an issue of this patch series, and the change looks good.

Patrick