diff mbox series

[6/8] refs: clear errno return in refs_resolve_ref_unsafe()

Message ID 1bb350ea5d21eacf95426192c89173b502b7e06a.1619710329.git.gitgitgadget@gmail.com (mailing list archive)
State New, archived
Headers show
Series refs: cleanup errno sideband ref related functions | expand

Commit Message

Han-Wen Nienhuys April 29, 2021, 3:32 p.m. UTC 
From: Han-Wen Nienhuys <hanwen@google.com>

This is done in a separate commit, to pinpoint the precise cause should there be
regressions in error reporting.

Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
---
 refs.c | 23 +++++++++++++++++------
 1 file changed, 17 insertions(+), 6 deletions(-)

Comments

Jonathan Tan June 3, 2021, 2:53 a.m. UTC | #1 
> @@ -1685,10 +1685,11 @@ int refs_read_raw_ref(struct ref_store *ref_store, const char *refname,
>  }
>  
>  /* This function needs to return a meaningful errno on failure */
> -const char *refs_resolve_ref_unsafe(struct ref_store *refs,
> -				    const char *refname,
> -				    int resolve_flags,
> -				    struct object_id *oid, int *flags)
> +static const char *refs_resolve_ref_unsafe_errno(struct ref_store *refs,
> +						 const char *refname,
> +						 int resolve_flags,
> +						 struct object_id *oid,
> +						 int *flags)

So a third function (refs_resolve_ref_unsafe_errno() - not to be
confused with refs_resolve_ref_unsafe_with_errno(), which has an extra
"with")? Couldn't we just swap the other 2 functions directly instead of
going through this intermediary?

> +const char *refs_resolve_ref_unsafe(struct ref_store *refs, const char *refname,
> +				    int resolve_flags, struct object_id *oid,
> +				    int *flags)
> +{
> +	const char *result = refs_resolve_ref_unsafe_errno(
> +		refs, refname, resolve_flags, oid, flags);
> +	errno = 0;
> +	return result;
> +}

This is the errno = 0 part that I was talking about in my review of patch 4.
Han-Wen Nienhuys June 10, 2021, 11:45 a.m. UTC | #2 
On Thu, Jun 3, 2021 at 4:53 AM Jonathan Tan <jonathantanmy@google.com> wrote:
>
> > @@ -1685,10 +1685,11 @@ int refs_read_raw_ref(struct ref_store *ref_store, const char *refname,
> >  }
> >
> >  /* This function needs to return a meaningful errno on failure */
> > -const char *refs_resolve_ref_unsafe(struct ref_store *refs,
> > -                                 const char *refname,
> > -                                 int resolve_flags,
> > -                                 struct object_id *oid, int *flags)
> > +static const char *refs_resolve_ref_unsafe_errno(struct ref_store *refs,
> > +                                              const char *refname,
> > +                                              int resolve_flags,
> > +                                              struct object_id *oid,
> > +                                              int *flags)
>
> So a third function (refs_resolve_ref_unsafe_errno() - not to be
> confused with refs_resolve_ref_unsafe_with_errno(), which has an extra
> "with")? Couldn't we just swap the other 2 functions directly instead of
> going through this intermediary?

I've clarified the name. I've done it this way, because it keeps the
diff small. Swapping the functions would require code changes that I
thought would be more work to review.
diff mbox series

Patch

diff --git a/refs.c b/refs.c
index 08f69e2a16f6..6e746cb01f24 100644
--- a/refs.c
+++ b/refs.c
@@ -1685,10 +1685,11 @@  int refs_read_raw_ref(struct ref_store *ref_store, const char *refname,
 }
 
 /* This function needs to return a meaningful errno on failure */
-const char *refs_resolve_ref_unsafe(struct ref_store *refs,
-				    const char *refname,
-				    int resolve_flags,
-				    struct object_id *oid, int *flags)
+static const char *refs_resolve_ref_unsafe_errno(struct ref_store *refs,
+						 const char *refname,
+						 int resolve_flags,
+						 struct object_id *oid,
+						 int *flags)
 {
 	static struct strbuf sb_refname = STRBUF_INIT;
 	struct object_id unused_oid;
@@ -1777,14 +1778,24 @@  const char *refs_resolve_ref_unsafe(struct ref_store *refs,
 	return NULL;
 }
 
+const char *refs_resolve_ref_unsafe(struct ref_store *refs, const char *refname,
+				    int resolve_flags, struct object_id *oid,
+				    int *flags)
+{
+	const char *result = refs_resolve_ref_unsafe_errno(
+		refs, refname, resolve_flags, oid, flags);
+	errno = 0;
+	return result;
+}
+
 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);
+	const char *result = refs_resolve_ref_unsafe_errno(
+		refs, refname, resolve_flags, oid, flags);
 	*failure_errno = errno;
 	return result;
 }