From b21ee60594c637b4ea5ab98d5cc60b34a66bdde4 Mon Sep 17 00:00:00 2001
From: John Ericson <John.Ericson@Obsidian.Systems>
Date: Mon, 11 Dec 2023 13:28:40 -0500
Subject: [PATCH] Get rid of `verifyAllValidPaths` boolean blindness

---
 src/libstore/local-overlay-store.cc |  7 +++++--
 src/libstore/local-overlay-store.hh |  2 +-
 src/libstore/local-store.cc         |  7 +++++--
 src/libstore/local-store.hh         | 26 ++++++++++++++++++++++----
 4 files changed, 33 insertions(+), 9 deletions(-)

diff --git a/src/libstore/local-overlay-store.cc b/src/libstore/local-overlay-store.cc
index f657d8341..598415db8 100644
--- a/src/libstore/local-overlay-store.cc
+++ b/src/libstore/local-overlay-store.cc
@@ -252,7 +252,7 @@ void LocalOverlayStore::optimiseStore()
 }
 
 
-std::pair<bool, StorePathSet> LocalOverlayStore::verifyAllValidPaths(RepairFlag repair)
+LocalStore::VerificationResult LocalOverlayStore::verifyAllValidPaths(RepairFlag repair)
 {
     StorePathSet done;
 
@@ -266,7 +266,10 @@ std::pair<bool, StorePathSet> LocalOverlayStore::verifyAllValidPaths(RepairFlag
     for (auto & i : queryAllValidPaths())
         verifyPath(i, existsInStoreDir, done, validPaths, repair, errors);
 
-    return { errors, validPaths };
+    return {
+        .errors = errors,
+        .validPaths = validPaths,
+    };
 }
 
 
diff --git a/src/libstore/local-overlay-store.hh b/src/libstore/local-overlay-store.hh
index 64ed2f20a..a93b069cc 100644
--- a/src/libstore/local-overlay-store.hh
+++ b/src/libstore/local-overlay-store.hh
@@ -188,7 +188,7 @@ private:
      * We don't verify the contents of both layers on the assumption that the lower layer is far bigger,
      * and also the observation that anything not in the upper db the overlayfs doesn't yet care about.
      */
-    std::pair<bool, StorePathSet> verifyAllValidPaths(RepairFlag repair) override;
+    VerificationResult verifyAllValidPaths(RepairFlag repair) override;
 
     /**
      * Deletion only effects the upper layer, so we ignore lower-layer referrers.
diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc
index 8561d58be..b35d59bbb 100644
--- a/src/libstore/local-store.cc
+++ b/src/libstore/local-store.cc
@@ -1443,7 +1443,7 @@ bool LocalStore::verifyStore(bool checkContents, RepairFlag repair)
 }
 
 
-std::pair<bool, StorePathSet> LocalStore::verifyAllValidPaths(RepairFlag repair)
+LocalStore::VerificationResult LocalStore::verifyAllValidPaths(RepairFlag repair)
 {
     StorePathSet storePathsInStoreDir;
     /* Why aren't we using `queryAllValidPaths`? Because that would
@@ -1476,7 +1476,10 @@ std::pair<bool, StorePathSet> LocalStore::verifyAllValidPaths(RepairFlag repair)
     for (auto & i : queryAllValidPaths())
         verifyPath(i, existsInStoreDir, done, validPaths, repair, errors);
 
-    return { errors, validPaths };
+    return {
+        .errors = errors,
+        .validPaths = validPaths,
+    };
 }
 
 
diff --git a/src/libstore/local-store.hh b/src/libstore/local-store.hh
index 9a9171e2f..dc2d5cb7c 100644
--- a/src/libstore/local-store.hh
+++ b/src/libstore/local-store.hh
@@ -264,12 +264,30 @@ public:
 
     bool verifyStore(bool checkContents, RepairFlag repair) override;
 
+protected:
+
     /**
-     * @return A pair of whether any errors were encountered, and a set of
-     * (so-far) valid paths. The store objects pointed to by those paths are
-     * suitable for further validation checking.
+     * Result of `verifyAllValidPaths`
      */
-    virtual std::pair<bool, StorePathSet> verifyAllValidPaths(RepairFlag repair);
+    struct VerificationResult {
+        /**
+         * Whether any errors were encountered
+         */
+        bool errors;
+
+        /**
+         * A set of so-far valid paths. The store objects pointed to by
+         * those paths are suitable for further validation checking.
+         */
+        StorePathSet validPaths;
+    };
+
+    /**
+     * First, unconditional step of `verifyStore`
+     */
+    virtual VerificationResult verifyAllValidPaths(RepairFlag repair);
+
+public:
 
     /**
      * Register the validity of a path, i.e., that `path` exists, that