refactor: factor out fs_section_checker

cmake/libdwarfs.cmake

@@ -51,6 +51,7 @@ add_library(
   src/internal/features.cpp
   src/internal/file_status_conv.cpp
   src/internal/fs_section.cpp
+  src/internal/fs_section_checker.cpp
   src/internal/glob_to_regex.cpp
   src/internal/string_table.cpp
   src/internal/unicode_case_folding.cpp

include/dwarfs/internal/fs_section.h

@@ -32,7 +32,6 @@
 #include <optional>
 #include <span>
 #include <string>
-#include <vector>
 
 #include <dwarfs/fstypes.h>
 
@@ -57,12 +56,19 @@ class fs_section {
   std::string name() const { return impl_->name(); }
   std::string description() const { return impl_->description(); }
   bool check_fast(mmif const& mm) const { return impl_->check_fast(mm); }
-  bool check(mmif const& mm) const { return impl_->check(mm); }
+
-  bool verify(mmif const& mm) const { return impl_->verify(mm); }
   std::span<uint8_t const> data(mmif const& mm) const {
     return impl_->data(mm);
   }
 
+  std::optional<std::span<uint8_t const>> checksum_span(mmif const& mm) const {
+    return impl_->checksum_span(mm);
+  }
+
+  std::optional<std::span<uint8_t const>> integrity_span(mmif const& mm) const {
+    return impl_->integrity_span(mm);
+  }
+
   size_t end() const { return start() + length(); }
 
   std::optional<uint32_t> section_number() const {
@@ -73,7 +79,7 @@ class fs_section {
     return impl_->xxh3_64_value();
   }
 
-  std::optional<std::vector<uint8_t>> sha2_512_256_value() const {
+  std::optional<std::span<uint8_t const>> sha2_512_256_value() const {
     return impl_->sha2_512_256_value();
   }
 
@@ -90,12 +96,15 @@ class fs_section {
     virtual std::string name() const = 0;
     virtual std::string description() const = 0;
     virtual bool check_fast(mmif const& mm) const = 0;
-    virtual bool check(mmif const& mm) const = 0;
-    virtual bool verify(mmif const& mm) const = 0;
     virtual std::span<uint8_t const> data(mmif const& mm) const = 0;
+    virtual std::optional<std::span<uint8_t const>>
+    checksum_span(mmif const& mm) const = 0;
+    virtual std::optional<std::span<uint8_t const>>
+    integrity_span(mmif const& mm) const = 0;
     virtual std::optional<uint32_t> section_number() const = 0;
     virtual std::optional<uint64_t> xxh3_64_value() const = 0;
-    virtual std::optional<std::vector<uint8_t>> sha2_512_256_value() const = 0;
+    virtual std::optional<std::span<uint8_t const>>
+    sha2_512_256_value() const = 0;
   };
 
  private:

include/dwarfs/internal/fs_section_checker.h

@@ -0,0 +1,48 @@
+/* vim:set ts=2 sw=2 sts=2 et: */
+/**
+ * \author     Marcus Holland-Moritz (github@mhxnet.de)
+ * \copyright  Copyright (c) Marcus Holland-Moritz
+ *
+ * This file is part of dwarfs.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the “Software”), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED “AS IS”, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ *
+ * SPDX-License-Identifier: MIT
+ */
+
+#pragma once
+
+#include <dwarfs/internal/fs_section.h>
+
+namespace dwarfs::internal {
+
+class fs_section_checker {
+ public:
+  fs_section_checker(mmif const& mm)
+      : mm_{mm} {}
+
+  bool check(fs_section const& section) const;
+  bool check(fs_section::impl const& section) const;
+  bool verify(fs_section const& section) const;
+
+ private:
+  mmif const& mm_;
+};
+
+} // namespace dwarfs::internal

src/internal/fs_section.cpp

@@ -37,6 +37,7 @@
 #include <dwarfs/mmif.h>
 
 #include <dwarfs/internal/fs_section.h>
+#include <dwarfs/internal/fs_section_checker.h>
 
 namespace dwarfs::internal {
 
@@ -108,13 +109,21 @@ class fs_section_v1 final : public fs_section::impl {
   }
 
   bool check_fast(mmif const&) const override { return true; }
-  bool check(mmif const&) const override { return true; }
-  bool verify(mmif const&) const override { return true; }
 
   std::span<uint8_t const> data(mmif const& mm) const override {
     return mm.span(start_, hdr_.length);
   }
 
+  std::optional<std::span<uint8_t const>>
+  checksum_span(mmif const&) const override {
+    return std::nullopt;
+  }
+
+  std::optional<std::span<uint8_t const>>
+  integrity_span(mmif const&) const override {
+    return std::nullopt;
+  }
+
   std::optional<uint32_t> section_number() const override {
     return std::nullopt;
   }
@@ -123,7 +132,7 @@ class fs_section_v1 final : public fs_section::impl {
     return std::nullopt;
   }
 
-  std::optional<std::vector<uint8_t>> sha2_512_256_value() const override {
+  std::optional<std::span<uint8_t const>> sha2_512_256_value() const override {
     return std::nullopt;
   }
 
@@ -181,24 +190,11 @@ class fs_section_v2 final : public fs_section::impl {
       return state == check_state::passed;
     }
 
-    return check(mm);
+    auto ok = fs_section_checker(mm).check(*this);
-  }
 
-  bool check(mmif const& mm) const override {
+    if (auto state = check_state_.load(); state == check_state::failed) {
-    if (check_state_.load() == check_state::failed) {
+      ok = false;
-      return false;
+    } else {
-    }
-
-    static constexpr auto kHdrCsLen =
-        sizeof(section_header_v2) - offsetof(section_header_v2, number);
-
-    auto ok = checksum::verify(
-        checksum::xxh3_64, mm.as<void>(start_ - kHdrCsLen),
-        hdr_.length + kHdrCsLen, &hdr_.xxh3_64, sizeof(hdr_.xxh3_64));
-
-    auto state = check_state_.load();
-
-    if (state != check_state::failed) {
       auto desired = ok ? check_state::passed : check_state::failed;
       check_state_.compare_exchange_strong(state, desired);
     }
@@ -206,19 +202,26 @@ class fs_section_v2 final : public fs_section::impl {
     return ok;
   }
 
-  bool verify(mmif const& mm) const override {
-    auto hdr_sha_len =
-        sizeof(section_header_v2) - offsetof(section_header_v2, xxh3_64);
-    return checksum::verify(checksum::sha2_512_256,
-                            mm.as<void>(start_ - hdr_sha_len),
-                            hdr_.length + hdr_sha_len, hdr_.sha2_512_256.data(),
-                            hdr_.sha2_512_256.size());
-  }
-
   std::span<uint8_t const> data(mmif const& mm) const override {
     return mm.span(start_, hdr_.length);
   }
 
+  std::optional<std::span<uint8_t const>>
+  checksum_span(mmif const& mm) const override {
+    static constexpr auto kHdrCsLen =
+        sizeof(section_header_v2) - offsetof(section_header_v2, number);
+
+    return mm.span(start_ - kHdrCsLen, hdr_.length + kHdrCsLen);
+  }
+
+  std::optional<std::span<uint8_t const>>
+  integrity_span(mmif const& mm) const override {
+    static constexpr auto kHdrShaLen =
+        sizeof(section_header_v2) - offsetof(section_header_v2, xxh3_64);
+
+    return mm.span(start_ - kHdrShaLen, hdr_.length + kHdrShaLen);
+  }
+
   std::optional<uint32_t> section_number() const override {
     return hdr_.number;
   }
@@ -227,9 +230,8 @@ class fs_section_v2 final : public fs_section::impl {
     return hdr_.xxh3_64;
   }
 
-  std::optional<std::vector<uint8_t>> sha2_512_256_value() const override {
+  std::optional<std::span<uint8_t const>> sha2_512_256_value() const override {
-    return std::vector<uint8_t>(hdr_.sha2_512_256.begin(),
+    return std::span{hdr_.sha2_512_256.data(), hdr_.sha2_512_256.size()};
-                                hdr_.sha2_512_256.end());
   }
 
  private:
@@ -270,14 +272,20 @@ class fs_section_v2_lazy final : public fs_section::impl {
     return section().check_fast(mm);
   }
 
-  bool check(mmif const& mm) const override { return section().check(mm); }
-
-  bool verify(mmif const& mm) const override { return section().verify(mm); }
-
   std::span<uint8_t const> data(mmif const& mm) const override {
     return section().data(mm);
   }
 
+  std::optional<std::span<uint8_t const>>
+  checksum_span(mmif const& mm) const override {
+    return section().checksum_span(mm);
+  }
+
+  std::optional<std::span<uint8_t const>>
+  integrity_span(mmif const& mm) const override {
+    return section().integrity_span(mm);
+  }
+
   std::optional<uint32_t> section_number() const override {
     return section().section_number();
   }
@@ -286,7 +294,7 @@ class fs_section_v2_lazy final : public fs_section::impl {
     return section().xxh3_64_value();
   }
 
-  std::optional<std::vector<uint8_t>> sha2_512_256_value() const override {
+  std::optional<std::span<uint8_t const>> sha2_512_256_value() const override {
     return section().sha2_512_256_value();
   }
 

src/internal/fs_section_checker.cpp

@@ -0,0 +1,71 @@
+/* vim:set ts=2 sw=2 sts=2 et: */
+/**
+ * \author     Marcus Holland-Moritz (github@mhxnet.de)
+ * \copyright  Copyright (c) Marcus Holland-Moritz
+ *
+ * This file is part of dwarfs.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the “Software”), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED “AS IS”, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ *
+ * SPDX-License-Identifier: MIT
+ */
+
+#include <dwarfs/checksum.h>
+
+#include <dwarfs/internal/fs_section.h>
+#include <dwarfs/internal/fs_section_checker.h>
+
+namespace dwarfs::internal {
+
+namespace {
+
+template <typename T>
+bool check_impl(T const& section, mmif const& mm) {
+  if (auto const cs_val = section.xxh3_64_value()) {
+    if (auto const cs_span = section.checksum_span(mm)) {
+      return checksum::verify(checksum::xxh3_64, cs_span->data(),
+                              cs_span->size(), &*cs_val, sizeof(*cs_val));
+    }
+  }
+
+  return true;
+}
+
+} // namespace
+
+bool fs_section_checker::check(fs_section const& section) const {
+  return check_impl(section, mm_);
+}
+
+bool fs_section_checker::check(fs_section::impl const& section) const {
+  return check_impl(section, mm_);
+}
+
+bool fs_section_checker::verify(fs_section const& section) const {
+  if (auto const cs_val = section.sha2_512_256_value()) {
+    if (auto const cs_span = section.integrity_span(mm_)) {
+      return checksum::verify(checksum::sha2_512_256, cs_span->data(),
+                              cs_span->size(), cs_val->data(), cs_val->size());
+    }
+  }
+
+  return true;
+}
+
+} // namespace dwarfs::internal

src/reader/filesystem_v2.cpp

@@ -55,6 +55,7 @@
 #include <dwarfs/vector_byte_buffer.h>
 
 #include <dwarfs/internal/fs_section.h>
+#include <dwarfs/internal/fs_section_checker.h>
 #include <dwarfs/internal/worker_group.h>
 #include <dwarfs/reader/internal/block_cache.h>
 #include <dwarfs/reader/internal/filesystem_parser.h>
@@ -539,13 +540,16 @@ int filesystem_<LoggerPolicy>::check(filesystem_check_level level,
   worker_group wg(LOG_GET_LOGGER, os_, "fscheck", num_threads);
   std::vector<std::future<fs_section>> sections;
 
+  fs_section_checker section_checker(*mm_);
+
   while (auto sp = parser.next_section()) {
     check_section(*sp);
 
-    std::packaged_task<fs_section()> task{[this, level, s = std::move(*sp)] {
+    std::packaged_task<fs_section()> task{[this, level, &section_checker,
+                                           s = std::move(*sp)] {
       if (level == filesystem_check_level::INTEGRITY ||
           level == filesystem_check_level::FULL) {
-        if (!s.verify(*mm_)) {
+        if (!section_checker.verify(s)) {
           DWARFS_THROW(runtime_error,
                        "integrity check error in section: " + s.name());
         }

src/reader/internal/cached_block.cpp

@@ -63,7 +63,7 @@ class cached_block_ final : public cached_block {
       , LOG_PROXY_INIT(lgr)
       , release_(release)
       , uncompressed_size_{decompressor_->uncompressed_size()} {
-    if (!disable_integrity_check && !section_.check(*mm_)) {
+    if (!disable_integrity_check && !section_.check_fast(*mm_)) {
       DWARFS_THROW(runtime_error, "block data integrity check failed");
     }
     std::atomic_fetch_add(&instance_count_, 1U);