Make verifyAllValidPaths more functional

return map rather than mutate one passed in by reference

src/libstore/local-overlay-store.cc

@@ -243,19 +243,21 @@ void LocalOverlayStore::optimiseStore()
 }
 
 
-bool LocalOverlayStore::verifyAllValidPaths(RepairFlag repair, StorePathSet & validPaths)
+std::pair<bool, StorePathSet> LocalOverlayStore::verifyAllValidPaths(RepairFlag repair)
 {
     StorePathSet done;
-    bool errors = false;
 
     auto existsInStoreDir = [&](const StorePath & storePath) {
         return pathExists(realStoreDir.get() + "/" + storePath.to_string());
     };
 
+    bool errors = false;
+    StorePathSet validPaths;
+
     for (auto & i : queryAllValidPaths())
         verifyPath(i, existsInStoreDir, done, validPaths, repair, errors);
 
-    return errors;
+    return { errors, validPaths };
 }
 
 

src/libstore/local-overlay-store.hh

@@ -135,11 +135,11 @@ private:
      *
      * Note that this includes store objects that reside in either overlayfs layer;
      * just enumerating the contents of the upper layer would skip them.
-     * 
+     *
      * 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.
      */
-    bool verifyAllValidPaths(RepairFlag repair, StorePathSet & validPaths) override;
+    std::pair<bool, StorePathSet> verifyAllValidPaths(RepairFlag repair) override;
 
     /**
      * For lower-store paths, we used the lower store location. This avoids the

src/libstore/local-store.cc

@@ -1503,9 +1503,7 @@ bool LocalStore::verifyStore(bool checkContents, RepairFlag repair)
     auto fdGCLock = openGCLock();
     FdLock gcLock(fdGCLock.get(), ltRead, true, "waiting for the big garbage collector lock...");
 
-    StorePathSet validPaths;
+    auto [errors, validPaths] = verifyAllValidPaths(repair);
-
-    bool errors = verifyAllValidPaths(repair, validPaths);
 
     /* Optionally, check the content hashes (slow). */
     if (checkContents) {
@@ -1591,7 +1589,7 @@ bool LocalStore::verifyStore(bool checkContents, RepairFlag repair)
 }
 
 
-bool LocalStore::verifyAllValidPaths(RepairFlag repair, StorePathSet & validPaths)
+std::pair<bool, StorePathSet> LocalStore::verifyAllValidPaths(RepairFlag repair)
 {
     StorePathSet storePathsInStoreDir;
     /* Why aren't we using `queryAllValidPaths`? Because that would
@@ -1613,16 +1611,18 @@ bool LocalStore::verifyAllValidPaths(RepairFlag repair, StorePathSet & validPath
     printInfo("checking path existence...");
 
     StorePathSet done;
-    bool errors = false;
 
     auto existsInStoreDir = [&](const StorePath & storePath) {
         return storePathsInStoreDir.count(storePath);
     };
 
+    bool errors = false;
+    StorePathSet validPaths;
+
     for (auto & i : queryAllValidPaths())
         verifyPath(i, existsInStoreDir, done, validPaths, repair, errors);
 
-    return errors;
+    return { errors, validPaths };
 }
 
 

src/libstore/local-store.hh

@@ -265,7 +265,12 @@ public:
 
     bool verifyStore(bool checkContents, RepairFlag repair) override;
 
-    virtual bool verifyAllValidPaths(RepairFlag repair, StorePathSet & validPaths);
+    /**
+     * @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.
+     */
+    virtual std::pair<bool, StorePathSet> verifyAllValidPaths(RepairFlag repair);
 
     /**
      * Register the validity of a path, i.e., that `path` exists, that