C++-ify get_path_in_cache(), add ConfigTester

Then use ConfigTester to test get_path_in_cache()

diff --git a/src/Config.hpp b/src/Config.hpp
index bf8dbfaa48c86f8965c3b3db4356baa394326b27..ad3c35c931998c2e461e1d093ffce6c45adcfb30 100644 (file)
--- a/src/Config.hpp
+++ b/src/Config.hpp
@@ -151,6 +151,8 @@ private:
                 bool from_env_variable,
                 bool negate,
                 const std::string& origin);
+
+  friend struct ConfigTester;
 };
 
 inline const std::string&

diff --git a/src/Util.cpp b/src/Util.cpp
index 1d932fb0e25a292d62f375e5d1b515dfce304f4a..ea22ffb716e0e772b023e605d9d5a67fcef12ec0 100644 (file)
--- a/src/Util.cpp
+++ b/src/Util.cpp
@@ -18,6 +18,7 @@
 
 #include "Util.hpp"
 
+#include "Config.hpp"
 #include "FormatNonstdStringView.hpp"
 #include "ccache.hpp"
 
@@ -214,6 +215,29 @@ get_level_1_files(const std::string& dir,
   get_cache_files_internal(dir, 1, progress_receiver, files);
 }
 
+std::string
+get_path_in_cache(nonstd::string_view name, nonstd::string_view suffix)
+{
+  std::string path = g_config.cache_dir();
+
+  auto cache_dir_levels = g_config.cache_dir_levels();
+  path.reserve(path.size() + cache_dir_levels * 2 + 1 + name.length()
+               - cache_dir_levels + suffix.length());
+
+  unsigned level = 0;
+  for (; level < cache_dir_levels; ++level) {
+    path.push_back('/');
+    path.push_back(name.at(level));
+  }
+
+  path.push_back('/');
+  string_view name_remaining = name.substr(level);
+  path.append(name_remaining.data(), name_remaining.length());
+  path.append(suffix.data(), suffix.length());
+
+  return path;
+}
+
 int
 parse_int(const std::string& value)
 {

diff --git a/src/Util.hpp b/src/Util.hpp
index 8a46306b57496dde92be2a40a90c2d5f821be6d9..215a46c62656acb86fd3a8d1e7f6da0c314570a4 100644 (file)
--- a/src/Util.hpp
+++ b/src/Util.hpp
@@ -141,6 +141,18 @@ void get_level_1_files(const std::string& dir,
                        const ProgressReceiver& progress_receiver,
                        std::vector<std::shared_ptr<CacheFile>>& files);
 
+// Join the global cache directory, a '/', `name`, and `suffix` into a single
+// path and return it. Additionally N single-character, '/'-separated subpaths
+// are split from the beginning of `name` before joining them all, where N is
+// the number of globally configured cache dir levels.
+//
+// Throws if cache dir levels is greater than the length of `name`.
+//
+// E.g. "ABCDEF" and ".foo" will become "/ccache/A/B/CDEF.foo" when
+// the cache directory is "/ccache" and cache dir levels is 2.
+std::string get_path_in_cache(nonstd::string_view name,
+                              nonstd::string_view suffix);
+
 // Write bytes in big endian order from an integer value.
 //
 // Parameters:

diff --git a/src/ccache.cpp b/src/ccache.cpp
index d1f65808eeb431118e7e2083c2ef52b3fee551eb..901a366eea701026fba8a3fa9eec1a6b2eaffd97 100644 (file)
--- a/src/ccache.cpp
+++ b/src/ccache.cpp
@@ -1237,7 +1237,8 @@ update_cached_result_globals(struct digest* result_name)
   char result_name_string[DIGEST_STRING_BUFFER_SIZE];
   digest_as_string(result_name, result_name_string);
   cached_result_name = result_name;
-  cached_result_path = get_path_in_cache(result_name_string, ".result");
+  cached_result_path =
+    x_strdup(Util::get_path_in_cache(result_name_string, ".result").c_str());
   stats_file =
     format("%s/%c/stats", g_config.cache_dir().c_str(), result_name_string[0]);
 }
@@ -2131,7 +2132,8 @@ calculate_result_name(struct args* args, struct hash* hash, int direct_mode)
 
     char manifest_name_string[DIGEST_STRING_BUFFER_SIZE];
     hash_result_as_string(hash, manifest_name_string);
-    manifest_path = get_path_in_cache(manifest_name_string, ".manifest");
+    manifest_path = x_strdup(
+      Util::get_path_in_cache(manifest_name_string, ".manifest").c_str());
     manifest_stats_file = format(
       "%s/%c/stats", g_config.cache_dir().c_str(), manifest_name_string[0]);
 

diff --git a/src/ccache.hpp b/src/ccache.hpp
index fb3a78276f7227a2695d85d8f6dbf4485e6487ab..d8ea395d30ae78dc76157eef422664c55bfc4c2e 100644 (file)
--- a/src/ccache.hpp
+++ b/src/ccache.hpp
@@ -160,7 +160,6 @@ void cc_dump_debug_log_buffer(const char* path);
 void fatal(const char* format, ...) ATTR_FORMAT(printf, 1, 2) ATTR_NORETURN;
 void warn(const char* format, ...) ATTR_FORMAT(printf, 1, 2);
 
