From: Joel Rosdahl <joel@rosdahl.net>
Date: Sat, 5 Aug 2023 10:45:54 +0000 (+0200)
Subject: refactor: fs::path-ify util::traverse_directory
X-Git-Tag: v4.9~64
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=1b1306339d22d4993e2762e4a205014f029fb461;p=thirdparty%2Fccache.git

refactor: fs::path-ify util::traverse_directory
---

diff --git a/src/Context.cpp b/src/Context.cpp
index 9eab8a869..fbbfb2fde 100644
--- a/src/Context.cpp
+++ b/src/Context.cpp
@@ -25,6 +25,7 @@
 
 #include <Win32Util.hpp>
 #include <util/TimePoint.hpp>
+#include <util/file.hpp>
 #include <util/path.hpp>
 #include <util/process.hpp>
 #include <util/string.hpp>
diff --git a/src/core/common.cpp b/src/core/common.cpp
index b7560976e..72acaab61 100644
--- a/src/core/common.cpp
+++ b/src/core/common.cpp
@@ -24,6 +24,7 @@
 #include <fmtmacros.hpp>
 #include <util/Tokenizer.hpp>
 #include <util/expected.hpp>
+#include <util/file.hpp>
 #include <util/filesystem.hpp>
 #include <util/path.hpp>
 
