refactor: Use memory buffers instead of streams for manifests

This is part of a larger refactoring effort with the goal of simplifying
how cache entries are read and processed.

diff --git a/src/ccache.cpp b/src/ccache.cpp
index 6b12739fe196f59bfa006b8e57a1fa132d893652..3587658174fccaf37c00f254501bc71ed1a93388 100644 (file)
--- a/src/ccache.cpp
+++ b/src/ccache.cpp
@@ -765,7 +765,10 @@ read_manifest(const std::string& path)
     try {
       core::FileReader file_reader(*file);
       core::CacheEntryReader reader(file_reader);
-      manifest.read(reader);
+      std::vector<uint8_t> payload;
+      payload.resize(reader.header().payload_size());
+      reader.read(payload.data(), payload.size());
+      manifest.read(payload);
       reader.finalize();
     } catch (const core::Error& e) {
       LOG("Error reading {}: {}", path, e.what());
@@ -790,7 +793,10 @@ save_manifest(const Config& config,
   header.set_entry_size_from_payload_size(manifest.serialized_size());
 
   core::CacheEntryWriter writer(file_writer, header);
-  manifest.write(writer);
+  std::vector<uint8_t> payload;
+  payload.reserve(header.payload_size());
+  manifest.serialize(payload);
+  writer.write(payload.data(), payload.size());
   writer.finalize();
   atomic_manifest_file.commit();
 }

diff --git a/src/core/CacheEntryDataReader.hpp b/src/core/CacheEntryDataReader.hpp
index 12466f791a6e11e3dc851e669217995f2ca414be..b889a0025ff4ea4b458febece6dc43d8d7739024 100644 (file)
--- a/src/core/CacheEntryDataReader.hpp
+++ b/src/core/CacheEntryDataReader.hpp
@@ -38,6 +38,10 @@ public:
   // Read `size` bytes. Throws `core::Error` on failure.
   nonstd::span<const uint8_t> read_bytes(size_t size);
 
+  // Read and copy `buffer.size()` bytes into `buffer`. Throws `core::Error` on
+  // failure.
+  void read_and_copy_bytes(nonstd::span<uint8_t> buffer);
+
   // Read a string of length `length`. Throws `core::Error` on failure.
   std::string_view read_str(size_t length);
 
@@ -69,6 +73,13 @@ CacheEntryDataReader::read_bytes(size_t size)
   return bytes;
 }
 
+inline void
+CacheEntryDataReader::read_and_copy_bytes(nonstd::span<uint8_t> buffer)
+{
+  const auto span = read_bytes(buffer.size());
+  memcpy(buffer.data(), span.data(), span.size());
+}
+
 inline std::string_view
 CacheEntryDataReader::read_str(const size_t length)
 {

diff --git a/src/core/Manifest.cpp b/src/core/Manifest.cpp
index 9bbe12964c403b086d3d53b526a896c2b810b056..b5ed4a2affbdb70d0ab3cc83314fb23517937a42 100644 (file)
--- a/src/core/Manifest.cpp
+++ b/src/core/Manifest.cpp
@@ -21,8 +21,8 @@
 #include <Context.hpp>
 #include <Hash.hpp>
 #include <Logging.hpp>
-#include <core/Reader.hpp>
-#include <core/Writer.hpp>
+#include <core/CacheEntryDataReader.hpp>
+#include <core/CacheEntryDataWriter.hpp>
 #include <core/exceptions.hpp>
 #include <fmtmacros.hpp>
 #include <hashutil.hpp>
@@ -79,19 +79,23 @@ namespace core {
 const uint8_t Manifest::k_format_version = 0;
 
 void
-Manifest::read(Reader& reader)
+Manifest::read(nonstd::span<const uint8_t> data)
 {
   clear();
 
+  core::CacheEntryDataReader reader(data);
+
   const auto format_version = reader.read_int<uint8_t>();
   if (format_version != k_format_version) {
-    throw core::Error(FMT(
-      "Unknown format version: {} != {}", format_version, k_format_version));
+    throw core::Error(FMT("Unknown manifest format version: {} != {}",
+                          format_version,
+                          k_format_version));
   }
 
   const auto file_count = reader.read_int<uint32_t>();
   for (uint32_t i = 0; i < file_count; ++i) {
-    m_files.push_back(reader.read_str(reader.read_int<uint16_t>()));
+    m_files.push_back(
+      std::string(reader.read_str(reader.read_int<uint16_t>())));
   }
 
   const auto file_info_count = reader.read_int<uint32_t>();
@@ -100,7 +104,7 @@ Manifest::read(Reader& reader)
     auto& entry = m_file_infos.back();
 
     reader.read_int(entry.index);
-    reader.read(entry.digest.bytes(), Digest::size());
+    reader.read_and_copy_bytes({entry.digest.bytes(), Digest::size()});
     reader.read_int(entry.fsize);
     reader.read_int(entry.mtime);
     reader.read_int(entry.ctime);
@@ -115,7 +119,7 @@ Manifest::read(Reader& reader)
     for (uint32_t j = 0; j < file_info_index_count; ++j) {
       entry.file_info_indexes.push_back(reader.read_int<uint32_t>());
     }
-    reader.read(entry.key.bytes(), Digest::size());
+    reader.read_and_copy_bytes({entry.key.bytes(), Digest::size()});
   }
 }
 
@@ -197,7 +201,7 @@ Manifest::add_result(
   }
 }
 
