From a179db209ba347209682b19298f5c4d4b4d4a5c7 Mon Sep 17 00:00:00 2001 From: Joel Rosdahl Date: Tue, 7 Mar 2023 20:40:10 +0100 Subject: refactor: Improve InodeCache::get signature --- src/InodeCache.cpp | 28 +++++++++------------ src/InodeCache.hpp | 7 +++--- src/hashutil.cpp | 6 ++--- unittest/test_InodeCache.cpp | 59 +++++++++++++++++--------------------------- 4 files changed, 40 insertions(+), 60 deletions(-) diff --git a/src/InodeCache.cpp b/src/InodeCache.cpp index 7610417e..db75d58b 100644 --- a/src/InodeCache.cpp +++ b/src/InodeCache.cpp @@ -465,22 +465,19 @@ InodeCache::available(int fd) return fd_is_on_known_to_work_file_system(fd); } -bool -InodeCache::get(const std::string& path, - ContentType type, - Digest& file_digest, - HashSourceCodeResult* return_value) +std::optional +InodeCache::get(const std::string& path, ContentType type, Digest& file_digest) { if (!initialize()) { - return false; + return std::nullopt; } Digest key_digest; if (!hash_inode(path, type, key_digest)) { - return false; + return std::nullopt; } - bool found = false; + std::optional result; 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) { @@ -491,28 +488,25 @@ InodeCache::get(const std::string& path, } file_digest = bucket->entries[0].file_digest; - if (return_value) { - *return_value = - HashSourceCodeResult::from_bitmask(bucket->entries[0].return_value); - } - found = true; + result = + HashSourceCodeResult::from_bitmask(bucket->entries[0].return_value); break; } } }); if (!success) { - return false; + return std::nullopt; } if (m_config.debug()) { - LOG("Inode cache {}: {}", found ? "hit" : "miss", path); - if (found) { + LOG("Inode cache {}: {}", result ? "hit" : "miss", path); + if (result) { ++m_sr->hits; } else { ++m_sr->misses; } } - return found; + return result; } bool diff --git a/src/InodeCache.hpp b/src/InodeCache.hpp index eea3b49d..98276a34 100644 --- a/src/InodeCache.hpp +++ b/src/InodeCache.hpp @@ -25,6 +25,7 @@ #include #include +#include #include class Config; @@ -77,10 +78,8 @@ public: // // Returns true if saved values could be retrieved from the cache, false // otherwise. - bool get(const std::string& path, - ContentType type, - Digest& file_digest, - HashSourceCodeResult* return_value = nullptr); + std::optional + get(const std::string& path, ContentType type, Digest& file_digest); // 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 f241903d..1b019f9e 100644 --- a/src/hashutil.cpp +++ b/src/hashutil.cpp @@ -187,9 +187,9 @@ do_hash_file(const Context& ctx, check_temporal_macros ? InodeCache::ContentType::checked_for_temporal_macros : InodeCache::ContentType::raw; if (ctx.config.inode_cache()) { - HashSourceCodeResult result; - if (ctx.inode_cache.get(path, content_type, digest, &result)) { - return result; + const auto result = ctx.inode_cache.get(path, content_type, digest); + if (result) { + return *result; } } #else diff --git a/unittest/test_InodeCache.cpp b/unittest/test_InodeCache.cpp index e60a0733..34bd0034 100644 --- a/unittest/test_InodeCache.cpp +++ b/unittest/test_InodeCache.cpp @@ -77,16 +77,13 @@ TEST_CASE("Test disabled") InodeCache inode_cache(config, util::Duration(0)); Digest digest; - HashSourceCodeResult return_value; - CHECK(!inode_cache.get("a", - InodeCache::ContentType::checked_for_temporal_macros, - digest, - &return_value)); + CHECK(!inode_cache.get( + "a", InodeCache::ContentType::checked_for_temporal_macros, digest)); CHECK(!inode_cache.put("a", InodeCache::ContentType::checked_for_temporal_macros, digest, - return_value)); + HashSourceCodeResult())); CHECK(inode_cache.get_hits() == -1); CHECK(inode_cache.get_misses() == -1); CHECK(inode_cache.get_errors() == -1); @@ -103,12 +100,9 @@ TEST_CASE("Test lookup nonexistent") util::write_file("a", ""); Digest digest; - HashSourceCodeResult return_value; - CHECK(!inode_cache.get("a", - InodeCache::ContentType::checked_for_temporal_macros, - digest, - &return_value)); + CHECK(!inode_cache.get( + "a", InodeCache::ContentType::checked_for_temporal_macros, digest)); CHECK(inode_cache.get_hits() == 0); CHECK(inode_cache.get_misses() == 1); CHECK(inode_cache.get_errors() == 0); @@ -129,14 +123,11 @@ TEST_CASE("Test put and lookup") CHECK(put(inode_cache, "a", "a text", result)); Digest digest; - HashSourceCodeResult return_value; - - CHECK(inode_cache.get("a", - InodeCache::ContentType::checked_for_temporal_macros, - digest, - &return_value)); + auto return_value = inode_cache.get( + "a", InodeCache::ContentType::checked_for_temporal_macros, digest); + REQUIRE(return_value); CHECK(digest == Hash().hash("a text").digest()); - CHECK(return_value.to_bitmask() + CHECK(return_value->to_bitmask() == static_cast(HashSourceCode::found_date)); CHECK(inode_cache.get_hits() == 1); CHECK(inode_cache.get_misses() == 0); @@ -144,10 +135,8 @@ TEST_CASE("Test put and lookup") util::write_file("a", "something else"); - CHECK(!inode_cache.get("a", - InodeCache::ContentType::checked_for_temporal_macros, - digest, - &return_value)); + CHECK(!inode_cache.get( + "a", InodeCache::ContentType::checked_for_temporal_macros, digest)); CHECK(inode_cache.get_hits() == 1); CHECK(inode_cache.get_misses() == 1); CHECK(inode_cache.get_errors() == 0); @@ -157,12 +146,11 @@ TEST_CASE("Test put and lookup") "something else", HashSourceCodeResult(HashSourceCode::found_time))); - CHECK(inode_cache.get("a", - InodeCache::ContentType::checked_for_temporal_macros, - digest, - &return_value)); + return_value = inode_cache.get( + "a", InodeCache::ContentType::checked_for_temporal_macros, digest); + REQUIRE(return_value); CHECK(digest == Hash().hash("something else").digest()); - CHECK(return_value.to_bitmask() + CHECK(return_value->to_bitmask() == static_cast(HashSourceCode::found_time)); CHECK(inode_cache.get_hits() == 2); CHECK(inode_cache.get_misses() == 1); @@ -209,20 +197,19 @@ TEST_CASE("Test content type") HashSourceCodeResult(HashSourceCode::found_time))); Digest digest; - HashSourceCodeResult return_value; - CHECK( - inode_cache.get("a", InodeCache::ContentType::raw, digest, &return_value)); + auto return_value = + inode_cache.get("a", InodeCache::ContentType::raw, digest); + REQUIRE(return_value); CHECK(digest == binary_digest); - CHECK(return_value.to_bitmask() + CHECK(return_value->to_bitmask() == static_cast(HashSourceCode::found_date)); - CHECK(inode_cache.get("a", - InodeCache::ContentType::checked_for_temporal_macros, - digest, - &return_value)); + return_value = inode_cache.get( + "a", InodeCache::ContentType::checked_for_temporal_macros, digest); + REQUIRE(return_value); CHECK(digest == code_digest); - CHECK(return_value.to_bitmask() + CHECK(return_value->to_bitmask() == static_cast(HashSourceCode::found_time)); } -- cgit v1.2.1