diff --git a/src/array_list.c b/src/array_list.c index 22dc8188..a9a08db8 100644 --- a/src/array_list.c +++ b/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--; diff --git a/test/test_array_list.cpp b/test/test_array_list.cpp index 6ed9d951..731514f0 100644 --- a/test/test_array_list.cpp +++ b/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;