fix: Fix undefined behavior in util::read_file_part for zero count

If util::read_file_part's count argument is 0 (which can currently only
happen in unit tests), we'll do &result[0] where result is
default-initialized std::unique_ptr<uint8_t[]>. This is not allowed even
though we won't actually dereference the pointer. Found by compiling
with -D_GLIBCXX_ASSERTIONS.

Fixes #1288.

diff --git a/src/util/file.cpp b/src/util/file.cpp
index d084f88c43151969357914527515ebb2aad93c12..57a6b560f128363166cc2918db420029b83e6dd9 100644 (file)
--- a/src/util/file.cpp
+++ b/src/util/file.cpp
@@ -214,6 +214,11 @@ template<typename T>
 nonstd::expected<T, std::string>
 read_file_part(const std::string& path, size_t pos, size_t count)
 {
+  T result;
+  if (count == 0) {
+    return result;
+  }
+
   Fd fd(open(path.c_str(), O_RDONLY | O_BINARY));
   if (!fd) {
     LOG("Failed to open {}: {}", path, strerror(errno));
@@ -226,7 +231,6 @@ read_file_part(const std::string& path, size_t pos, size_t count)
 
   int64_t ret = 0;
   size_t bytes_read = 0;
-  T result;
   result.resize(count);
 
   while (true) {

diff --git a/unittest/test_util_file.cpp b/unittest/test_util_file.cpp
index 77e5c0bec8f0079b1413ee6b2c77c39480c1dcaf..4695750c2e15ab7246623f4bea11b0b27e5c6145 100644 (file)
--- a/unittest/test_util_file.cpp
+++ b/unittest/test_util_file.cpp
@@ -147,7 +147,9 @@ TEST_CASE("util::read_file_part")
 
   SUBCASE("util::Bytes")
   {
-    CHECK(util::read_file_part<util::Bytes>("test", 0, 0) == util::to_span(""));
+    auto lhs = util::read_file_part<util::Bytes>("test", 0, 0);
+    auto rhs = util::to_span("");
+    CHECK(lhs == rhs);
     CHECK(util::read_file_part<util::Bytes>("test", 0, 6)
           == util::to_span("banana"));
     CHECK(util::read_file_part<util::Bytes>("test", 0, 1000)