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

[Patrick Steinhardt <ps@pks.im>]

On Mon, Nov 04, 2024 at 07:02:43PM +0000, Elijah Newren via GitGitGadget wrote:
> 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