[v14,03/10] refs: standardize output of refs_read_symbolic_ref

Commit Message

Bence Ferdinandy Nov. 21, 2024, 10:55 p.m. UTC

When the symbolic reference we want to read with refs_read_symbolic_ref
is actually not a symbolic reference, the files and the reftable
backends return different values (1 and -1 respectively). Standardize
the returned values so that 0 is success, -1 is a generic error and -2
is that the reference was actually non-symbolic.

Signed-off-by: Bence Ferdinandy <[e-mail]>
---

Notes:
    v13: new patch
    
    v14: - simplified the logic in reftables backend (thanks Junio)
         - update the comment in refs.h (thanks Patrick)
         - rewrote comment in refs-internal.h to point to the one in refs.h
         - created NOT_A_SYMREF=-2 constant

 refs.h                  | 11 +++++++++++
 refs/files-backend.c    |  7 +++----
 refs/refs-internal.h    |  5 +++++
 refs/reftable-backend.c |  8 +++++---
 4 files changed, 24 insertions(+), 7 deletions(-)

Patch

diff --git a/refs.h b/refs.h
index 108dfc93b3..22c2b45b8b 100644
--- a/refs.h
+++ b/refs.h
@@ -83,6 +83,17 @@  int refs_read_ref_full(struct ref_store *refs, const char *refname,
 
 int refs_read_ref(struct ref_store *refs, const char *refname, struct object_id *oid);
 
+#define NOT_A_SYMREF -2
+
+/*
+ * Read the symbolic ref named "refname" and write its immediate referent into
+ * the provided buffer. Referent is left empty if "refname" is not a symbolic
+ * ref. It does not resolve the symbolic reference recursively in case the
+ * target is also a symbolic ref.
+ *
+ * Returns 0 on success, -2 if the "refname" is not a symbolic ref,
+ * -1 otherwise.
+ */
 int refs_read_symbolic_ref(struct ref_store *ref_store, const char *refname,
 			   struct strbuf *referent);
 
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 0824c0b8a9..4cc43c32f2 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -596,10 +596,9 @@  static int files_read_symbolic_ref(struct ref_store *ref_store, const char *refn
 	unsigned int type;
 
 	ret = read_ref_internal(ref_store, refname, &oid, referent, &type, &failure_errno, 1);
-	if (ret)
-		return ret;
-
-	return !(type & REF_ISSYMREF);
+	if (!ret && !(type & REF_ISSYMREF))
+		return NOT_A_SYMREF;
+	return ret;
 }
 
 int parse_loose_ref_contents(const struct git_hash_algo *algop,
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index 2313c830d8..1399fee61c 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -673,6 +673,11 @@  struct ref_storage_be {
 
 	ref_iterator_begin_fn *iterator_begin;
 	read_raw_ref_fn *read_raw_ref;
+
+	/*
+	 * Please refer to `refs_read_symbolic_ref()` for the expected
+	 * behaviour.
+	 */
 	read_symbolic_ref_fn *read_symbolic_ref;
 
 	reflog_iterator_begin_fn *reflog_iterator_begin;
diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c
index 38eb14d591..1809e3426a 100644
--- a/refs/reftable-backend.c
+++ b/refs/reftable-backend.c
@@ -830,10 +830,12 @@  static int reftable_be_read_symbolic_ref(struct ref_store *ref_store,
 		return ret;
 
 	ret = reftable_stack_read_ref(stack, refname, &ref);
-	if (ret == 0 && ref.value_type == REFTABLE_REF_SYMREF)
+        if (ret)
+                ret = -1;
+        else if (ref.value_type == REFTABLE_REF_SYMREF)
 		strbuf_addstr(referent, ref.value.symref);
-	else
-		ret = -1;
+        else
+                ret = NOT_A_SYMREF;
 
 	reftable_ref_record_release(&ref);
 	return ret;