Fixing potentially dangerous memcpy and cleaning up includes and clang-tidy checks

.clang-tidy

@@ -39,6 +39,7 @@ CheckOptions:
   cppcoreguidelines-rvalue-reference-param-not-moved.AllowPartialMove: 'true'
   cppcoreguidelines-avoid-magic-numbers.IgnoredIntegerValues: '1;2;3;4;10'
   cppcoreguidelines-avoid-magic-numbers.IgnoredFloatingPointValues: '1.0;2.0;10.0;100.0'
+  misc-non-private-member-variables-in-classes.IgnoreClassesWithAllMemberVariablesBeingPublic: true
   google-readability-braces-around-statements.ShortStatementLines: '1'
   google-readability-namespace-comments.ShortNamespaceLines: '10'
   google-readability-namespace-comments.SpacesBeforeComments: '2'

.cppcheck_suppress

@@ -35,7 +35,7 @@ missingInclude
 *:*CLI11.hpp
 *:*json.hpp
 *:*boost_unordered.hpp
-*:fmt/*.h
+*:*fmt/*.h
 # Problem caused by external files
 toomanyconfigs
 # More problems caused by external files -- with too many ifdefs

src/bgzf_reader.cpp

@@ -1,8 +1,11 @@
 // SPDX-License-Identifier: MIT; Copyright 2026 Andrew D Smith
 
 #include "bgzf_reader.hpp"
+
 #include "bgzf_block.hpp"
 
+#include <algorithm>
+#include <array>
 #include <bit>
 #include <cassert>
 #include <cerrno>
@@ -15,34 +18,6 @@
 #include <system_error>
 #include <utility>
 
-[[nodiscard]] static inline constexpr auto
-get_unaligned_le32(const auto p) -> std::int32_t {
-  std::int32_t value{};
-  std::memcpy(std::addressof(value), p, sizeof(std::int32_t));
-  if constexpr (std::endian::native == std::endian::big)
-    return std::byteswap(value);
-  else
-    return value;
-}
-
-[[nodiscard]] static inline constexpr auto
-get_isize(const auto data, const auto data_size) {
-  static constexpr decltype(data_size) isize_size = 4;
-  assert(data_size > isize_size);
-  // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic)
-  const auto data_isize = data + data_size - isize_size;
-  return data_size < isize_size ? 0 : get_unaligned_le32(data_isize);
-}
-
-static inline auto
-assign(gzip_header &hdr, const auto data) -> void {
-  // ADS: data from the file takes 18 bytes, the fields of the struct take 18
-  // bytes, but the struct occupies 20 due to uint32_t members
-  // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast)
-  std::memcpy(reinterpret_cast<std::uint8_t *>(&hdr), std::to_address(data),
-              gzip_header_size);
-}
-
 bgzf_reader::bgzf_reader(const std::string &filename,
                          const std::int64_t buf_size) :
   fp(std::fopen(std::data(filename), "r"), &std::fclose),     //
@@ -61,7 +36,7 @@ bgzf_reader::read_data() -> bool {
   if (at_eof())
     return false;
   const auto unused_in = std::distance(next_in, end_in);
-  std::memcpy(inbuf.get(), next_in, unused_in);
+  std::copy_n(next_in, unused_in, inbuf.get());
   next_in = inbuf.get();
   // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic)
   end_in = next_in + unused_in;
@@ -75,17 +50,84 @@ bgzf_reader::read_data() -> bool {
   return n_bytes > 0;
 }
 
+namespace {
+
+static constexpr auto gzip_header_size = 18;
+
+struct gzip_header {
+  static constexpr auto magic1 = 0x1F;
+  static constexpr auto magic2 = 0x8B;
+  static constexpr auto size_index = 16;
+
+  std::array<std::uint8_t, gzip_header_size> data{};
+
+  /// internal structure
+  // std::uint8_t id1{};       // 0
+  // std::uint8_t id2{};       // 1
+  // std::uint8_t cm_eight{};  // 2
+  // std::uint8_t flg{};       // 3
+  // std::uint32_t mtime{};    // 7 [4]
+  // std::uint8_t xfl{};       // 8
+  // std::uint8_t os{};        // 9
+  // std::uint16_t xlen{};     // 11 [2]
+  // char b{};                 // 12
+  // char c{};                 // 13
+  // std::uint16_t two{};      // 14 [2]
+  // std::uint16_t size{};     // 16 [2]
+
+  [[nodiscard]] auto
+  get_size() const -> std::uint16_t {
+    std::uint16_t s{};
+    std::memcpy(std::addressof(s), std::addressof(data[size_index]),
+                sizeof(std::uint16_t));
+    return s;
+  }
+
+  [[nodiscard]] auto
+  check_magic() const -> bool {
+    return data[0] == magic1 && data[1] == magic2;  // ADS: check b and c also?
+  }
+};
+
+[[nodiscard]] static inline constexpr auto
+get_unaligned_le32(const auto p) -> std::int32_t {
+  std::int32_t value{};
+  std::memcpy(std::addressof(value), p, sizeof(std::int32_t));
+  if constexpr (std::endian::native == std::endian::big)
+    return std::byteswap(value);
+  else
+    return value;
+}
+
+[[nodiscard]] static inline constexpr auto
+get_isize(const auto data, const auto data_size) {
+  static constexpr decltype(data_size) isize_size = 4;
+  assert(data_size > isize_size);
+  // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic)
+  const auto data_isize = data + data_size - isize_size;
+  return data_size < isize_size ? 0 : get_unaligned_le32(data_isize);
+}
+
+static inline auto
+assign(gzip_header &hdr,  // cppcheck-suppress constParameterReference
+       const auto data) -> void {
+  std::memcpy(std::data(hdr.data), std::to_address(data), gzip_header_size);
+}
+
 [[nodiscard]] static inline constexpr auto
 get_gzip_body_size(const gzip_header &gh) -> std::uint32_t {
-  return (static_cast<std::uint32_t>(gh.size) + 1) - gzip_header_size;
+  return (static_cast<std::uint32_t>(gh.get_size()) + 1) - gzip_header_size;
 }
 
+}  // namespace
+
 [[nodiscard]] auto
 bgzf_reader::get_decomp_task(char *out_itr) -> bgzf_block_t {
   if (std::distance(next_out, end_out) < max_bgzf_block_size)
     return bgzf_block_t{};
   if (std::distance(next_in, end_in) < gzip_header_size && !read_data())
     return bgzf_block_t{};
+  gzip_header gh;
   assign(gh, next_in);
   assert(gh.check_magic());
   // NOLINTBEGIN(cppcoreguidelines-pro-bounds-pointer-arithmetic)

src/bgzf_reader.hpp

@@ -11,42 +11,13 @@
 #include <memory>
 #include <string>
 
-static constexpr auto gzip_header_size = 18;
-
-struct gzip_header {
-  static constexpr auto magic1 = 0x1F;
-  static constexpr auto magic2 = 0x8B;
-
-  std::uint8_t id1{};       // 0
-  std::uint8_t id2{};       // 1
-  std::uint8_t cm_eight{};  // 2
-  std::uint8_t flg{};       // 3
-  std::uint32_t mtime{};    // 7 [4]
-  std::uint8_t xfl{};       // 8
-  std::uint8_t os{};        // 9
-  std::uint16_t xlen{};     // 11 [2]
-  char b{};                 // 12
-  char c{};                 // 13
-  std::uint16_t two{};      // 14 [2]
-  std::uint16_t size{};     // 16 [2]
-  // 18
-
-  // ADS: total size is 20 bytes because of alignment and 32-bit values
-
-  [[nodiscard]] auto
-  check_magic() const -> bool {
-    return id1 == magic1 && id2 == magic2;  // ADS: check b and c also
-  }
-};
-
 // reads data and provides serialized compressed chunks for deflation
 class bgzf_reader {
 private:
   static constexpr auto inbuf_size = 16 * max_bgzf_block_size;
 
   std::unique_ptr<std::FILE, int (*)(std::FILE *)> fp;
   std::uint64_t filesize{};
-  gzip_header gh;
   std::unique_ptr<char[]> inbuf;   // NOLINT(cppcoreguidelines-avoid-c-arrays)
   std::unique_ptr<char[]> outbuf;  // NOLINT(cppcoreguidelines-avoid-c-arrays)
   char *next_in{};

src/contaminant_set.cpp

@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: MIT; Copyright 2026 Andrew D Smith
 
 #include "contaminant_set.hpp"
+
 #include "falco_utils.hpp"
 
 #include <algorithm>
@@ -11,7 +12,7 @@
 #include <stdexcept>
 #include <string>
 #include <string_view>
-#include <tuple>  // for std::get
+#include <tuple>
 #include <utility>
 #include <vector>
 

src/contaminant_set.hpp

@@ -5,7 +5,7 @@
 
 #include <cstdint>
 #include <iterator>
-#include <ranges>  // for std::pair
+#include <ranges>
 #include <string>
 #include <utility>
 #include <vector>

src/falco.cpp

@@ -345,17 +345,19 @@ main(int argc, char *argv[]) {
     argv = app.ensure_utf8(argv);
     app.usage(
       std::format("Usage: {} [options] -o OUTDIR INFILES", PROJECT_NAME));
+    // NOLINTBEGIN(clang-analyzer-cplusplus.NewDeleteLeaks)
     if (argc >= 2)
       app.footer(std::format(description, falco::get_share_dir()));
+    // NOLINTEND(clang-analyzer-cplusplus.NewDeleteLeaks)
 
     // clang-format off
     // NOLINTNEXTLINE(cppcoreguidelines-avoid-magic-numbers)
     app.get_formatter()->long_option_alignment_ratio(0.2);
     app.set_help_flag("-h,--help", "Print more detailed help");
-    // NOLINTNEXTLINE(clang-analyzer-cplusplus.NewDeleteLeaks)
     app.set_version_flag("--version", VERSION, "Print program version");
-    app.add_flag("--license", [&](auto) {
-      std::print("{}", license_text); throw CLI::Success(); },
+    app.add_flag("--license",
+      // NOLINTNEXTLINE(clang-analyzer-cplusplus.NewDeleteLeaks)
+      [&](auto) { std::print("{}", license_text); throw CLI::Success(); },
       "Print full license")
       ->callback_priority(CLI::CallbackPriority::PreRequirementsCheck);
     auto infiles_opt =

src/falco_analyzer.cpp

@@ -20,7 +20,7 @@
 #include <mutex>
 #include <ranges>
 #include <thread>
-#include <tuple>  // for std::get (iwyu fp)
+#include <tuple>
 #include <utility>
 #include <variant>
 #include <vector>

src/html.cpp

@@ -29,7 +29,7 @@
 #include <span>
 #include <stdexcept>
 #include <string>
-#include <tuple>    // for std::get probably in fmt
+#include <tuple>
 #include <utility>  // for pair
 #include <vector>
 
@@ -371,8 +371,8 @@ basic_stats_html(const file_info &info,
 
 [[nodiscard]] auto
 tile_html(const tile_processor::tiles_centered_t &centered,
-          const std::vector<base_group_t> &groups, const file_grades &grades)
-  -> std::string {
+          const std::vector<base_group_t> &groups,
+          const file_grades &grades) -> std::string {
   static constexpr auto label = "tile";
   static constexpr auto n_quants = 20.0;
   // ADS: ??? (-10: red, 0: light blue, +10: dark blue)

src/report.cpp

@@ -209,8 +209,8 @@ basic_stats_report(const file_info &info,
 
 [[nodiscard]] auto
 tile_report(const tile_processor::tiles_centered_t &centered,
-            const std::vector<base_group_t> &groups, const file_grades &grades)
-  -> std::string {
+            const std::vector<base_group_t> &groups,
+            const file_grades &grades) -> std::string {
   static constexpr auto label = "tile";
   static constexpr auto max_precision{std::numeric_limits<double>::digits10};
   static constexpr auto start_tag = ">>Per tile sequence quality\t{}\n";

src/tile_processor.cpp

@@ -26,7 +26,7 @@
 #include <ranges>
 #include <stdexcept>
 #include <string>
-#include <tuple>  // for std::get
+#include <tuple>
 #include <utility>
 #include <vector>