From: Joel Rosdahl <joel@rosdahl.net>
Date: Sat, 5 Sep 2020 14:55:39 +0000 (+0200)
Subject: Allow for 64-bit values in statistics counters
X-Git-Tag: v4.0~127
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=7b59804385cb66d300a4e109bf0d3b566f43233a;p=thirdparty%2Fccache.git

Allow for 64-bit values in statistics counters

This is primaily done to avoid overflowing timestamp values in 2038.
---

diff --git a/src/Config.hpp b/src/Config.hpp
index 008d4ca08..cb009d199 100644
--- a/src/Config.hpp
+++ b/src/Config.hpp
@@ -62,7 +62,7 @@ public:
   bool keep_comments_cpp() const;
   double limit_multiple() const;
   const std::string& log_file() const;
-  uint32_t max_files() const;
+  uint64_t max_files() const;
   uint64_t max_size() const;
   const std::string& path() const;
   bool pch_external_checksum() const;
@@ -86,7 +86,7 @@ public:
   void set_ignore_options(const std::string& value);
   void set_inode_cache(bool value);
   void set_limit_multiple(double value);
-  void set_max_files(uint32_t value);
+  void set_max_files(uint64_t value);
   void set_max_size(uint64_t value);
   void set_run_second_cpp(bool value);
 
@@ -152,7 +152,7 @@ private:
   bool m_keep_comments_cpp = false;
   double m_limit_multiple = 0.8;
   std::string m_log_file = "";
-  uint32_t m_max_files = 0;
+  uint64_t m_max_files = 0;
   uint64_t m_max_size = 5ULL * 1000 * 1000 * 1000;
   std::string m_path = "";
   bool m_pch_external_checksum = false;
@@ -318,7 +318,7 @@ Config::log_file() const
   return m_log_file;
 }
 
-inline uint32_t
+inline uint64_t
 Config::max_files() const
 {
   return m_max_files;
@@ -460,7 +460,7 @@ Config::set_limit_multiple(double value)
 }
 
 inline void
-Config::set_max_files(uint32_t value)
+Config::set_max_files(uint64_t value)
 {
   m_max_files = value;
 }
diff --git a/src/Counters.cpp b/src/Counters.cpp
index 262b56270..a680fd773 100644
--- a/src/Counters.cpp
+++ b/src/Counters.cpp
@@ -27,7 +27,7 @@ Counters::Counters() : m_counters(static_cast<size_t>(Statistic::END))
 }
 
 // clang-format off
-unsigned&
+uint64_t&
 Counters::operator[](Statistic index)
 // clang-format on
 {
@@ -39,7 +39,7 @@ Counters::operator[](Statistic index)
 }
 
 // clang-format off
-unsigned
+uint64_t
 Counters::operator[](Statistic index) const
 // clang-format on
 {
diff --git a/src/Counters.hpp b/src/Counters.hpp
index b72d83a50..cdd6c1515 100644
--- a/src/Counters.hpp
+++ b/src/Counters.hpp
@@ -1,4 +1,4 @@
-// Copyright (C) 2010-2019 Joel Rosdahl and other contributors
+// Copyright (C) 2010-2020 Joel Rosdahl and other contributors
 //
 // See doc/AUTHORS.adoc for a complete list of contributors.
 //
@@ -24,15 +24,15 @@
 
 enum class Statistic;
 
-// A simple wrapper around a vector of integers
-// used for the statistics counters.
+// A simple wrapper around a vector of integers used for the statistics
+// counters.
 class Counters
 {
 public:
   Counters();
 
-  unsigned& operator[](Statistic index);
-  unsigned operator[](Statistic index) const;
+  uint64_t& operator[](Statistic index);
+  uint64_t operator[](Statistic index) const;
 
   size_t size() const;
 
@@ -40,5 +40,5 @@ public:
   bool all_zero() const;
 
 private:
-  std::vector<unsigned> m_counters;
+  std::vector<uint64_t> m_counters;
 };
diff --git a/src/Util.cpp b/src/Util.cpp
index 24d88c0e3..e646aa81c 100644
--- a/src/Util.cpp
+++ b/src/Util.cpp
@@ -938,7 +938,7 @@ normalize_absolute_path(string_view path)
 #endif
 }
 
