fixes elem_size for default memcmp-based compare function not robust against reallocations

Fri, 19 Dec 2025 17:24:18 +0100

author
Mike Becker <universe@uap-core.de>
date
Fri, 19 Dec 2025 17:24:18 +0100
changeset 1633
fe24b68758bf
parent 1632
f74e4fc496a2
child 1634
006e076a8db7

fixes elem_size for default memcmp-based compare function not robust against reallocations

resolves #615

src/list.c file | annotate | diff | comparison | revisions
tests/test_list.c file | annotate | diff | comparison | revisions
--- a/src/list.c	Fri Dec 19 14:15:54 2025 +0100
+++ b/src/list.c	Fri Dec 19 17:24:18 2025 +0100
@@ -339,6 +339,13 @@
     return 0;
 }
 
+static int cx_list_ccmp_safe_memcmp(const void *a, const void *b, void *c) {
+    // it is not safe to store a pointer to the size in the list
+    // because the entire list structure might get reallocated
+    size_t elem_size = (size_t)(uintptr_t)c;
+    return memcmp(a, b, elem_size);
+}
+
 void cx_list_init(
     struct cx_list_s *list,
     struct cx_list_class_s *cl,
@@ -352,8 +359,8 @@
     if (elem_size > 0) {
         list->collection.elem_size = elem_size;
         list->collection.simple_cmp = NULL;
-        list->collection.advanced_cmp = cx_ccmp_memcmp;
-        list->collection.cmp_data = &list->collection.elem_size;
+        list->collection.advanced_cmp = cx_list_ccmp_safe_memcmp;
+        list->collection.cmp_data = (void*)(uintptr_t)list->collection.elem_size;
         list->collection.store_pointer = false;
     } else {
         list->collection.elem_size = sizeof(void *);
--- a/tests/test_list.c	Fri Dec 19 14:15:54 2025 +0100
+++ b/tests/test_list.c	Fri Dec 19 17:24:18 2025 +0100
@@ -1464,41 +1464,6 @@
     cx_testing_allocator_destroy(&talloc);
 }
 
-CX_TEST(test_list_ll_create_simple) {
-    CxList *list = cxLinkedListCreate(NULL, sizeof(int));
-    CX_TEST_DO {
-        CX_TEST_ASSERT(list != NULL);
-        CX_TEST_ASSERT(list->collection.elem_size == sizeof(int));
-        CX_TEST_ASSERT(list->collection.simple_destructor == NULL);
-        CX_TEST_ASSERT(list->collection.advanced_destructor == NULL);
-        CX_TEST_ASSERT(list->collection.destructor_data == NULL);
-        CX_TEST_ASSERT(cxListSize(list) == 0);
-        CX_TEST_ASSERT(list->collection.allocator == cxDefaultAllocator);
-        CX_TEST_ASSERT(list->collection.simple_cmp == NULL);
-        CX_TEST_ASSERT(list->collection.advanced_cmp == cx_ccmp_memcmp);
-        CX_TEST_ASSERT(list->collection.cmp_data == &list->collection.elem_size);
-        CX_TEST_ASSERT(!list->collection.store_pointer);
-    }
-    cxListFree(list);
-}
-
-CX_TEST(test_list_ll_create_simple_for_pointers) {
-    CxList *list = cxLinkedListCreate(NULL, CX_STORE_POINTERS);
-    CX_TEST_DO {
-        CX_TEST_ASSERT(list != NULL);
-        CX_TEST_ASSERT(list->collection.elem_size == sizeof(void*));
-        CX_TEST_ASSERT(list->collection.simple_destructor == NULL);
-        CX_TEST_ASSERT(list->collection.advanced_destructor == NULL);
-        CX_TEST_ASSERT(list->collection.destructor_data == NULL);
-        CX_TEST_ASSERT(cxListSize(list) == 0);
-        CX_TEST_ASSERT(list->collection.allocator == cxDefaultAllocator);
-        CX_TEST_ASSERT(list->collection.simple_cmp == cx_cmp_ptr);
-        CX_TEST_ASSERT(list->collection.advanced_cmp == NULL);
-        CX_TEST_ASSERT(list->collection.store_pointer);
-    }
-    cxListFree(list);
-}
-
 CX_TEST(test_list_arl_create) {
     CxTestingAllocator talloc;
     cx_testing_allocator_init(&talloc);
@@ -1522,41 +1487,6 @@
     cx_testing_allocator_destroy(&talloc);
 }
 
-CX_TEST(test_list_arl_create_simple) {
-    CxList *list = cxArrayListCreate(NULL, sizeof(int), 8);
-    CX_TEST_DO {
-        CX_TEST_ASSERT(list != NULL);
-        CX_TEST_ASSERT(list->collection.elem_size == sizeof(int));
-        CX_TEST_ASSERT(list->collection.simple_destructor == NULL);
-        CX_TEST_ASSERT(list->collection.advanced_destructor == NULL);
-        CX_TEST_ASSERT(list->collection.destructor_data == NULL);
-        CX_TEST_ASSERT(cxListSize(list) == 0);
-        CX_TEST_ASSERT(list->collection.allocator == cxDefaultAllocator);
-        CX_TEST_ASSERT(list->collection.simple_cmp == NULL);
-        CX_TEST_ASSERT(list->collection.advanced_cmp == cx_ccmp_memcmp);
-        CX_TEST_ASSERT(list->collection.cmp_data == &list->collection.elem_size);
-        CX_TEST_ASSERT(!list->collection.store_pointer);
-    }
-    cxListFree(list);
-}
-
-CX_TEST(test_list_arl_create_simple_for_pointers) {
-    CxList *list = cxArrayListCreate(NULL, CX_STORE_POINTERS, 8);
-    CX_TEST_DO {
-        CX_TEST_ASSERT(list != NULL);
-        CX_TEST_ASSERT(list->collection.elem_size == sizeof(void*));
-        CX_TEST_ASSERT(list->collection.simple_destructor == NULL);
-        CX_TEST_ASSERT(list->collection.advanced_destructor == NULL);
-        CX_TEST_ASSERT(list->collection.destructor_data == NULL);
-        CX_TEST_ASSERT(cxListSize(list) == 0);
-        CX_TEST_ASSERT(list->collection.allocator == cxDefaultAllocator);
-        CX_TEST_ASSERT(list->collection.simple_cmp == cx_cmp_ptr);
-        CX_TEST_ASSERT(list->collection.advanced_cmp == NULL);
-        CX_TEST_ASSERT(list->collection.store_pointer);
-    }
-    cxListFree(list);
-}
-
 static void test_fake_simple_int_destr(void *elem) {
     *(int *) elem = 42;
 }
@@ -1770,7 +1700,7 @@
 
 #define array_init(...) {__VA_ARGS__}
 
-static inline int *int_test_data_added_to_list(CxList *list, bool isptrlist, size_t len) {
+static int *int_test_data_added_to_list(CxList *list, bool isptrlist, size_t len) {
     int *testdata = int_test_data(len);
     if (isptrlist) {
         for (size_t i = 0; i < len; i++) {
@@ -2311,18 +2241,27 @@
 
     unsigned exp = rand() % testdata_len; // NOLINT(cert-msc50-cpp)
     int val = testdata[exp];
+    int *x = &testdata[exp];
     // randomly picked number could occur earlier in list - find first position
     for (unsigned i = 0 ; i < exp ; i++) {
         if (testdata[i] == val) {
             exp = i;
+            x = &testdata[i];
             break;
         }
     }
+
+    // if this is not a cx_cmp_ptr test, we use &val to mix up the address
+    // otherwise, we need the exact address
+    if (!list->collection.store_pointer || list->collection.simple_cmp != cx_cmp_ptr) {
+        x = &val;
+    }
+
     CX_TEST_ASSERT(cxListSize(list) == testdata_len);
-    CX_TEST_ASSERT(cxListFind(list, &val) == exp);
-    CX_TEST_ASSERT(cxListFindRemove(list, &val) == exp);
+    CX_TEST_ASSERT(cxListFind(list, x) == exp);
+    CX_TEST_ASSERT(cxListFindRemove(list, x) == exp);
     CX_TEST_ASSERT(cxListSize(list) == testdata_len - 1);
-    CX_TEST_ASSERT(cxListFind(list, &val) != exp);
+    CX_TEST_ASSERT(cxListFind(list, x) != exp);
 
     int notinlist = -1;
     CX_TEST_ASSERT(cxListFindRemove(list, &notinlist) == cxListSize(list));
@@ -2331,6 +2270,44 @@
     free(testdata);
 })
 
+CX_TEST(test_list_ll_find_remove_with_default_cmp) {
+    set_up_combo
+    CxList *list = cxLinkedListCreate(alloc, sizeof(int));
+    CX_TEST_CALL_SUBROUTINE(test_list_verify_find_remove, list, false);
+    tear_down_combo
+}
+CX_TEST(test_list_arl_find_remove_with_default_cmp) {
+    set_up_combo
+    CxList *list = cxArrayListCreate(alloc, sizeof(int), 8);
+    CX_TEST_CALL_SUBROUTINE(test_list_verify_find_remove, list, false);
+    tear_down_combo
+}
+CX_TEST(test_list_kvl_find_remove_with_default_cmp) {
+    set_up_combo
+    CxList *list = cxKvListCreate(alloc, sizeof(int));
+    CX_TEST_CALL_SUBROUTINE(test_list_verify_find_remove, list, false);
+    tear_down_combo
+}
+CX_TEST(test_list_pll_find_remove_with_default_cmp) {
+    set_up_combo
+    CxList *list = cxLinkedListCreate(alloc, CX_STORE_POINTERS);
+    CX_TEST_CALL_SUBROUTINE(test_list_verify_find_remove, list, true);
+    tear_down_combo
+}
+CX_TEST(test_list_parl_find_remove_with_default_cmp) {
+    set_up_combo
+    CxList *list = cxArrayListCreate(alloc, CX_STORE_POINTERS, 8);
+    CX_TEST_CALL_SUBROUTINE(test_list_verify_find_remove, list, true);
+    tear_down_combo
+}
+CX_TEST(test_list_pkvl_find_remove_with_default_cmp) {
+    set_up_combo
+    CxList *list = cxKvListCreate(alloc, CX_STORE_POINTERS);
+    cxSetCompareFunc(list, cx_cmp_int);
+    CX_TEST_CALL_SUBROUTINE(test_list_verify_find_remove, list, true);
+    tear_down_combo
+}
+
 roll_out_test_combos(find_remove_sorted, {
     const size_t testdata_len = 250;
     int *testdata = int_test_data_added_to_list(list, isptrlist, testdata_len);
@@ -3479,8 +3456,6 @@
     CxTestSuite *suite = cx_test_suite_new("array_list");
 
     cx_test_register(suite, test_list_arl_create);
-    cx_test_register(suite, test_list_arl_create_simple);
-    cx_test_register(suite, test_list_arl_create_simple_for_pointers);
     cx_test_register(suite, test_list_parl_destroy_no_destr);
     cx_test_register(suite, test_list_parl_destroy_simple_destr);
     cx_test_register(suite, test_list_parl_destroy_adv_destr);
@@ -3509,6 +3484,8 @@
     cx_test_register(suite, test_list_parl_remove_array);
     cx_test_register(suite, test_list_arl_find_remove);
     cx_test_register(suite, test_list_parl_find_remove);
+    cx_test_register(suite, test_list_arl_find_remove_with_default_cmp);
+    cx_test_register(suite, test_list_parl_find_remove_with_default_cmp);
     cx_test_register(suite, test_list_arl_find_remove_sorted);
     cx_test_register(suite, test_list_parl_find_remove_sorted);
     cx_test_register(suite, test_list_arl_contains);
@@ -3625,8 +3602,6 @@
     cx_test_register(suite, test_linked_list_reverse);
 
     cx_test_register(suite, test_list_ll_create);
-    cx_test_register(suite, test_list_ll_create_simple);
-    cx_test_register(suite, test_list_ll_create_simple_for_pointers);
     cx_test_register(suite, test_list_pll_destroy_no_destr);
     cx_test_register(suite, test_list_pll_destroy_simple_destr);
     cx_test_register(suite, test_list_pll_destroy_adv_destr);
@@ -3655,6 +3630,8 @@
     cx_test_register(suite, test_list_pll_remove_array);
     cx_test_register(suite, test_list_ll_find_remove);
     cx_test_register(suite, test_list_pll_find_remove);
+    cx_test_register(suite, test_list_ll_find_remove_with_default_cmp);
+    cx_test_register(suite, test_list_pll_find_remove_with_default_cmp);
     cx_test_register(suite, test_list_ll_find_remove_sorted);
     cx_test_register(suite, test_list_pll_find_remove_sorted);
     cx_test_register(suite, test_list_ll_contains);
@@ -3773,6 +3750,8 @@
     cx_test_register(suite, test_list_pkvl_remove_array);
     cx_test_register(suite, test_list_kvl_find_remove);
     cx_test_register(suite, test_list_pkvl_find_remove);
+    cx_test_register(suite, test_list_kvl_find_remove_with_default_cmp);
+    cx_test_register(suite, test_list_pkvl_find_remove_with_default_cmp);
     cx_test_register(suite, test_list_kvl_find_remove_sorted);
     cx_test_register(suite, test_list_pkvl_find_remove_sorted);
     cx_test_register(suite, test_list_kvl_contains);

mercurial