Merge pull request eclipse-iceoryx#1825 from ApexAI/iox-1823-fix-vect…

…or-emplace iox-eclipse-iceoryx#1823 Fix double move in emplace
tobiasblass · Dec 15, 2022 · 928a7e6 · 928a7e6
2 parents 6765566 + bbb33bd
commit 928a7e6
Show file tree

Hide file tree

Showing 3 changed files with 8 additions and 5 deletions.
diff --git a/doc/website/release-notes/iceoryx-unreleased.md b/doc/website/release-notes/iceoryx-unreleased.md
@@ -59,6 +59,7 @@
 - Remove `cxx::unique_ptr::reset` [\#1655](https://github.com/eclipse-iceoryx/iceoryx/issues/1655)
 - CI uses outdated clang-format [\#1736](https://github.com/eclipse-iceoryx/iceoryx/issues/1736)
 - Avoid UB when accessing `iox::expected` [\#1750](https://github.com/eclipse-iceoryx/iceoryx/issues/1750)
+- Fix double move in `vector::emplace` [\#1823](https://github.com/eclipse-iceoryx/iceoryx/issues/1823)
 
 **Refactoring:**
 

diff --git a/iceoryx_hoofs/include/iceoryx_hoofs/internal/cxx/vector.inl b/iceoryx_hoofs/include/iceoryx_hoofs/internal/cxx/vector.inl
@@ -174,17 +174,18 @@ template <typename T, uint64_t Capacity>
 template <typename... Targs>
 inline bool vector<T, Capacity>::emplace(const uint64_t position, Targs&&... args) noexcept
 {
-    if ((m_size >= Capacity) || ((position >= Capacity) || (position > m_size)))
+    const auto sizeBeforeEmplace = m_size;
+    if ((m_size >= Capacity) || ((position >= Capacity) || (position > sizeBeforeEmplace)))
     {
         return false;
     }
 
-    if (position == m_size)
+    if (position == sizeBeforeEmplace)
     {
         return emplace_back(std::forward<Targs>(args)...);
     }
-    IOX_DISCARD_RESULT(emplace_back(std::move(at_unchecked(m_size - 1U))));
-    for (uint64_t i{m_size - 1U}; i > position; --i)
+    IOX_DISCARD_RESULT(emplace_back(std::move(at_unchecked(sizeBeforeEmplace - 1U))));
+    for (uint64_t i{sizeBeforeEmplace - 1U}; i > position; --i)
     {
         at_unchecked(i) = std::move(at_unchecked(i - 1U));
     }

diff --git a/iceoryx_hoofs/test/moduletests/test_cxx_vector.cpp b/iceoryx_hoofs/test/moduletests/test_cxx_vector.cpp
@@ -470,7 +470,8 @@ TEST_F(vector_test, EmplacingElementInTheMiddleCallsDTor)
         EXPECT_THAT(customCTor, Eq(EXPECTED_NUMBER_OF_CTOR_CALLS - 1U));
         EXPECT_TRUE(sut.emplace(EMPLACE_POSITION, 42U));
         EXPECT_THAT(customCTor, Eq(EXPECTED_NUMBER_OF_CTOR_CALLS));
-        EXPECT_THAT(moveAssignment, Eq(EMPLACE_POSITION - 1U));
+        EXPECT_THAT(moveCTor, Eq(1U));
+        EXPECT_THAT(moveAssignment, Eq(CAPACITY_OF_VECTOR - 1U - EMPLACE_POSITION - 1U));
         EXPECT_THAT(dTor, Eq(1U));
     }
     // Last element in the vector is moved and not constructed, hence #moveCTor + #customCTor = #dTor