iox-eclipse-iceoryx#1614 Fix review findings

Use aligned_storage in optional, use char array in posix call, add tests

Signed-off-by: Marika Lehmann <marika.lehmann@apex.ai>

iceoryx_hoofs/include/iceoryx_hoofs/cxx/optional.hpp

@@ -19,7 +19,6 @@
 
 #include "iceoryx_hoofs/cxx/functional_interface.hpp"
 #include "iceoryx_hoofs/cxx/requires.hpp"
-#include "iceoryx_hoofs/internal/containers/uninitialized_array.hpp"
 
 #include <new> // needed for placement new in the construct_value member function
 #include <utility>
@@ -230,7 +229,7 @@ class optional final : public FunctionalInterface<optional<T>, T, void>
     //     initHandle(&handle);
     //   }
     bool m_hasValue{false};
-    containers::UninitializedArray<T, 1> m_data;
+    typename std::aligned_storage<sizeof(T), alignof(T)>::type m_data;
 
   private:
     template <typename... Targs>

iceoryx_hoofs/include/iceoryx_hoofs/internal/containers/uninitialized_array.hpp

@@ -36,15 +36,15 @@ struct ZeroedBuffer
     // NOLINTBEGIN(cppcoreguidelines-avoid-c-arrays, hicpp-avoid-c-arrays)
     using element_t = cxx::byte_t[sizeof(ElementType)];
     // AXIVION Next Construct AutosarC++19_03-A18.1.1 : required by low level UninitializedArray building block and encapsulated in abstraction
-    alignas(ElementType) element_t value[Capacity]{{0}};
+    alignas(ElementType) element_t value[Capacity]{};
     // NOLINTEND(cppcoreguidelines-avoid-c-arrays, hicpp-avoid-c-arrays)
 };
 
 /// @brief struct used as policy parameter in UninitializedArray to wrap an uninitialized array
 /// @tparam ElementType is the array type
 /// @tparam Capacity is the array size
 template <typename ElementType, uint64_t Capacity>
