refactor: Drop out parameter from get_level_1_files (#779)

diff --git a/src/CacheFile.hpp b/src/CacheFile.hpp
index 59c180bea9698d180f41dea9dec3e843a45c73d9..05410685a831138ad8cac61cef111ccbe0348dc7 100644 (file)
--- a/src/CacheFile.hpp
+++ b/src/CacheFile.hpp
@@ -21,7 +21,6 @@
 #include "system.hpp"
 
 #include "Stat.hpp"
-#include "exceptions.hpp"
 
 #include "third_party/nonstd/optional.hpp"
 
@@ -34,15 +33,12 @@ public:
 
   explicit CacheFile(const std::string& path);
 
-  CacheFile(const CacheFile&) = delete;
-  CacheFile& operator=(const CacheFile&) = delete;
-
   const Stat& lstat() const;
   const std::string& path() const;
   Type type() const;
 
 private:
-  const std::string m_path;
+  std::string m_path;
   mutable nonstd::optional<Stat> m_stat;
 };
 

diff --git a/src/Util.cpp b/src/Util.cpp
index 4be232edcba84713e99c6de143fed8367ed374df..71de5f9cdb57fc49c335deb5654e280e4eb54d98 100644 (file)
--- a/src/Util.cpp
+++ b/src/Util.cpp
@@ -654,13 +654,14 @@ get_extension(string_view path)
   }
 }
 
-void
+std::vector<CacheFile>
 get_level_1_files(const std::string& dir,
-                  const ProgressReceiver& progress_receiver,
-                  std::vector<std::shared_ptr<CacheFile>>& files)
+                  const ProgressReceiver& progress_receiver)
 {
+  std::vector<CacheFile> files;
+
   if (!Stat::stat(dir)) {
-    return;
+    return files;
   }
 
   size_t level_2_directories = 0;
@@ -672,7 +673,7 @@ get_level_1_files(const std::string& dir,
     }
 
     if (!is_dir) {
-      files.push_back(std::make_shared<CacheFile>(path));
+      files.emplace_back(path);
     } else if (path != dir
                && path.find('/', dir.size() + 1) == std::string::npos) {
       ++level_2_directories;
@@ -681,6 +682,7 @@ get_level_1_files(const std::string& dir,
   });
 
   progress_receiver(1.0);
+  return files;
 }
 
 std::string

diff --git a/src/Util.hpp b/src/Util.hpp
index 819950b5f3a00990750f8b5282279d2f10e510ff..7db8d95af189663ab358f45167ca3b01133571d0 100644 (file)
--- a/src/Util.hpp
+++ b/src/Util.hpp
@@ -208,10 +208,9 @@ nonstd::string_view get_extension(nonstd::string_view path);
 // Parameters:
 // - dir: The directory to traverse recursively.
 // - progress_receiver: Function that will be called for progress updates.
-// - files: Found files.
-void get_level_1_files(const std::string& dir,
-                       const ProgressReceiver& progress_receiver,
-                       std::vector<std::shared_ptr<CacheFile>>& files);
+std::vector<CacheFile>
+get_level_1_files(const std::string& dir,
+                  const ProgressReceiver& progress_receiver);
 
 // Return the current user's home directory, or throw `Fatal` if it can't
 // be determined.