-char* get_path_in_cache(const char* name, const char* suffix);
 bool copy_fd(int fd_in, int fd_out);
 bool clone_file(const char* src, const char* dest, bool via_tmp_file);
 bool copy_file(const char* src, const char* dest, bool via_tmp_file);

diff --git a/src/legacy_util.cpp b/src/legacy_util.cpp
index 3e714423082f45aeb0bd728b908d2ec4c9e81386..5273debd119530201466730896099b3dee87d278 100644 (file)
--- a/src/legacy_util.cpp
+++ b/src/legacy_util.cpp
@@ -310,24 +310,6 @@ fatal(const char* format, ...)
   x_exit(1);
 }
 
-// Transform a name to a full path into the cache directory, creating needed
-// sublevels if needed. Caller frees.
-char*
-get_path_in_cache(const char* name, const char* suffix)
-{
-  char* path = x_strdup(g_config.cache_dir().c_str());
-  for (unsigned i = 0; i < g_config.cache_dir_levels(); ++i) {
-    char* p = format("%s/%c", path, name[i]);
-    free(path);
-    path = p;
-  }
-
-  char* result =
-    format("%s/%s%s", path, name + g_config.cache_dir_levels(), suffix);
-  free(path);
-  return result;
-}
-
 // Copy all data from fd_in to fd_out.
 bool
 copy_fd(int fd_in, int fd_out)

diff --git a/unittest/test_Config.cpp b/unittest/test_Config.cpp
index 2521421bd266fc52fe8779b3057800d60142d8db..0edb75075ae1741e7a5c0f2aac231d70497e4fbe 100644 (file)
--- a/unittest/test_Config.cpp
+++ b/unittest/test_Config.cpp
@@ -16,6 +16,8 @@
 // this program; if not, write to the Free Software Foundation, Inc., 51
 // Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
 
+#include "test_Config.hpp"
+
 #include "../src/Config.hpp"
 #include "../src/Error.hpp"
 #include "../src/Util.hpp"

diff --git a/unittest/test_Config.hpp b/unittest/test_Config.hpp
new file mode 100644 (file)
index 0000000..f5e4fbf
--- /dev/null
+++ b/unittest/test_Config.hpp
@@ -0,0 +1,11 @@
+#pragma once
+
+#include "../src/Config.hpp"
+
+// Helper to modify private Config variables without adding setters that would
+// only be used for testing.
+// This struct MUST NOT have any data members, only the required private member
+// variables should be exposed.
+struct ConfigTester : Config {
+  using Config::m_cache_dir_levels;
+};

diff --git a/unittest/test_Util.cpp b/unittest/test_Util.cpp
index c73872608f92f500b52567ff40914964e01a44f3..ce809aa5026a4a409942e4fe3bc6d21792a368c2 100644 (file)
--- a/unittest/test_Util.cpp
+++ b/unittest/test_Util.cpp
@@ -16,10 +16,13 @@
 // this program; if not, write to the Free Software Foundation, Inc., 51
 // Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
 
+#include "../src/Config.hpp"
 #include "../src/Util.hpp"
+#include "test_Config.hpp"
 
 #include "third_party/catch.hpp"
 
+using Catch::EndsWith;
 using Catch::Equals;
 
 TEST_CASE("Util::base_name")
@@ -246,6 +249,58 @@ TEST_CASE("Util::get_level_1_files")
   }
 }
 
+TEST_CASE("Util::get_path_in_cache")
+{
+  Config saved = g_config;
+
+  ConfigTester testconfig;
+  testconfig.set_cache_dir("/zz/ccache");
+
+  {
+    testconfig.m_cache_dir_levels = 0;
+    g_config = testconfig;
+    std::string path = Util::get_path_in_cache("ABCDEF", ".suffix");
+    CHECK(path == "/zz/ccache/ABCDEF.suffix");
+  }
+
+  {
+    testconfig.m_cache_dir_levels = 1;
+    g_config = testconfig;
+    std::string path = Util::get_path_in_cache("ABCDEF", ".suffix");
+    CHECK(path == "/zz/ccache/A/BCDEF.suffix");
+  }
+
+  {
+    testconfig.m_cache_dir_levels = 4;
+    g_config = testconfig;
+    std::string path = Util::get_path_in_cache("ABCDEF", ".suffix");
+    CHECK(path == "/zz/ccache/A/B/C/D/EF.suffix");
+  }
+
+  {
+    testconfig.m_cache_dir_levels = 0;
+    g_config = testconfig;
+    std::string path = Util::get_path_in_cache("", ".suffix");
+    CHECK(path == "/zz/ccache/.suffix");
+  }
+
+  {
+    testconfig.m_cache_dir_levels = 2;
+    g_config = testconfig;
+    std::string path = Util::get_path_in_cache("AB", ".suffix");
+    CHECK(path == "/zz/ccache/A/B/.suffix");
+  }
+
+  {
+    testconfig.m_cache_dir_levels = 3;
+    g_config = testconfig;
+    REQUIRE_THROWS_WITH(Util::get_path_in_cache("AB", ".suffix"),
+                        EndsWith("string_view::at()"));
+  }
+
+  g_config = saved;
+}
+
 TEST_CASE("Util::int_to_big_endian")
 {
   uint8_t bytes[8];