From: Joel Rosdahl <joel@rosdahl.net>
Date: Fri, 7 Jul 2023 14:01:59 +0000 (+0200)
Subject: refactor: Improve InodeCache::get signature
X-Git-Tag: v4.9~145
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=cd4e26dfade620070401d6fefd7de91f616443d5;p=thirdparty%2Fccache.git

refactor: Improve InodeCache::get signature
---

diff --git a/src/InodeCache.cpp b/src/InodeCache.cpp
index c4ceecd05..4f17ebba3 100644
--- a/src/InodeCache.cpp
+++ b/src/InodeCache.cpp
@@ -466,10 +466,8 @@ InodeCache::available(int fd)
   return fd_is_on_known_to_work_file_system(fd);
 }
 
-std::optional<HashSourceCodeResult>
-InodeCache::get(const std::string& path,
-                ContentType type,
-                Hash::Digest& file_digest)
+std::optional<std::pair<HashSourceCodeResult, Hash::Digest>>
+InodeCache::get(const std::string& path, ContentType type)
 {
   if (!initialize()) {
     return std::nullopt;
@@ -481,6 +479,7 @@ InodeCache::get(const std::string& path,
   }
 
   std::optional<HashSourceCodeResult> result;
+  Hash::Digest file_digest;
   const bool success = with_bucket(key_digest, [&](const auto bucket) {
     for (uint32_t i = 0; i < k_num_entries; ++i) {
       if (bucket->entries[i].key_digest == key_digest) {
@@ -509,7 +508,11 @@ InodeCache::get(const std::string& path,
       ++m_sr->misses;
     }
   }
-  return result;
+  if (result) {
+    return std::make_pair(*result, file_digest);
+  } else {
+    return std::nullopt;
+  }
 }
 
 bool
diff --git a/src/InodeCache.hpp b/src/InodeCache.hpp
index 8d54a1fbe..94dd00895 100644
--- a/src/InodeCache.hpp
+++ b/src/InodeCache.hpp
@@ -28,6 +28,7 @@
 #include <functional>
 #include <optional>
 #include <string>
+#include <utility>
 
 class Config;
 class Context;
@@ -75,11 +76,8 @@ public:
 
   // Get saved hash digest and return value from a previous call to
   // do_hash_file() in hashutil.cpp.
-  //
-  // Returns true if saved values could be retrieved from the cache, false
-  // otherwise.
-  std::optional<HashSourceCodeResult>
-  get(const std::string& path, ContentType type, Hash::Digest& file_digest);
+  std::optional<std::pair<HashSourceCodeResult, Hash::Digest>>
+  get(const std::string& path, ContentType type);
 
   // Put hash digest and return value from a successful call to do_hash_file()
   // in hashutil.cpp.
diff --git a/src/hashutil.cpp b/src/hashutil.cpp
index 921238ae0..a06c787f9 100644
--- a/src/hashutil.cpp
+++ b/src/hashutil.cpp
@@ -186,9 +186,10 @@ do_hash_file(const Context& ctx,
     check_temporal_macros ? InodeCache::ContentType::checked_for_temporal_macros
                           : InodeCache::ContentType::raw;
   if (ctx.config.inode_cache()) {
-    const auto result = ctx.inode_cache.get(path, content_type, digest);
+    const auto result = ctx.inode_cache.get(path, content_type);
     if (result) {
-      return *result;
+      digest = result->second;
+      return result->first;
     }
   }
 #else
diff --git a/unittest/test_InodeCache.cpp b/unittest/test_InodeCache.cpp
index 5e331f639..523a79304 100644
--- a/unittest/test_InodeCache.cpp
+++ b/unittest/test_InodeCache.cpp
@@ -76,13 +76,11 @@ TEST_CASE("Test disabled")
   config.set_inode_cache(false);
   InodeCache inode_cache(config, util::Duration(0));
 
-  Hash::Digest digest;
-
-  CHECK(!inode_cache.get(
-    "a", InodeCache::ContentType::checked_for_temporal_macros, digest));
+  CHECK(!inode_cache.get("a",
+                         InodeCache::ContentType::checked_for_temporal_macros));
   CHECK(!inode_cache.put("a",
                          InodeCache::ContentType::checked_for_temporal_macros,
-                         digest,
+                         Hash::Digest(),
                          HashSourceCodeResult()));
   CHECK(inode_cache.get_hits() == -1);
   CHECK(inode_cache.get_misses() == -1);
@@ -99,10 +97,8 @@ TEST_CASE("Test lookup nonexistent")
   InodeCache inode_cache(config, util::Duration(0));
   util::write_file("a", "");
 
-  Hash::Digest digest;
-
-  CHECK(!inode_cache.get(
-    "a", InodeCache::ContentType::checked_for_temporal_macros, digest));
+  CHECK(!inode_cache.get("a",
+                         InodeCache::ContentType::checked_for_temporal_macros));
   CHECK(inode_cache.get_hits() == 0);
   CHECK(inode_cache.get_misses() == 1);
   CHECK(inode_cache.get_errors() == 0);
@@ -122,21 +118,20 @@ TEST_CASE("Test put and lookup")
   result.insert(HashSourceCode::found_date);
   CHECK(put(inode_cache, "a", "a text", result));
 
-  Hash::Digest digest;
-  auto return_value = inode_cache.get(
-    "a", InodeCache::ContentType::checked_for_temporal_macros, digest);
+  auto return_value =
+    inode_cache.get("a", InodeCache::ContentType::checked_for_temporal_macros);
   REQUIRE(return_value);
-  CHECK(digest == Hash().hash("a text").digest());
-  CHECK(return_value->to_bitmask()
+  CHECK(return_value->first.to_bitmask()
         == static_cast<int>(HashSourceCode::found_date));
+  CHECK(return_value->second == Hash().hash("a text").digest());
   CHECK(inode_cache.get_hits() == 1);
   CHECK(inode_cache.get_misses() == 0);
   CHECK(inode_cache.get_errors() == 0);
 
   util::write_file("a", "something else");
 
-  CHECK(!inode_cache.get(
-    "a", InodeCache::ContentType::checked_for_temporal_macros, digest));
+  CHECK(!inode_cache.get("a",
+                         InodeCache::ContentType::checked_for_temporal_macros));
   CHECK(inode_cache.get_hits() == 1);
   CHECK(inode_cache.get_misses() == 1);
   CHECK(inode_cache.get_errors() == 0);
@@ -146,12 +141,12 @@ TEST_CASE("Test put and lookup")
             "something else",
             HashSourceCodeResult(HashSourceCode::found_time)));
 
-  return_value = inode_cache.get(
-    "a", InodeCache::ContentType::checked_for_temporal_macros, digest);
+  return_value =
+    inode_cache.get("a", InodeCache::ContentType::checked_for_temporal_macros);
   REQUIRE(return_value);
-  CHECK(digest == Hash().hash("something else").digest());
-  CHECK(return_value->to_bitmask()
+  CHECK(return_value->first.to_bitmask()
         == static_cast<int>(HashSourceCode::found_time));
+  CHECK(return_value->second == Hash().hash("something else").digest());
   CHECK(inode_cache.get_hits() == 2);
   CHECK(inode_cache.get_misses() == 1);
   CHECK(inode_cache.get_errors() == 0);
@@ -166,9 +161,7 @@ TEST_CASE("Drop file")
 
   InodeCache inode_cache(config, util::Duration(0));
 
-  Hash::Digest digest;
-
-  inode_cache.get("a", InodeCache::ContentType::raw, digest);
+  inode_cache.get("a", InodeCache::ContentType::raw);
   CHECK(Stat::stat(inode_cache.get_file()));
   CHECK(inode_cache.drop());
   CHECK(!Stat::stat(inode_cache.get_file()));
@@ -196,21 +189,18 @@ TEST_CASE("Test content type")
                         code_digest,
                         HashSourceCodeResult(HashSourceCode::found_time)));
 
-  Hash::Digest digest;
-
-  auto return_value =
-    inode_cache.get("a", InodeCache::ContentType::raw, digest);
+  auto return_value = inode_cache.get("a", InodeCache::ContentType::raw);
   REQUIRE(return_value);
-  CHECK(digest == binary_digest);
-  CHECK(return_value->to_bitmask()
+  CHECK(return_value->first.to_bitmask()
         == static_cast<int>(HashSourceCode::found_date));
+  CHECK(return_value->second == binary_digest);
 
-  return_value = inode_cache.get(
-    "a", InodeCache::ContentType::checked_for_temporal_macros, digest);
+  return_value =
+    inode_cache.get("a", InodeCache::ContentType::checked_for_temporal_macros);
   REQUIRE(return_value);
-  CHECK(digest == code_digest);
-  CHECK(return_value->to_bitmask()
+  CHECK(return_value->first.to_bitmask()
         == static_cast<int>(HashSourceCode::found_time));
+  CHECK(return_value->second == code_digest);
 }
 
 TEST_SUITE_END();