fix for issue #45 (#46)

fixes #45
OpenCyphal · Jul 6, 2023 · 8b0a3b9 · 8b0a3b9
1 parent 4c9eae1
commit 8b0a3b9
Show file tree

Hide file tree

Showing 2 changed files with 36 additions and 4 deletions.
diff --git a/cetlvast/suites/unittest/test_pf17_monotonic_buffer_resource.cpp b/cetlvast/suites/unittest/test_pf17_monotonic_buffer_resource.cpp
@@ -103,3 +103,24 @@ TYPED_TEST(TestMonotonicBufferResource, TestAllocationOrder)
     void* upstream_memory = subject.allocate(size_bytes);
     ASSERT_NE(nullptr, upstream_memory);
 }
+
+// +----------------------------------------------------------------------+
+
+/// Verify fix for issue #45
+TYPED_TEST(TestMonotonicBufferResource, TestIssue45)
+{
+    constexpr std::size_t                   size_bytes = 1024;
+    std::array<cetl::byte, size_bytes>      buffer{};
+    TypeParam subject{buffer.data(), buffer.size(), cetlvast::MRH::null_memory_resource<TypeParam>()};
+    for(std::size_t i = 0; i < 1024 / (2 * alignof(cetl::byte)); ++i)
+    {
+        void*     memory0 = subject.allocate(1, alignof(cetl::byte));
+        ASSERT_NE(nullptr, memory0);
+        void*     memory1 = subject.allocate(1, alignof(cetl::byte));
+        ASSERT_NE(memory0, memory1);
+        ASSERT_LT(memory0, memory1);
+        std::ptrdiff_t range = static_cast<unsigned char*>(memory1) - static_cast<unsigned char*>(memory0);
+        ASSERT_GE(range, sizeof(cetl::byte));
+    }
+    subject.release();
+}
diff --git a/include/cetl/pf17/memory_resource.hpp b/include/cetl/pf17/memory_resource.hpp
@@ -742,12 +742,23 @@ class basic_monotonic_buffer_resource : public memory_resource
         void* result = nullptr;
         if (current_buffer_->buffer && current_buffer_->remaining_buffer_size >= size_bytes)
         {
-            void*       buffer      = current_buffer_->buffer;
-            std::size_t buffer_size = current_buffer_->remaining_buffer_size;
-            result                  = std::align(alignment, size_bytes, buffer, buffer_size);
+            CETL_DEBUG_ASSERT(current_buffer_->buffer_size >= current_buffer_->remaining_buffer_size,
+                              "remaining_buffer_size exceeded total buffer size? We have corrupt internal logic.");
+            const std::size_t current_buffer_size =
+                current_buffer_->buffer_size - current_buffer_->remaining_buffer_size;
+            void*       buffer = &static_cast<unsigned char*>(current_buffer_->buffer)[current_buffer_size];
+            std::size_t remaining_aligned_size = current_buffer_->remaining_buffer_size;
+            result                             = std::align(alignment, size_bytes, buffer, remaining_aligned_size);
             if (result)
             {
-                current_buffer_->remaining_buffer_size = buffer_size - size_bytes;
+                CETL_DEBUG_ASSERT(remaining_aligned_size <= current_buffer_->remaining_buffer_size,
+                                  "std::align must never increase the space parameter");
+                const std::size_t buffer_used =
+                    size_bytes + (current_buffer_->remaining_buffer_size - remaining_aligned_size);
+                CETL_DEBUG_ASSERT(buffer_used <= current_buffer_->remaining_buffer_size,
+                                  "std::align must never return non-null if there isn't enough buffer remaining to "
+                                  "align the pointer.");
+                current_buffer_->remaining_buffer_size -= buffer_used;
             }
         }
         return result;