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 my 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>
---
refs.c | 21 ++++++++++++++++++++
refs.h | 10 ++++++++++
refs/files-backend.c | 54 ++++++++++++++++++++--------------------------------
3 files changed, 52 insertions(+), 33 deletions(-)
Show changes to 3 files +52 −33
refs.c, refs.h, refs/files-backend.c
diff --git a/refs.c b/refs.c
index e06e0cb072..739bf9fefc 100644
--- a/refs.c
+++ b/refs.c
@@ -320,6 +320,27 @@ int check_refname_format(const char *refname, int flags)
return check_or_sanitize_refname(refname, flags, NULL);
}
+int refs_fsck_symref(struct ref_store *refs UNUSED, struct fsck_options *o,
+ struct fsck_ref_report *report,
+ const char *refname UNUSED, const char *target)
+{
+ if (is_root_ref(target))
+ return 0;
+
+ if (check_refname_format(target, 0) &&
+ fsck_report_ref(o, report, FSCK_MSG_BAD_REFERENT_NAME,
+ "points to invalid refname '%s'", target))
+ return -1;
+
+ if (!starts_with(target, "refs/") &&
+ !starts_with(target, "worktrees/") &&
+ fsck_report_ref(o, report, FSCK_MSG_SYMREF_TARGET_IS_NOT_A_REF,
+ "points to non-ref target '%s'", target))
+ return -1;
+
+ return 0;
+}
+
int refs_fsck(struct ref_store *refs, struct fsck_options *o,
struct worktree *wt)
{
diff --git a/refs.h b/refs.h
index d9051bbb04..d91fcb2d2f 100644
--- a/refs.h
+++ b/refs.h
@@ -653,6 +653,16 @@ int refs_for_each_reflog(struct ref_store *refs, each_reflog_fn fn, void *cb_dat
*/
int check_refname_format(const char *refname, int flags);
+struct fsck_ref_report;
+
+/*
+ * Perform generic checks for a specific symref target. This function is
+ * expected to be called by the ref backends for every symbolic ref.
+ */
+int refs_fsck_symref(struct ref_store *refs, struct fsck_options *o,
+ struct fsck_ref_report *report,
+ const char *refname, const char *target);
+
/*
* Check the reference database for consistency. Return 0 if refs and
* reflogs are consistent, and non-zero otherwise. The errors will be
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 0ff047d0df..72c1db849e 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;
}
Patrick Steinhardt
