refs/files: extract generic symref target checks

The consistency checks for the "files" backend contain a couple of
verifications for symrefs that verify generic properties of the target
reference. These properties need to hold for every backend, no matter
whether it's using the "files" or "reftable" backend.

Reimplementing these checks for every single backend doesn't really make
sense. Extract it into a generic `refs_fsck_symref()` function that can
be used by other backends, as well. The "reftable" backend will be wired
up in a subsequent commit.

While at it, improve the consistency checks so that we don't complain
about refs pointing to a non-ref target in case the target refname
format does not verify. Otherwise it's very likely that we'll generate
both error messages, which feels somewhat redundant in this case.

Note that the function has a couple of `UNUSED` parameters. These will
become referenced in a subsequent commit.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

1 files changed, 21 insertions, 33 deletions

diff --git a/refs/files-backend.c b/refs/files-backend.c
index 9ae80b700a..687c26ddcb 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -3718,53 +3718,39 @@ typedef int (*files_fsck_refs_fn)(struct ref_store *ref_store,
 				  const char *path,
 				  int mode);
 
-static int files_fsck_symref_target(struct fsck_options *o,
+static int files_fsck_symref_target(struct ref_store *ref_store,
+				    struct fsck_options *o,
 				    struct fsck_ref_report *report,
+				    const char *refname,
 				    struct strbuf *referent,
 				    unsigned int symbolic_link)
 {
-	int is_referent_root;
 	char orig_last_byte;
 	size_t orig_len;
 	int ret = 0;
 
 	orig_len = referent->len;
 	orig_last_byte = referent->buf[orig_len - 1];
-	if (!symbolic_link)
-		strbuf_rtrim(referent);
-
-	is_referent_root = is_root_ref(referent->buf);
-	if (!is_referent_root &&
-	    !starts_with(referent->buf, "refs/") &&
-	    !starts_with(referent->buf, "worktrees/")) {
-		ret |= fsck_report_ref(o, report,
-				       FSCK_MSG_SYMREF_TARGET_IS_NOT_A_REF,
-				       "points to non-ref target '%s'", referent->buf);
-	}
 
-	if (!is_referent_root && check_refname_format(referent->buf, 0)) {
-		ret |= fsck_report_ref(o, report,
-				       FSCK_MSG_BAD_REFERENT_NAME,
-				       "points to invalid refname '%s'", referent->buf);
-	}
+	if (!symbolic_link) {
+		strbuf_rtrim(referent);
 
-	if (symbolic_link)
-		goto out;
+		if (referent->len == orig_len ||
+		    (referent->len < orig_len && orig_last_byte != '\n')) {
+			ret |= fsck_report_ref(o, report,
+					       FSCK_MSG_REF_MISSING_NEWLINE,
+					       "misses LF at the end");
+		}
 
-	if (referent->len == orig_len ||
-	    (referent->len < orig_len && orig_last_byte != '\n')) {
-		ret |= fsck_report_ref(o, report,
-				       FSCK_MSG_REF_MISSING_NEWLINE,
-				       "misses LF at the end");
+		if (referent->len != orig_len && referent->len != orig_len - 1) {
+			ret |= fsck_report_ref(o, report,
+					       FSCK_MSG_TRAILING_REF_CONTENT,
+					       "has trailing whitespaces or newlines");
+		}
 	}
 
-	if (referent->len != orig_len && referent->len != orig_len - 1) {
-		ret |= fsck_report_ref(o, report,
-				       FSCK_MSG_TRAILING_REF_CONTENT,
-				       "has trailing whitespaces or newlines");
-	}
+	ret |= refs_fsck_symref(ref_store, o, report, refname, referent->buf);
 
-out:
 	return ret ? -1 : 0;
 }
 
@@ -3807,7 +3793,8 @@ static int files_fsck_refs_content(struct ref_store *ref_store,
 		else
 			strbuf_addbuf(&referent, &ref_content);
 
-		ret |= files_fsck_symref_target(o, &report, &referent, 1);
+		ret |= files_fsck_symref_target(ref_store, o, &report,
+						target_name, &referent, 1);
 		goto cleanup;
 	}
 
@@ -3847,7 +3834,8 @@ static int files_fsck_refs_content(struct ref_store *ref_store,
 			goto cleanup;
 		}
 	} else {
-		ret = files_fsck_symref_target(o, &report, &referent, 0);
+		ret = files_fsck_symref_target(ref_store, o, &report,
+					       target_name, &referent, 0);
 		goto cleanup;
 	}