diff --git a/src/cleanup.cpp b/src/cleanup.cpp
index a3ee774ee46b305b39294ee4b91381d7f312387a..5c76ebbebafe5d0f85dab2cc8612cb7c3ebd2b44 100644 (file)
--- a/src/cleanup.cpp
+++ b/src/cleanup.cpp
@@ -91,9 +91,8 @@ clean_up_dir(const std::string& subdir,
 {
   LOG("Cleaning up cache directory {}", subdir);
 
-  std::vector<std::shared_ptr<CacheFile>> files;
-  Util::get_level_1_files(
-    subdir, [&](double progress) { progress_receiver(progress / 3); }, files);
+  std::vector<CacheFile> files = Util::get_level_1_files(
+    subdir, [&](double progress) { progress_receiver(progress / 3); });
 
   uint64_t cache_size = 0;
   uint64_t files_in_cache = 0;
@@ -103,29 +102,27 @@ clean_up_dir(const std::string& subdir,
        ++i, progress_receiver(1.0 / 3 + 1.0 * i / files.size() / 3)) {
     const auto& file = files[i];
 
-    if (!file->lstat().is_regular()) {
+    if (!file.lstat().is_regular()) {
       // Not a file or missing file.
       continue;
     }
 
     // Delete any tmp files older than 1 hour right away.
-    if (file->lstat().mtime() + 3600 < current_time
-        && Util::base_name(file->path()).find(".tmp.") != std::string::npos) {
-      Util::unlink_tmp(file->path());
+    if (file.lstat().mtime() + 3600 < current_time
+        && Util::base_name(file.path()).find(".tmp.") != std::string::npos) {
+      Util::unlink_tmp(file.path());
       continue;
     }
 
-    cache_size += file->lstat().size_on_disk();
+    cache_size += file.lstat().size_on_disk();
     files_in_cache += 1;
   }
 
   // Sort according to modification time, oldest first.
-  std::sort(files.begin(),
-            files.end(),
-            [](const std::shared_ptr<CacheFile>& f1,
-               const std::shared_ptr<CacheFile>& f2) {
-              return f1->lstat().mtime() < f2->lstat().mtime();
-            });
+  std::sort(
+    files.begin(), files.end(), [](const CacheFile& f1, const CacheFile& f2) {
+      return f1.lstat().mtime() < f2.lstat().mtime();
+    });
 
   LOG("Before cleanup: {:.0f} KiB, {:.0f} files",
       static_cast<double>(cache_size) / 1024,
@@ -136,27 +133,26 @@ clean_up_dir(const std::string& subdir,
        ++i, progress_receiver(2.0 / 3 + 1.0 * i / files.size() / 3)) {
     const auto& file = files[i];
 
-    if (!file->lstat() || file->lstat().is_directory()) {
+    if (!file.lstat() || file.lstat().is_directory()) {
       continue;
     }
 
     if ((max_size == 0 || cache_size <= max_size)
         && (max_files == 0 || files_in_cache <= max_files)
         && (max_age == 0
-            || file->lstat().mtime()
+            || file.lstat().mtime()
                  > (current_time - static_cast<int64_t>(max_age)))) {
       break;
     }
 
-    if (Util::ends_with(file->path(), ".stderr")) {
+    if (Util::ends_with(file.path(), ".stderr")) {
       // In order to be nice to legacy ccache versions, make sure that the .o
       // file is deleted before .stderr, because if the ccache process gets
       // killed after deleting the .stderr but before deleting the .o, the
       // cached result will be inconsistent. (.stderr is the only file that is
       // optional for legacy ccache versions; any other file missing from the
       // cache will be detected.)
-      std::string o_file =
-        file->path().substr(0, file->path().size() - 6) + "o";
+      std::string o_file = file.path().substr(0, file.path().size() - 6) + "o";
 
       // Don't subtract this extra deletion from the cache size; that
       // bookkeeping will be done when the loop reaches the .o file. If the
@@ -168,7 +164,7 @@ clean_up_dir(const std::string& subdir,
     }
 
     delete_file(
-      file->path(), file->lstat().size_on_disk(), &cache_size, &files_in_cache);
+      file.path(), file.lstat().size_on_disk(), &cache_size, &files_in_cache);
     cleaned = true;
   }
 
@@ -208,12 +204,11 @@ wipe_dir(const std::string& subdir,
 {
   LOG("Clearing out cache directory {}", subdir);
 
-  std::vector<std::shared_ptr<CacheFile>> files;
-  Util::get_level_1_files(
-    subdir, [&](double progress) { progress_receiver(progress / 2); }, files);
+  const std::vector<CacheFile> files = Util::get_level_1_files(
+    subdir, [&](double progress) { progress_receiver(progress / 2); });
 
   for (size_t i = 0; i < files.size(); ++i) {
-    Util::unlink_safe(files[i]->path());
+    Util::unlink_safe(files[i].path());
     progress_receiver(0.5 + 0.5 * i / files.size());
   }
 

diff --git a/src/compress.cpp b/src/compress.cpp
index 42e0179cd89c0f352385d5b149908db4dfedab95..1164b795010d72968629c24d52d80d628e29f0fc 100644 (file)
--- a/src/compress.cpp
+++ b/src/compress.cpp
@@ -221,23 +221,20 @@ compress_stats(const Config& config,
     config.cache_dir(),
     [&](const std::string& subdir,
         const Util::ProgressReceiver& sub_progress_receiver) {
-      std::vector<std::shared_ptr<CacheFile>> files;
-      Util::get_level_1_files(
-        subdir,
-        [&](double progress) { sub_progress_receiver(progress / 2); },
-        files);
+      const std::vector<CacheFile> files = Util::get_level_1_files(
+        subdir, [&](double progress) { sub_progress_receiver(progress / 2); });
 
       for (size_t i = 0; i < files.size(); ++i) {
         const auto& cache_file = files[i];
-        on_disk_size += cache_file->lstat().size_on_disk();
+        on_disk_size += cache_file.lstat().size_on_disk();
 
         try {
-          auto file = open_file(cache_file->path(), "rb");
-          auto reader = create_reader(*cache_file, file.get());
-          compr_size += cache_file->lstat().size();
+          auto file = open_file(cache_file.path(), "rb");
+          auto reader = create_reader(cache_file, file.get());
+          compr_size += cache_file.lstat().size();
           content_size += reader->content_size();
         } catch (Error&) {
-          incompr_size += cache_file->lstat().size();
+          incompr_size += cache_file.lstat().size();
         }
 
         sub_progress_receiver(1.0 / 2 + 1.0 * i / files.size() / 2);
@@ -290,27 +287,26 @@ compress_recompress(Context& ctx,
     ctx.config.cache_dir(),
     [&](const std::string& subdir,
         const Util::ProgressReceiver& sub_progress_receiver) {
-      std::vector<std::shared_ptr<CacheFile>> files;
-      Util::get_level_1_files(
-        subdir,
-        [&](double progress) { sub_progress_receiver(0.1 * progress); },
-        files);
+      std::vector<CacheFile> files =
+        Util::get_level_1_files(subdir, [&](double progress) {
+          sub_progress_receiver(0.1 * progress);
+        });
 
       auto stats_file = subdir + "/stats";
 
       for (size_t i = 0; i < files.size(); ++i) {
         const auto& file = files[i];
 
-        if (file->type() != CacheFile::Type::unknown) {
+        if (file.type() != CacheFile::Type::unknown) {
           thread_pool.enqueue([&statistics, stats_file, file, level] {
             try {
-              recompress_file(statistics, stats_file, *file, level);
+              recompress_file(statistics, stats_file, file, level);
             } catch (Error&) {
               // Ignore for now.
             }
           });
         } else {
-          statistics.update(0, 0, 0, file->lstat().size());
+          statistics.update(0, 0, 0, file.lstat().size());
         }
 
         sub_progress_receiver(0.1 + 0.9 * i / files.size());

diff --git a/unittest/test_Util.cpp b/unittest/test_Util.cpp
index ce3c1115deb57ae85000f8adf9ad9d6fbc2b92a1..5fd52399d7a7e54fc7258273dbf039b4263d463d 100644 (file)
--- a/unittest/test_Util.cpp
+++ b/unittest/test_Util.cpp
@@ -376,43 +376,40 @@ TEST_CASE("Util::get_level_1_files")
   Util::write_file("0/1/file_c", "12");
   Util::write_file("0/f/c/file_d", "123");
 
-  std::vector<std::shared_ptr<CacheFile>> files;
   auto null_receiver = [](double) {};
 
   SUBCASE("nonexistent subdirectory")
   {
-    Util::get_level_1_files("2", null_receiver, files);
+    const auto files = Util::get_level_1_files("2", null_receiver);
     CHECK(files.empty());
   }
 
   SUBCASE("empty subdirectory")
   {
-    Util::get_level_1_files("e", null_receiver, files);
+    const auto files = Util::get_level_1_files("e", null_receiver);
     CHECK(files.empty());
   }
 
   SUBCASE("simple case")
   {
-    Util::get_level_1_files("0", null_receiver, files);
+    auto files = Util::get_level_1_files("0", null_receiver);
     REQUIRE(files.size() == 4);
 
     // Files within a level are in arbitrary order, sort them to be able to
     // verify them.
-    std::sort(files.begin(),
-              files.end(),
-              [](const std::shared_ptr<CacheFile>& f1,
-                 const std::shared_ptr<CacheFile>& f2) {
-                return f1->path() < f2->path();
-              });
-
-    CHECK(files[0]->path() == os_path("0/1/file_b"));
-    CHECK(files[0]->lstat().size() == 1);
-    CHECK(files[1]->path() == os_path("0/1/file_c"));
-    CHECK(files[1]->lstat().size() == 2);
-    CHECK(files[2]->path() == os_path("0/f/c/file_d"));
-    CHECK(files[2]->lstat().size() == 3);
-    CHECK(files[3]->path() == os_path("0/file_a"));
-    CHECK(files[3]->lstat().size() == 0);
+    std::sort(
+      files.begin(), files.end(), [](const CacheFile& f1, const CacheFile& f2) {
+        return f1.path() < f2.path();
+      });
+
+    CHECK(files[0].path() == os_path("0/1/file_b"));
+    CHECK(files[0].lstat().size() == 1);
+    CHECK(files[1].path() == os_path("0/1/file_c"));
+    CHECK(files[1].lstat().size() == 2);
+    CHECK(files[2].path() == os_path("0/f/c/file_d"));
+    CHECK(files[2].lstat().size() == 3);
+    CHECK(files[3].path() == os_path("0/file_a"));
+    CHECK(files[3].lstat().size() == 0);
   }
 }