From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Sun, 11 Jul 2021 11:52:00 GMT
Subject: Re: [PATCH v5 3/6] refs: make errno output explicit for read_raw_ref_fn
Message-ID: <87im1h3w3w.fsf@evledraar.gmail.com>
In-Reply-To: <7feedb9720127605b5e84aa1143c6ddc736597f3.1625684869.git.gitgitgadget@gmail.com>


On Wed, Jul 07 2021, Han-Wen Nienhuys via GitGitGadget wrote:

> diff --git a/refs.c b/refs.c
> index 8c9490235ea..5e5e3af8da0 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -1675,13 +1675,19 @@ int refs_read_raw_ref(struct ref_store *ref_store,
>  		      const char *refname, struct object_id *oid,
>  		      struct strbuf *referent, unsigned int *type)
>  {
> +	int result;
> +	int failure_errno;
>  	if (!strcmp(refname, "FETCH_HEAD") || !strcmp(refname, "MERGE_HEAD")) {
>  		return refs_read_special_head(ref_store, refname, oid, referent,
>  					      type);
>  	}
>  
> -	return ref_store->be->read_raw_ref(ref_store, refname, oid, referent,
> -					   type);
> +	failure_errno = 0;
> +	result = ref_store->be->read_raw_ref(ref_store, refname, oid, referent,
> +					     type, &failure_errno);
> +	if (failure_errno)
> +		errno = failure_errno;
> +	return result;
>  }

Just reading along again in sequence, and having forgotten where (if
anywhere) this was headed I still find this verbose pattern for /looking
like/ we'll be doing something that's not setting errno, but actually
just setting errno. I.e. the logic is the same as the below squash, but
let's read on...

diff --git a/refs.c b/refs.c
index 5e5e3af8da0..7b3f05a66ff 100644
--- a/refs.c
+++ b/refs.c
@@ -1675,19 +1675,13 @@ int refs_read_raw_ref(struct ref_store *ref_store,
 		      const char *refname, struct object_id *oid,
 		      struct strbuf *referent, unsigned int *type)
 {
-	int result;
-	int failure_errno;
 	if (!strcmp(refname, "FETCH_HEAD") || !strcmp(refname, "MERGE_HEAD")) {
 		return refs_read_special_head(ref_store, refname, oid, referent,
 					      type);
 	}
 
-	failure_errno = 0;
-	result = ref_store->be->read_raw_ref(ref_store, refname, oid, referent,
-					     type, &failure_errno);
-	if (failure_errno)
-		errno = failure_errno;
-	return result;
+	return ref_store->be->read_raw_ref(ref_store, refname, oid, referent,
+					   type, &errno);
 }
 
 /* This function needs to return a meaningful errno on failure */