Subject: Re: [PATCH v2 4/8] refs: make errno output explicit for refs_resolve_ref_unsafe

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Thu, 01 Jul 2021 13:56:19 +0200

On Thu, Jun 10 2021, Han-Wen Nienhuys via GitGitGadget wrote:

> From: Han-Wen Nienhuys <[email-masked]>
>
> This introduces refs_resolve_ref_unsafe_with_errno(), which makes the API
> contract for the errno output explicit. The implementation still relies on
> the global errno variable to ensure no side effects of this refactoring.
>
> In a follow-on commits, we will
>
> * migrate callers that need this error information
>
> * clear errno in refs_resolve_ref_unsafe() to make sure these other callers
> aren't using the error output accidentally.
>
> Signed-off-by: Han-Wen Nienhuys <[email-masked]>
> Reviewed-by: Jonathan Tan <[email-masked]>
> ---
>  refs.c               | 12 ++++++++++++
>  refs/refs-internal.h |  8 ++++++++
>  2 files changed, 20 insertions(+)
>
> diff --git a/refs.c b/refs.c
> index bebe3f584da7..64e2d55adcfb 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -1781,6 +1781,18 @@ const char *refs_resolve_ref_unsafe(struct ref_store *refs,
>  	return NULL;
>  }
>  
> +const char *refs_resolve_ref_unsafe_with_errno(struct ref_store *refs,
> +					       const char *refname,
> +					       int resolve_flags,
> +					       struct object_id *oid,
> +					       int *flags, int *failure_errno)
> +{
> +	const char *result = refs_resolve_ref_unsafe(refs, refname,
> +						     resolve_flags, oid, flags);
> +	*failure_errno = errno;
> +	return result;
> +}
> +
>  /* backend functions */
>  int refs_init_db(struct strbuf *err)
>  {
> diff --git a/refs/refs-internal.h b/refs/refs-internal.h
> index 904b2a9e51ae..eb97023658f8 100644
> --- a/refs/refs-internal.h
> +++ b/refs/refs-internal.h
> @@ -153,6 +153,14 @@ int refs_read_raw_ref(struct ref_store *ref_store,
>  		      const char *refname, struct object_id *oid,
>  		      struct strbuf *referent, unsigned int *type);
>  
> +/* Like refs_resolve_ref_unsafe, but provide access to errno code that lead to a
> + * failure. */
> +const char *refs_resolve_ref_unsafe_with_errno(struct ref_store *refs,
> +					       const char *refname,
> +					       int resolve_flags,
> +					       struct object_id *oid,
> +					       int *flags, int *failure_errno);
> +
>  /*
>   * Write an error to `err` and return a nonzero value iff the same
>   * refname appears multiple times in `refnames`. `refnames` must be

Okey, this might be eligable for comments similar to what I've had so
far, but no callers yet. Let's read on to those commits, right now I'm
thinking "this looks small enough that it should be squashed into that
future commit"....