-size_t
+uint32_t
 Manifest::serialized_size() const
 {
   uint64_t size = 0;
@@ -216,12 +220,21 @@ Manifest::serialized_size() const
     size += Digest::size();
   }
 
+  // In order to support 32-bit ccache builds, restrict size to uint32_t for
+  // now. This restriction can be lifted when we drop 32-bit support.
+  const auto max = std::numeric_limits<uint32_t>::max();
+  if (size > max) {
+    throw core::Error(
+      FMT("Serialized manifest too large ({} > {})", size, max));
+  }
   return size;
 }
 
 void
-Manifest::write(Writer& writer) const
+Manifest::serialize(std::vector<uint8_t>& output) const
 {
+  core::CacheEntryDataWriter writer(output);
+
   writer.write_int(k_format_version);
   writer.write_int<uint32_t>(m_files.size());
   for (const auto& file : m_files) {
@@ -232,7 +245,7 @@ Manifest::write(Writer& writer) const
   writer.write_int<uint32_t>(m_file_infos.size());
   for (const auto& file_info : m_file_infos) {
     writer.write_int<uint32_t>(file_info.index);
-    writer.write(file_info.digest.bytes(), Digest::size());
+    writer.write_bytes({file_info.digest.bytes(), Digest::size()});
     writer.write_int(file_info.fsize);
     writer.write_int(file_info.mtime);
     writer.write_int(file_info.ctime);
@@ -244,7 +257,7 @@ Manifest::write(Writer& writer) const
     for (auto index : result.file_info_indexes) {
       writer.write_int(index);
     }
-    writer.write(result.key.bytes(), Digest::size());
+    writer.write_bytes({result.key.bytes(), Digest::size()});
   }
 }
 
@@ -410,7 +423,7 @@ Manifest::result_matches(
 }
 
 void
-Manifest::dump(FILE* const stream) const
+Manifest::inspect(FILE* const stream) const
 {
   PRINT(stream, "Manifest format version: {}\n", k_format_version);
 

diff --git a/src/core/Manifest.hpp b/src/core/Manifest.hpp
index ace63be8339009a692ab9d62ca7783dd1e36e9dc..6f6d853acf421f6751b1a0e0d8a8859853c71d61 100644 (file)
--- a/src/core/Manifest.hpp
+++ b/src/core/Manifest.hpp
@@ -20,6 +20,8 @@
 
 #include <Digest.hpp>
 
+#include <third_party/nonstd/span.hpp>
+
 #include <cstdint>
 #include <optional>
 #include <string>
@@ -30,9 +32,6 @@ class Context;
 
 namespace core {
 
-class Reader;
-class Writer;
-
 class Manifest
 {
 public:
@@ -40,17 +39,19 @@ public:
 
   Manifest() = default;
 
-  void read(Reader& reader);
+  void read(nonstd::span<const uint8_t> data);
+
   std::optional<Digest> look_up_result_digest(const Context& ctx) const;
 
   bool add_result(const Digest& result_key,
                   const std::unordered_map<std::string, Digest>& included_files,
                   time_t time_of_compilation,
                   bool save_timestamp);
-  size_t serialized_size() const;
-  void write(Writer& writer) const;
 
-  void dump(FILE* stream) const;
+  uint32_t serialized_size() const;
+  void serialize(std::vector<uint8_t>& output) const;
+
+  void inspect(FILE* stream) const;
 
 private:
   struct FileStats
@@ -86,6 +87,7 @@ private:
   std::vector<ResultEntry> m_results;
 
   void clear();
+
   uint32_t get_file_info_index(
     const std::string& path,
     const Digest& digest,
@@ -93,6 +95,7 @@ private:
     const std::unordered_map<FileInfo, uint32_t>& mf_file_infos,
     time_t time_of_compilation,
     bool save_timestamp);
+
   bool
   result_matches(const Context& ctx,
                  const ResultEntry& result,

diff --git a/src/core/mainoptions.cpp b/src/core/mainoptions.cpp
index baa842b29a8cb6ead7a527dcc64e8d493fd2206c..28b7fc8c82044552367b20a6f010c102efd7d2d6 100644 (file)
--- a/src/core/mainoptions.cpp
+++ b/src/core/mainoptions.cpp
@@ -171,17 +171,18 @@ inspect_path(const std::string& path)
   const auto& header = cache_entry_reader.header();
   header.inspect(stdout);
 
+  std::vector<uint8_t> data;
+  data.resize(cache_entry_reader.header().payload_size());
+  cache_entry_reader.read(data.data(), data.size());
+
   switch (header.entry_type) {
   case core::CacheEntryType::manifest: {
     core::Manifest manifest;
-    manifest.read(cache_entry_reader);
-    manifest.dump(stdout);
+    manifest.read(data);
+    manifest.inspect(stdout);
     break;
   }
   case core::CacheEntryType::result:
-    std::vector<uint8_t> data;
-    data.resize(cache_entry_reader.header().payload_size());
-    cache_entry_reader.read(data.data(), data.size());
     Result::Deserializer result_deserializer(data);
     ResultInspector result_inspector(stdout);
     result_deserializer.visit(result_inspector);