-struct NotZeroedBuffer
+struct NonZeroedBuffer
 {
     // AXIVION Next Construct AutosarC++19_03-A18.1.1 : required by low level UninitializedArray building block and encapsulated in abstraction
     // NOLINTBEGIN(cppcoreguidelines-avoid-c-arrays, hicpp-avoid-c-arrays)
@@ -58,10 +58,10 @@ struct NotZeroedBuffer
 /// all elements can be zeroed via template parameter ZeroedBuffer.
 /// @tparam ElementType is the array type
 /// @tparam Capacity is the array size
-/// @tparam Buffer is the policy parameter to choose between an uninitialized, not zeroed array (=NotZeroedBuffer,
+/// @tparam Buffer is the policy parameter to choose between an uninitialized, not zeroed array (=NonZeroedBuffer,
 /// default) and an uninitialized array with all elements zeroed (=ZeroedBuffer)
 /// @note Out of bounds access leads to undefined behavior
-template <typename ElementType, uint64_t Capacity, template <typename, uint64_t> class Buffer = NotZeroedBuffer>
+template <typename ElementType, uint64_t Capacity, template <typename, uint64_t> class Buffer = NonZeroedBuffer>
 class UninitializedArray
 {
     static_assert(Capacity > 0U, "The size of the UninitializedArray must be greater than 0!");

iceoryx_hoofs/include/iceoryx_hoofs/internal/cxx/vector.inl

@@ -349,7 +349,7 @@ inline bool vector<T, Capacity>::erase(iterator position) noexcept
 {
     if ((begin() <= position) && (position < end()))
     {
-        uint64_t index{static_cast<uint64_t>(position - begin()) % sizeof(decltype(m_data))};
+        uint64_t index{static_cast<uint64_t>(position - begin())};
         size_t n{index};
         while ((n + 1U) < size())
         {

iceoryx_hoofs/include/iceoryx_hoofs/internal/posix_wrapper/posix_call.inl

@@ -73,7 +73,9 @@ inline cxx::string<POSIX_CALL_ERROR_STRING_SIZE> errorLiteralToString(const char
 template <typename T>
 inline cxx::string<POSIX_CALL_ERROR_STRING_SIZE> PosixCallResult<T>::getHumanReadableErrnum() const noexcept
 {
-    containers::UninitializedArray<char, POSIX_CALL_ERROR_STRING_SIZE, containers::ZeroedBuffer> buffer;
+    /// NOLINTJUSTIFICATION needed by POSIX function which is wrapped here
+    /// NOLINTNEXTLINE(hicpp-avoid-c-arrays, cppcoreguidelines-avoid-c-arrays)
+    char buffer[POSIX_CALL_ERROR_STRING_SIZE];
     return internal::errorLiteralToString(strerror_r(errnum, &buffer[0], POSIX_CALL_ERROR_STRING_SIZE), &buffer[0]);
 }
 

iceoryx_hoofs/test/moduletests/test_containers_uninitialized_array.cpp

@@ -25,6 +25,46 @@ using namespace ::testing;
 
 using iox::containers::UninitializedArray;
 
+struct Integer
+{
+    static uint32_t ctor;
+    static uint32_t dtor;
+
+    Integer()
+        : Integer(0)
+    {
+    }
+
+    // NOLINTNEXTLINE(hicpp-explicit-conversions) required for typed tests
+    Integer(int value)
+        : value(value)
+    {
+        ctor++;
+    }
+
+    ~Integer()
+    {
+        dtor++;
+    }
+
+    Integer(const Integer&) = delete;
+    Integer(Integer&&) = delete;
+    Integer& operator=(const Integer&) = delete;
+    Integer& operator=(Integer&&) = delete;
+
+    int value{0};
+
+    // so that it behaves like an int for comparison purposes
+    // NOLINTNEXTLINE(hicpp-explicit-conversions) required for typed tests
+    operator int() const
+    {
+        return value;
+    }
+};
+
+uint32_t Integer::ctor;
+uint32_t Integer::dtor;
+
 template <typename T>
 class UninitializedArrayTest : public ::testing::Test
 {
@@ -43,23 +83,11 @@ class UninitializedArrayTest : public ::testing::Test
             new (&buffer[i]) Type(value++);
         }
     }
-};
-
-struct Integer
-{
-    // NOLINTNEXTLINE(hicpp-explicit-conversions) required for typed tests
-    Integer(int value = 0)
-        : value(value)
-    {
-    }
-
-    int value{0};
 
-    // so that it behaves like an int for comparison purposes
-    // NOLINTNEXTLINE(hicpp-explicit-conversions) required for typed tests
-    operator int() const
+    void SetUp() override
     {
-        return value;
+        Integer::ctor = 0;
+        Integer::dtor = 0;
     }
 };
 
@@ -203,7 +231,7 @@ TYPED_TEST(UninitializedArrayTest, BeginAndEndConstIteratorNotEqualInFullUniniti
     EXPECT_NE(const_cast<const decltype(buffer)>(buffer).begin(), const_cast<const decltype(buffer)>(buffer).end());
 }
 
-TEST(UninitializedArrayTest, IteratorIteratesThroughNonEmptyUninitializedArray)
+TEST(UninitializedArrayTest, IteratorIteratesThroughUninitializedArray)
 {
     ::testing::Test::RecordProperty("TEST_ID", "b42d93c9-cbe8-481f-8a0b-5b3fb8e9020c");
     constexpr uint64_t CAPACITY = 3;
@@ -226,7 +254,7 @@ TEST(UninitializedArrayTest, IteratorIteratesThroughNonEmptyUninitializedArray)
     EXPECT_EQ(count, CAPACITY);
 }
 
-TEST(UninitializedArrayTest, ConstIteratorIteratesThroughNonEmptyUninitializedArray)
+TEST(UninitializedArrayTest, ConstIteratorIteratesThroughUninitializedArray)
 {
     ::testing::Test::RecordProperty("TEST_ID", "e8d7ac7f-9ec7-4264-8b27-d0469b167375");
     constexpr uint64_t CAPACITY = 3;
@@ -249,4 +277,44 @@ TEST(UninitializedArrayTest, ConstIteratorIteratesThroughNonEmptyUninitializedAr
     }
     EXPECT_EQ(count, CAPACITY);
 }
+
+TEST(UninitializedArrayTest, UninitializedArrayDoesNotInitializeOrDestroyElements)
+{
+    ::testing::Test::RecordProperty("TEST_ID", "60334385-60e8-49bc-b297-61f5a5a3b175");
+    constexpr uint64_t CAPACITY{15};
+    Integer::ctor = 0;
+    Integer::dtor = 0;
+
+    {
+        UninitializedArray<Integer, CAPACITY> buffer{};
+        EXPECT_EQ(Integer::ctor, 0);
+
+        for (uint64_t i{0}; i < CAPACITY; ++i)
+        {
+            new (&buffer[i]) Integer(51);
+        }
+        EXPECT_EQ(Integer::ctor, CAPACITY);
+        EXPECT_EQ(Integer::dtor, 0);
+    }
+    EXPECT_EQ(Integer::dtor, 0);
+}
+
+TYPED_TEST(UninitializedArrayTest, SizeOfUninitializedArrayEqualsCStyleArray)
+{
+    ::testing::Test::RecordProperty("TEST_ID", "e1c7ddec-b883-4eee-a4a4-a8dfbcaaec6d");
+    using Buffer = typename TestFixture::Buffer;
+    if (std::is_same<Buffer, UninitializedArray<Integer, 10>>::value
+        || std::is_same<Buffer, UninitializedArray<Integer, 10, iox::containers::ZeroedBuffer>>::value)
+    {
+        // NOLINTNEXTLINE(hicpp-avoid-c-arrays, cppcoreguidelines-avoid-c-arrays) : needed for test purpose
+        Integer testArray[10];
+        EXPECT_EQ(sizeof(this->buffer), sizeof(testArray));
+    }
+    else
+    {
+        // NOLINTNEXTLINE(hicpp-avoid-c-arrays, cppcoreguidelines-avoid-c-arrays) : needed for test purpose
+        int testArray[10];
+        EXPECT_EQ(sizeof(this->buffer), sizeof(testArray));
+    }
+}
 } // namespace