refs/files: improve error handling when verifying symrefs

The error handling when verifying symbolic refs is a bit on the wild
side:

  - `fsck_report_ref()` can be told to ignore specific errors. If an
    error has been ignored and a previous check raised an unignored
    error, then assigning `ret = fsck_report_ref()` will cause us to
    swallow the previous error.

  - When the target reference is not valid we bail out early without
    checking for other errors.

Fix both of these issues by consistently or'ing the return value and not
bailing out early.

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

diff --git a/refs/files-backend.c b/refs/files-backend.c
index 9972221f9f48197664edc911685fe3cb6ad63b5b..abc21653395b132a577977f56275b6a2ec289ad9 100644 (file)
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -3737,17 +3737,15 @@ static int files_fsck_symref_target(struct fsck_options *o,
        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);
-
+               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);
-               goto out;
+               ret |= fsck_report_ref(o, report,
+                                      FSCK_MSG_BAD_REFERENT_NAME,
+                                      "points to invalid refname '%s'", referent->buf);
        }
 
        if (symbolic_link)
@@ -3755,19 +3753,19 @@ static int files_fsck_symref_target(struct fsck_options *o,
 
        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");
+               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");
+               ret |= fsck_report_ref(o, report,
+                                      FSCK_MSG_TRAILING_REF_CONTENT,
+                                      "has trailing whitespaces or newlines");
        }
 
 out:
-       return ret;
+       return ret ? -1 : 0;
 }
 
 static int files_fsck_refs_content(struct ref_store *ref_store,