# HG changeset patch # User Mike Becker # Date 1766161458 -3600 # Node ID fe24b68758bf802bf73983df8abe64f42df5b326 # Parent f74e4fc496a2ad86048f7930b72e01ca68a3989b fixes elem_size for default memcmp-based compare function not robust against reallocations resolves #615 diff -r f74e4fc496a2 -r fe24b68758bf src/list.c --- 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 *); diff -r f74e4fc496a2 -r fe24b68758bf tests/test_list.c --- 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, ¬inlist) == 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);