diff --git a/src/core/mainoptions.cpp b/src/core/mainoptions.cpp
index 02341b47f..f12c4f6ec 100644
--- a/src/core/mainoptions.cpp
+++ b/src/core/mainoptions.cpp
@@ -302,23 +302,22 @@ trim_dir(const std::string& dir,
   std::vector<DirEntry> files;
   uint64_t initial_size = 0;
 
-  util::throw_on_error<core::Error>(util::traverse_directory(
-    dir, [&](const std::string& path, const bool is_dir) {
-      if (is_dir || util::TemporaryFile::is_tmp_file(path)) {
+  util::throw_on_error<core::Error>(
+    util::traverse_directory(dir, [&](const auto& de) {
+      if (de.is_directory() || util::TemporaryFile::is_tmp_file(de.path())) {
         return;
       }
-      DirEntry entry(path);
-      if (!entry) {
+      if (!de) {
         // Probably some race, ignore.
         return;
       }
-      initial_size += entry.size_on_disk();
-      const auto name = Util::base_name(path);
+      initial_size += de.size_on_disk();
+      const auto name = de.path().filename();
       if (name == "ccache.conf" || name == "stats") {
         throw Fatal(
-          FMT("this looks like a local cache directory (found {})", path));
+          FMT("this looks like a local cache directory (found {})", de.path()));
       }
-      files.emplace_back(std::move(entry));
+      files.emplace_back(de);
     }));
 
   std::sort(files.begin(), files.end(), [&](const auto& f1, const auto& f2) {
diff --git a/src/storage/local/LocalStorage.cpp b/src/storage/local/LocalStorage.cpp
index fea093cdb..8d54f81e1 100644
--- a/src/storage/local/LocalStorage.cpp
+++ b/src/storage/local/LocalStorage.cpp
@@ -1421,20 +1421,16 @@ LocalStorage::clean_internal_tempdir()
 
   LOG("Cleaning up {}", m_config.temporary_dir());
   core::ensure_dir_exists(m_config.temporary_dir());
-  util::traverse_directory(
-    m_config.temporary_dir(),
-    [now](const std::string& path, bool is_dir) {
-      if (is_dir) {
-        return;
-      }
-      DirEntry dir_entry(path, DirEntry::LogOnError::yes);
-      if (dir_entry && dir_entry.mtime() + k_tempdir_cleanup_interval < now) {
-        util::remove(path);
-      }
-    })
-    .or_else([&](const auto& error) {
-      LOG("Failed to clean up {}: {}", m_config.temporary_dir(), error);
-    });
+  util::traverse_directory(m_config.temporary_dir(), [now](const auto& de) {
+    if (de.is_directory()) {
+      return;
+    }
+    if (de && de.mtime() + k_tempdir_cleanup_interval < now) {
+      util::remove(de.path());
+    }
+  }).or_else([&](const auto& error) {
+    LOG("Failed to clean up {}: {}", m_config.temporary_dir(), error);
+  });
 
   util::write_file(cleaned_stamp, "");
 }
diff --git a/src/storage/local/util.cpp b/src/storage/local/util.cpp
index 871f8361e..9aacb94d6 100644
--- a/src/storage/local/util.cpp
+++ b/src/storage/local/util.cpp
@@ -73,15 +73,15 @@ get_cache_dir_files(const std::string& dir)
     return files;
   }
   util::throw_on_error<core::Error>(
-    util::traverse_directory(dir, [&](const std::string& path, bool is_dir) {
-      auto name = Util::base_name(path);
+    util::traverse_directory(dir, [&](const auto& de) {
+      auto name = de.path().filename().string();
       if (name == "CACHEDIR.TAG" || name == "stats"
-          || util::starts_with(std::string(name), ".nfs")) {
+          || util::starts_with(name, ".nfs")) {
         return;
       }
 
-      if (!is_dir) {
-        files.emplace_back(path);
+      if (!de.is_directory()) {
+        files.emplace_back(de);
       }
     }));
 
diff --git a/src/util/DirEntry.cpp b/src/util/DirEntry.cpp
index c6335dd15..ca686050f 100644
--- a/src/util/DirEntry.cpp
+++ b/src/util/DirEntry.cpp
@@ -22,6 +22,7 @@
 #include <Logging.hpp>
 #include <Win32Util.hpp>
 #include <fmtmacros.hpp>
+#include <util/file.hpp>
 #include <util/wincompat.hpp>
 
 #ifdef _WIN32
@@ -210,6 +211,16 @@ auto stat_func = ::stat;
 
 namespace util {
 
+uint64_t
+DirEntry::size_on_disk() const
+{
+#ifdef _WIN32
+  return util::likely_size_on_disk(size());
+#else
+  return do_stat().st_blocks * 512;
+#endif
+}
+
 const DirEntry::stat_t&
 DirEntry::do_stat() const
 {
diff --git a/src/util/DirEntry.hpp b/src/util/DirEntry.hpp
index 9b4b81c0b..8d5b5314f 100644
--- a/src/util/DirEntry.hpp
+++ b/src/util/DirEntry.hpp
@@ -19,7 +19,6 @@
 #pragma once
 
 #include <util/TimePoint.hpp>
-#include <util/file.hpp>
 #include <util/wincompat.hpp>
 
 #include <sys/stat.h>
@@ -226,16 +225,6 @@ DirEntry::size() const
   return do_stat().st_size;
 }
 
-inline uint64_t
-DirEntry::size_on_disk() const
-{
-#ifdef _WIN32
-  return util::likely_size_on_disk(size());
-#else
-  return do_stat().st_blocks * 512;
-#endif
-}
-
 inline bool
 DirEntry::is_directory() const
 {
diff --git a/src/util/file.cpp b/src/util/file.cpp
index 8099a1022..8882fa5df 100644
--- a/src/util/file.cpp
+++ b/src/util/file.cpp
@@ -514,10 +514,10 @@ traverse_directory(const fs::path& directory,
     if (is_dir) {
       traverse_directory(path, visitor);
     } else {
-      visitor(path.string(), false);
+      visitor(path);
     }
   }
-  visitor(directory.string(), true);
+  visitor(directory);
 
   return {};
 }
@@ -542,12 +542,12 @@ traverse_directory(const fs::path& directory,
   try {
     for (const auto& entry : std::filesystem::directory_iterator(directory)) {
       if (entry.is_directory()) {
-        traverse_directory(entry.path().string(), visitor);
+        traverse_directory(entry.path(), visitor);
       } else {
-        visitor(entry.path().string(), entry.is_directory());
+        visitor(entry.path());
       }
     }
-    visitor(directory.string(), true);
+    visitor(directory);
   } catch (const std::filesystem::filesystem_error& e) {
     return tl::unexpected(e.what());
   }
diff --git a/src/util/file.hpp b/src/util/file.hpp
index 7fd25003b..a7942d67e 100644
--- a/src/util/file.hpp
+++ b/src/util/file.hpp
@@ -19,6 +19,7 @@
 #pragma once
 
 #include <util/Bytes.hpp>
+#include <util/DirEntry.hpp>
 #include <util/TimePoint.hpp>
 #include <util/types.hpp>
 
@@ -43,8 +44,7 @@ enum class InPlace { yes, no };
 enum class LogFailure { yes, no };
 enum class ViaTmpFile { yes, no };
 
-using TraverseDirectoryVisitor =
-  std::function<void(const std::string& path, bool is_dir)>;
+using TraverseDirectoryVisitor = std::function<void(const DirEntry& dir_entry)>;
 
 // Copy a file from `src` to `dest`. If `via_tmp_file` is yes, `src` is copied
 // to a temporary file and then renamed to dest.
diff --git a/unittest/test_util_file.cpp b/unittest/test_util_file.cpp
index 4af0b0758..30936b6f3 100644
--- a/unittest/test_util_file.cpp
+++ b/unittest/test_util_file.cpp
@@ -217,8 +217,8 @@ TEST_CASE("util::traverse_directory")
   REQUIRE(fs::create_directory("empty-dir"));
 
   std::vector<std::string> visited;
-  auto visitor = [&visited](const std::string& path, bool is_dir) {
-    visited.push_back(FMT("[{}] {}", is_dir ? 'd' : 'f', path));
+  auto visitor = [&visited](const auto& de) {
+    visited.push_back(FMT("[{}] {}", de.is_directory() ? 'd' : 'f', de.path()));
   };
 
   SUBCASE("traverse nonexistent path")