iox-eclipse-iceoryx#391 introduced safe static storage memory clear s…

…emantics Signed-off-by: Matthias Killat <matthias.killat@apex.ai>
ApexAI · May 26, 2021 · fe839d9 · fe839d9
1 parent f489d61
commit fe839d9
Show file tree

Hide file tree

Showing 4 changed files with 26 additions and 7 deletions.
diff --git a/iceoryx_hoofs/include/iceoryx_hoofs/internal/cxx/static_storage.hpp b/iceoryx_hoofs/include/iceoryx_hoofs/internal/cxx/static_storage.hpp
@@ -79,8 +79,9 @@ class static_storage
     ///       nor is it overwritten. Setting the memory to zero can be done with clear.
     constexpr void deallocate() noexcept;
 
-    /// @brief set the managed static memory to all zeros
-    constexpr void clear() noexcept;
+    /// @brief set the managed static memory to all zeros if there is no object currently stored
+    /// @return true if the memory was set to zero, false otherwise
+    constexpr bool clear() noexcept;
 
     /// @brief get the storage capacity in bytes
     /// @return maximum number of bytes available in the static storage

diff --git a/iceoryx_hoofs/include/iceoryx_hoofs/internal/cxx/static_storage.inl b/iceoryx_hoofs/include/iceoryx_hoofs/internal/cxx/static_storage.inl
@@ -84,9 +84,14 @@ constexpr void static_storage<Capacity, Align>::deallocate() noexcept
 }
 
 template <uint64_t Capacity, uint64_t Align>
-constexpr void static_storage<Capacity, Align>::clear() noexcept
+constexpr bool static_storage<Capacity, Align>::clear() noexcept
 {
-    std::memset(m_bytes, 0, Capacity);
+    if (m_ptr == nullptr)
+    {
+        std::memset(m_bytes, 0, Capacity);
+        return true;
+    }
+    return false;
 }
 
 template <uint64_t Capacity, uint64_t Align>

diff --git a/iceoryx_hoofs/include/iceoryx_hoofs/internal/cxx/storable_function.inl b/iceoryx_hoofs/include/iceoryx_hoofs/internal/cxx/storable_function.inl
@@ -245,7 +245,6 @@ template <typename S, typename ReturnType, typename... Args>
 template <typename CallableType>
 void storable_function<S, signature<ReturnType, Args...>>::destroy(storable_function& f) noexcept
 {
-    // TODO: this is not an indication allocate was called anymore (due to fptrs also being stored in m_callable)
     if (f.m_callable)
     {
         auto ptr = static_cast<CallableType*>(f.m_callable);

diff --git a/iceoryx_hoofs/test/moduletests/test_static_storage.cpp b/iceoryx_hoofs/test/moduletests/test_static_storage.cpp
@@ -157,7 +157,7 @@ TEST(static_storage_test, TypedAllocateAfterDeallocateSucceeds)
     EXPECT_NE(sut.allocate<Data>(), nullptr);
 }
 
-TEST(static_storage_test, ClearSetsStorageBytesToZero)
+TEST(static_storage_test, ClearSetsStorageBytesToZeroIfThereIsNoObjectStored)
 {
     using Data = Bytes<16, 4>;
     static_storage<18, 2> sut;
@@ -166,10 +166,24 @@ TEST(static_storage_test, ClearSetsStorageBytesToZero)
     data->set(37);
     EXPECT_TRUE(data->hasValue(37));
 
-    sut.clear();
+    sut.deallocate();
+    EXPECT_TRUE(sut.clear());
     EXPECT_TRUE(data->hasValue(0));
 }
 
+TEST(static_storage_test, ClearHasNoEffectIfThereIsAnObjectStored)
+{
+    using Data = Bytes<16, 4>;
+    static_storage<18, 2> sut;
+    auto data = sut.allocate<Data>();
+    ASSERT_NE(data, nullptr);
+    data->set(37);
+    EXPECT_TRUE(data->hasValue(37));
+
+    EXPECT_FALSE(sut.clear());
+    EXPECT_TRUE(data->hasValue(37));
+}
+
 TEST(static_storage_test, DeallocateDoesNotClearStorageBytes)
 {
     using Data = Bytes<16, 4>;