Re: [PATCH v2 1/4] refs: add function to translate errors to strings

From: Jeff King <peff@peff.net>
Date: May 15, 2025, 19:11 UTC
Message-ID: <20250515191153.GB3309052@coredump.intra.peff.net>
In-Reply-To: <20250515-501-update-git-fetch-1-to-use-partial-transactions-v2-1-80cbaaa55d2e@gmail.com>

On Thu, May 15, 2025 at 04:07:25PM +0200, Karthik Nayak wrote:

Show 29 quoted lines

> +char *ref_transaction_error_msg(enum ref_transaction_error err)
> +{
> +	const char *reason = "";
> +
> +	switch (err) {
> +	case REF_TRANSACTION_ERROR_NAME_CONFLICT:
> +		reason = "refname conflict";
> +		break;
> +	case REF_TRANSACTION_ERROR_CREATE_EXISTS:
> +		reason = "reference already exists";
> +		break;
> +	case REF_TRANSACTION_ERROR_NONEXISTENT_REF:
> +		reason = "reference does not exist";
> +		break;
> +	case REF_TRANSACTION_ERROR_INCORRECT_OLD_VALUE:
> +		reason = "incorrect old value provided";
> +		break;
> +	case REF_TRANSACTION_ERROR_INVALID_NEW_VALUE:
> +		reason = "invalid new value provided";
> +		break;
> +	case REF_TRANSACTION_ERROR_EXPECTED_SYMREF:
> +		reason = "expected symref but found regular ref";
> +		break;
> +	default:
> +		reason = "unkown failure";
> +	}
> +
> +	return xstrdup(reason);
> +}

The assignment of "" is dead code, I think? We will always assign "unknown failure" as a last resort. Not a big deal, but just something I noticed while reading this related to what's going on in patch 4.

Also, s/unkown/unknown/, but that is present in the pre-image. I hope we don't need to retain it for bug-for-bug plumbing compatibility. :)

(I guess the dead store of "" was present in the original, too, for that matter).

-Peff

PS Sorry for all the nit-picky comments. I was just going down the
   Coverity rabbit hole and didn't really review the rest of the series.
   But I wanted to say that the numbers you are seeing are very cool!