memmove for overlaping memory (ros2#434) (ros2#437)

* memmove for overlaping memory

* Add test for memcpy to memmove

Signed-off-by: Tyler Weaver <[email protected]>
(cherry picked from commit 5b99aad)

Co-authored-by: Tyler Weaver <[email protected]>

Author: mergify[bot]

src/array_list.c

@@ -172,7 +172,11 @@ rcutils_array_list_remove(rcutils_array_list_t * array_list, size_t index)
   if (copy_count > 0) {
     uint8_t * dst_ptr = rcutils_array_list_get_pointer_for_index(array_list, index);
     uint8_t * src_ptr = rcutils_array_list_get_pointer_for_index(array_list, index + 1);
-    memcpy(dst_ptr, src_ptr, array_list->impl->data_size * copy_count);
+    // If the size of the list is >1 the regions of memory overlap.
+    // POSIX and C standards are explicit that employing memcpy() with overlapping
+    // areas produces undefined behavior. The recomendation is to use memmove.
+    // Reference: https://man7.org/linux/man-pages/man3/memcpy.3.html
+    memmove(dst_ptr, src_ptr, array_list->impl->data_size * copy_count);
   }
 
   array_list->impl->size--;

test/test_array_list.cpp

@@ -248,6 +248,30 @@ TEST_F(ArrayListPreInitTest, remove_success_removes_from_list) {
   EXPECT_EQ(size, (size_t)0);
 }
 
+TEST_F(ArrayListPreInitTest, remove_success_removes_from_list_with_multiple_items) {
+  uint32_t data = 22;
+  size_t index = 0;
+  size_t size = 0;
+  rcutils_ret_t ret = RCUTILS_RET_OK;
+
+  // Add a few things first so we know the index isn't out of bounds
+  for (size_t i = 0; i < 3; ++i) {
+    ret = rcutils_array_list_add(&list, &data);
+    ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string().str;
+  }
+
+  ret = rcutils_array_list_get_size(&list, &size);
+  ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string().str;
+  EXPECT_EQ(size, (size_t)3);
+
+  ret = rcutils_array_list_remove(&list, index);
+  ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string().str;
+
+  ret = rcutils_array_list_get_size(&list, &size);
+  ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string().str;
+  EXPECT_EQ(size, (size_t)2);
+}
+
 TEST_F(ArrayListTest, get_list_null_fails) {
   size_t index = 0;
   uint32_t data = 0;