-uint32_t
+uint64_t
 parse_duration(const std::string& duration)
 {
   unsigned factor = 0;
diff --git a/src/Util.hpp b/src/Util.hpp
index f83365683..9d3f42f02 100644
--- a/src/Util.hpp
+++ b/src/Util.hpp
@@ -312,9 +312,9 @@ bool matches_dir_prefix_or_file(nonstd::string_view dir_prefix_or_file,
 // On Windows: Backslashes are replaced with forward slashes.
 std::string normalize_absolute_path(nonstd::string_view path);
 
-// Parse `duration`, an unsigned 32-bit integer with d (days) or s (seconds)
-// suffix, into seconds. Throws `Error` on error.
-uint32_t parse_duration(const std::string& duration);
+// Parse `duration`, an unsigned integer with d (days) or s (seconds) suffix,
+// into seconds. Throws `Error` on error.
+uint64_t parse_duration(const std::string& duration);
 
 // Parse a string into a signed integer.
 //
diff --git a/src/ccache.cpp b/src/ccache.cpp
index 01bfcd5dd..8e84d1c63 100644
--- a/src/ccache.cpp
+++ b/src/ccache.cpp
@@ -1725,7 +1725,7 @@ create_initial_config_file(Config& config)
     return;
   }
 
-  unsigned max_files;
+  uint64_t max_files;
   uint64_t max_size;
   std::string stats_dir = fmt::format("{}/0", config.cache_dir());
   if (Stat::stat(stats_dir)) {
diff --git a/src/cleanup.cpp b/src/cleanup.cpp
index 0401d39fa..9301108d0 100644
--- a/src/cleanup.cpp
+++ b/src/cleanup.cpp
@@ -39,7 +39,7 @@ static void
 delete_file(const std::string& path,
             uint64_t size,
             uint64_t* cache_size,
-            uint32_t* files_in_cache)
+            uint64_t* files_in_cache)
 {
   bool deleted = Util::unlink_safe(path, Util::UnlinkLog::ignore_failure);
   if (!deleted && errno != ENOENT && errno != ESTALE) {
@@ -57,7 +57,7 @@ delete_file(const std::string& path,
 void
 clean_old(const Context& ctx,
           const Util::ProgressReceiver& progress_receiver,
-          time_t max_age)
+          uint64_t max_age)
 {
   Util::for_each_level_1_subdir(
     ctx.config.cache_dir(),
@@ -72,8 +72,8 @@ clean_old(const Context& ctx,
 void
 clean_up_dir(const std::string& subdir,
              uint64_t max_size,
-             uint32_t max_files,
-             time_t max_age,
+             uint64_t max_files,
+             uint64_t max_age,
              const Util::ProgressReceiver& progress_receiver)
 {
   log("Cleaning up cache directory {}", subdir);
@@ -83,7 +83,7 @@ clean_up_dir(const std::string& subdir,
     subdir, [&](double progress) { progress_receiver(progress / 3); }, files);
 
   uint64_t cache_size = 0;
-  uint32_t files_in_cache = 0;
+  uint64_t files_in_cache = 0;
   time_t current_time = time(nullptr);
 
   for (size_t i = 0; i < files.size();
@@ -129,7 +129,9 @@ clean_up_dir(const std::string& subdir,
 
     if ((max_size == 0 || cache_size <= max_size)
         && (max_files == 0 || files_in_cache <= max_files)
-        && (max_age == 0 || file->lstat().mtime() > (current_time - max_age))) {
+        && (max_age == 0
+            || file->lstat().mtime()
+                 > (current_time - static_cast<int64_t>(max_age)))) {
       break;
     }
 
diff --git a/src/cleanup.hpp b/src/cleanup.hpp
index 750bcd47c..053eb53e0 100644
--- a/src/cleanup.hpp
+++ b/src/cleanup.hpp
@@ -29,12 +29,12 @@ class Context;
 
 void clean_old(const Context& ctx,
                const Util::ProgressReceiver& progress_receiver,
-               time_t max_age);
+               uint64_t max_age);
 
 void clean_up_dir(const std::string& subdir,
                   uint64_t max_size,
-                  uint32_t max_files,
-                  time_t max_age,
+                  uint64_t max_files,
+                  uint64_t max_age,
                   const Util::ProgressReceiver& progress_receiver);
 
 void clean_up_all(const Config& config,
diff --git a/src/stats.cpp b/src/stats.cpp
index ae211e600..8b8a5a1ef 100644
--- a/src/stats.cpp
+++ b/src/stats.cpp
@@ -157,7 +157,7 @@ parse_stats(Counters& counters, const std::string& buf)
   const char* p = buf.c_str();
   while (true) {
     char* p2;
-    long val = strtol(p, &p2, 10);
+    unsigned long long val = std::strtoull(p, &p2, 10);
     if (p2 == p) {
       break;
     }
@@ -188,18 +188,18 @@ stats_write(const std::string& path, const Counters& counters)
 static double
 stats_hit_rate(const Counters& counters)
 {
-  unsigned direct = counters[Statistic::direct_cache_hit];
-  unsigned preprocessed = counters[Statistic::preprocessed_cache_hit];
-  unsigned hit = direct + preprocessed;
-  unsigned miss = counters[Statistic::cache_miss];
-  unsigned total = hit + miss;
+  uint64_t direct = counters[Statistic::direct_cache_hit];
+  uint64_t preprocessed = counters[Statistic::preprocessed_cache_hit];
+  uint64_t hit = direct + preprocessed;
+  uint64_t miss = counters[Statistic::cache_miss];
+  uint64_t total = hit + miss;
   return total > 0 ? (100.0 * hit) / total : 0.0;
 }
 
 static void
 stats_collect(const Config& config, Counters& counters, time_t* last_updated)
 {
-  unsigned zero_timestamp = 0;
+  uint64_t zero_timestamp = 0;
 
   *last_updated = 0;
 
@@ -319,7 +319,7 @@ stats_flush_to_file(const Config& config,
     double factor = config.limit_multiple() / 16;
     uint64_t max_size = round(config.max_size() * factor);
     uint32_t max_files = round(config.max_files() * factor);
-    uint32_t max_age = 0;
+    time_t max_age = 0;
     clean_up_dir(
       subdir, max_size, max_files, max_age, [](double /*progress*/) {});
   }
@@ -453,7 +453,7 @@ stats_zero(const Context& ctx)
 // Get the per-directory limits.
 void
 stats_get_obsolete_limits(const std::string& dir,
-                          unsigned* maxfiles,
+                          uint64_t* maxfiles,
                           uint64_t* maxsize)
 {
   assert(maxfiles);
@@ -463,13 +463,12 @@ stats_get_obsolete_limits(const std::string& dir,
   std::string sname = dir + "/stats";
   stats_read(sname, counters);
   *maxfiles = counters[Statistic::obsolete_max_files];
-  *maxsize =
-    static_cast<uint64_t>(counters[Statistic::obsolete_max_size]) * 1024;
+  *maxsize = counters[Statistic::obsolete_max_size] * 1024;
 }
 
 // Set the per-directory sizes.
 void
-stats_set_sizes(const std::string& dir, unsigned num_files, uint64_t total_size)
+stats_set_sizes(const std::string& dir, uint64_t num_files, uint64_t total_size)
 {
   Counters counters;
   std::string statsfile = dir + "/stats";
@@ -484,7 +483,7 @@ stats_set_sizes(const std::string& dir, unsigned num_files, uint64_t total_size)
 
 // Count directory cleanup run.
 void
-stats_add_cleanup(const std::string& dir, unsigned count)
+stats_add_cleanup(const std::string& dir, uint64_t count)
 {
   Counters counters;
   std::string statsfile = dir + "/stats";
diff --git a/src/stats.hpp b/src/stats.hpp
index 4561dace7..a7a13c117 100644
--- a/src/stats.hpp
+++ b/src/stats.hpp
@@ -77,11 +77,11 @@ void stats_print(const Config& config);
 
 void stats_update_size(Counters& counters, int64_t size, int files);
 void stats_get_obsolete_limits(const std::string& dir,
-                               unsigned* maxfiles,
+                               uint64_t* maxfiles,
                                uint64_t* maxsize);
 void stats_set_sizes(const std::string& dir,
-                     unsigned num_files,
+                     uint64_t num_files,
                      uint64_t total_size);
-void stats_add_cleanup(const std::string& dir, unsigned count);
+void stats_add_cleanup(const std::string& dir, uint64_t count);
 void stats_read(const std::string& path, Counters& counters);
 void stats_write(const std::string& path, const Counters& counters);
diff --git a/test/run b/test/run
index e0e9e3af3..aaf92d802 100755
--- a/test/run
+++ b/test/run
@@ -148,7 +148,7 @@ objdump_grep_cmd() {
 expect_stat() {
     local stat="$1"
     local expected_value="$2"
-    local value="$(echo $($CCACHE -s | fgrep "$stat" | cut -c34-))"
+    local value="$(echo $($CCACHE -s | fgrep "$stat" | cut -c33-))"
 
     if [ "$expected_value" != "$value" ]; then
         test_failed "Expected \"$stat\" to be $expected_value, actual $value"
diff --git a/test/suites/base.bash b/test/suites/base.bash
index 2090e3657..65cba561d 100644
--- a/test/suites/base.bash
+++ b/test/suites/base.bash
@@ -376,6 +376,18 @@ base_tests() {
     expect_contains "$stats_file" 101
     expect_newer_than "$stats_file" "$CCACHE_DIR/timestamp_reference"
 
+    # -------------------------------------------------------------------------
+    TEST "stats file with large counter values"
+
+    mkdir -p "$CCACHE_DIR/4/"
+    stats_file="$CCACHE_DIR/4/stats"
+
+    echo "0 0 0 0 1234567890123456789" >"$stats_file"
+
+    expect_stat 'cache miss' 1234567890123456789
+    $CCACHE_COMPILE -c test1.c
+    expect_stat 'cache miss' 1234567890123456790
+
     # -------------------------------------------------------------------------
     TEST "CCACHE_RECACHE"