Sun, 26 Oct 2025 13:08:42 +0100
add more test coverage for new map functions and fixes some issues
the "what if dst already contains a key" part did not really work
plus the cxMapListDifference() did not fallback to a default allocator
really resolves #746
| src/map.c | file | annotate | diff | comparison | revisions | |
| tests/test_hash_map.c | file | annotate | diff | comparison | revisions |
--- a/src/map.c Sun Oct 26 12:50:43 2025 +0100 +++ b/src/map.c Sun Oct 26 13:08:42 2025 +0100 @@ -148,7 +148,7 @@ const CxMapEntry *entry = cxIteratorCurrent(src_iter); void **dst_mem = cxMapEmplace(dst, *(entry->key)); if (dst_mem == NULL) { - return 1; + return 1; // LCOV_EXCL_LINE } void *target = cxCollectionStoresPointers(dst) ? NULL : dst_mem; void *dst_ptr = clone_func(target, entry->value, clone_allocator, data); @@ -168,27 +168,32 @@ cx_clone_func clone_func, const CxAllocator *clone_allocator, void *data) { if (clone_allocator == NULL) clone_allocator = cxDefaultAllocator; - const bool map_was_not_empty = cxMapSize(dst) > 0; + // if the destination map already contains something, + // remove what does not belong to the difference + CxMapIterator dst_iter = cxMapIteratorKeys(dst); + cx_foreach(const CxHashKey *, key, dst_iter) { + if (cxMapContains(subtrahend, *key)) { + cxIteratorFlagRemoval(dst_iter); + } + } + CxMapIterator src_iter = cxMapIterator(minuend); cx_foreach(const CxMapEntry *, entry, src_iter) { if (cxMapContains(subtrahend, *entry->key)) { - if (map_was_not_empty) { - cxMapRemove(dst, *entry->key); - } - } else { - void** dst_mem = cxMapEmplace(dst, *entry->key); - if (dst_mem == NULL) { - return 1; - } - void *target = cxCollectionStoresPointers(dst) ? NULL : dst_mem; - void* dst_ptr = clone_func(target, entry->value, clone_allocator, data); - if (dst_ptr == NULL) { - cx_map_remove_uninitialized_entry(dst, *(entry->key)); - return 1; - } - if (cxCollectionStoresPointers(dst)) { - *dst_mem = dst_ptr; - } + continue; + } + void** dst_mem = cxMapEmplace(dst, *entry->key); + if (dst_mem == NULL) { + return 1; // LCOV_EXCL_LINE + } + void *target = cxCollectionStoresPointers(dst) ? NULL : dst_mem; + void* dst_ptr = clone_func(target, entry->value, clone_allocator, data); + if (dst_ptr == NULL) { + cx_map_remove_uninitialized_entry(dst, *(entry->key)); + return 1; + } + if (cxCollectionStoresPointers(dst)) { + *dst_mem = dst_ptr; } } return 0; @@ -196,27 +201,34 @@ int cxMapListDifference(CxMap *dst, const CxMap *src, const CxList *keys, cx_clone_func clone_func, const CxAllocator *clone_allocator, void *data) { - const bool map_was_not_empty = cxMapSize(dst) > 0; + if (clone_allocator == NULL) clone_allocator = cxDefaultAllocator; + + // if the destination map already contains something, + // remove what does not belong to the difference + CxMapIterator dst_iter = cxMapIteratorKeys(dst); + cx_foreach(const CxHashKey *, key, dst_iter) { + if (cxListContains(keys, key)) { + cxIteratorFlagRemoval(dst_iter); + } + } + CxMapIterator src_iter = cxMapIterator(src); cx_foreach(const CxMapEntry *, entry, src_iter) { if (cxListContains(keys, entry->key)) { - if (map_was_not_empty) { - cxMapRemove(dst, *entry->key); - } - } else { - void** dst_mem = cxMapEmplace(dst, *entry->key); - if (dst_mem == NULL) { - return 1; - } - void *target = cxCollectionStoresPointers(dst) ? NULL : dst_mem; - void* dst_ptr = clone_func(target, entry->value, clone_allocator, data); - if (dst_ptr == NULL) { - cx_map_remove_uninitialized_entry(dst, *(entry->key)); - return 1; - } - if (cxCollectionStoresPointers(dst)) { - *dst_mem = dst_ptr; - } + continue; + } + void** dst_mem = cxMapEmplace(dst, *entry->key); + if (dst_mem == NULL) { + return 1; // LCOV_EXCL_LINE + } + void *target = cxCollectionStoresPointers(dst) ? NULL : dst_mem; + void* dst_ptr = clone_func(target, entry->value, clone_allocator, data); + if (dst_ptr == NULL) { + cx_map_remove_uninitialized_entry(dst, *(entry->key)); + return 1; + } + if (cxCollectionStoresPointers(dst)) { + *dst_mem = dst_ptr; } } return 0;
--- a/tests/test_hash_map.c Sun Oct 26 12:50:43 2025 +0100 +++ b/tests/test_hash_map.c Sun Oct 26 13:08:42 2025 +0100 @@ -655,6 +655,34 @@ cxMapFree(s2); } +CX_TEST(test_hash_map_difference_alloc_fail) { + CxMap *dst = cxHashMapCreateSimple(sizeof(int)); + + CxMap *s1 = cxHashMapCreateSimple(CX_STORE_POINTERS); + CxMap *s2 = cxHashMapCreateSimple(CX_STORE_POINTERS); + const char *s1_keys[] = {"k1", "k2", "k3"}; + int s1_values[] = {1, 3, 4}; + const char *s2_keys[] = {"k4", "k2", "k5"}; + int s2_values[] = {7, 9, 15}; + for (unsigned int i = 0 ; i < 3 ; i++) { + cxMapPut(s1, s1_keys[i], &s1_values[i]); + cxMapPut(s2, s2_keys[i], &s2_values[i]); + } + CX_TEST_DO { + int c = 4; + test_hash_map_clone_func_max_clones = 1; + CX_TEST_ASSERT(1 == cxMapDifference(dst, s1, s2, test_hash_map_clone_func, NULL, &c)); + CX_TEST_ASSERT(cxMapSize(dst) == 1); + // the concrete element which is affected might change when the hash function changes + CX_TEST_ASSERT(cxMapGet(dst, "k1") == NULL); + CX_TEST_ASSERT(cxMapGet(dst, "k2") == NULL); + CX_TEST_ASSERT(*((int*)cxMapGet(dst, "k3")) == 8); + } + cxMapFree(dst); + cxMapFree(s1); + cxMapFree(s2); +} + CX_TEST(test_hash_map_list_difference) { CxMap *dst = cxHashMapCreateSimple(sizeof(int)); CxMap *src = cxHashMapCreateSimple(sizeof(int)); @@ -684,6 +712,108 @@ cxListFree(keys); } +CX_TEST(test_hash_map_list_difference_alloc_fail) { + CxMap *dst = cxHashMapCreateSimple(sizeof(int)); + CxMap *src = cxHashMapCreateSimple(sizeof(int)); + CxList *keys = cxArrayListCreate(NULL, cx_hash_key_cmp, sizeof(CxHashKey), 4); + + const char *src_keys[] = {"k1", "k2", "k3"}; + int src_values[] = {1, 3, 4}; + for (unsigned int i = 0 ; i < 3 ; i++) { + cxMapPut(src, src_keys[i], &src_values[i]); + } + const char *k[] = {"k4", "k2", "k5"}; + for (unsigned int i = 0 ; i < 3 ; i++) { + CxHashKey key = CX_HASH_KEY(k[i]); + cxListAdd(keys, &key); + } + CX_TEST_DO { + int c = 4; + test_hash_map_clone_func_max_clones = 1; + CX_TEST_ASSERT(1 == cxMapListDifference(dst, src, keys, test_hash_map_clone_func, NULL, &c)); + CX_TEST_ASSERT(cxMapSize(dst) == 1); + // the concrete element which is affected might change when the hash function changes + CX_TEST_ASSERT(cxMapGet(dst, "k1") == NULL); + CX_TEST_ASSERT(cxMapGet(dst, "k2") == NULL); + CX_TEST_ASSERT(*((int*)cxMapGet(dst, "k3")) == 8); + } + cxMapFree(dst); + cxMapFree(src); + cxListFree(keys); +} + +CX_TEST(test_hash_map_difference_non_empty_target) { + CxMap *dst = cxHashMapCreateSimple(CX_STORE_POINTERS); + cxDefineAdvancedDestructor(dst, cxFree, (void*) cxDefaultAllocator); + + CxMap *s1 = cxHashMapCreateSimple(sizeof(int)); + CxMap *s2 = cxHashMapCreateSimple(sizeof(int)); + const char *s1_keys[] = {"k1", "k2", "k3"}; + int s1_values[] = {1, 3, 4}; + const char *s2_keys[] = {"k4", "k2", "k5"}; + int s2_values[] = {7, 9, 15}; + for (unsigned int i = 0 ; i < 3 ; i++) { + cxMapPut(s1, s1_keys[i], &s1_values[i]); + cxMapPut(s2, s2_keys[i], &s2_values[i]); + } + + // add k5 to dst which is not in src, but shall also be not in the difference + int *k5 = cxMallocDefault(sizeof(int)); + *k5 = 1337; + cxMapPut(dst, "k5",k5); + + CX_TEST_DO { + int c = 4; + test_hash_map_clone_func_max_clones = 100; // no limit + CX_TEST_ASSERT(0 == cxMapDifference(dst, s1, s2, test_hash_map_clone_func, NULL, &c)); + CX_TEST_ASSERT(cxMapSize(dst) == 2); + CX_TEST_ASSERT(*((int*)cxMapGet(dst, "k1")) == 5); + CX_TEST_ASSERT(cxMapGet(dst, "k2") == NULL); + CX_TEST_ASSERT(*((int*)cxMapGet(dst, "k3")) == 8); + CX_TEST_ASSERT(cxMapGet(dst, "k5") == NULL); + } + cxMapFree(dst); + cxMapFree(s1); + cxMapFree(s2); +} + +CX_TEST(test_hash_map_list_difference_non_empty_target) { + CxMap *dst = cxHashMapCreateSimple(CX_STORE_POINTERS); + cxDefineAdvancedDestructor(dst, cxFree, (void*) cxDefaultAllocator); + CxMap *src = cxHashMapCreateSimple(sizeof(int)); + CxList *keys = cxArrayListCreate(NULL, cx_hash_key_cmp, sizeof(CxHashKey), 4); + + const char *src_keys[] = {"k1", "k2", "k3"}; + int src_values[] = {1, 3, 4}; + for (unsigned int i = 0 ; i < 3 ; i++) { + cxMapPut(src, src_keys[i], &src_values[i]); + } + const char *k[] = {"k4", "k2", "k5"}; + for (unsigned int i = 0 ; i < 3 ; i++) { + CxHashKey key = CX_HASH_KEY(k[i]); + cxListAdd(keys, &key); + } + + // add k5 to dst which is not in src, but shall also be not in the difference + int *k5 = cxMallocDefault(sizeof(int)); + *k5 = 1337; + cxMapPut(dst, "k5",k5); + + CX_TEST_DO { + int c = 4; + test_hash_map_clone_func_max_clones = 100; // no limit + CX_TEST_ASSERT(0 == cxMapListDifference(dst, src, keys, test_hash_map_clone_func, NULL, &c)); + CX_TEST_ASSERT(cxMapSize(dst) == 2); + CX_TEST_ASSERT(*((int*)cxMapGet(dst, "k1")) == 5); + CX_TEST_ASSERT(cxMapGet(dst, "k2") == NULL); + CX_TEST_ASSERT(*((int*)cxMapGet(dst, "k3")) == 8); + CX_TEST_ASSERT(cxMapGet(dst, "k5") == NULL); + } + cxMapFree(dst); + cxMapFree(src); + cxListFree(keys); +} + CX_TEST(test_hash_map_difference_ptr) { CxTestingAllocator talloc; cx_testing_allocator_init(&talloc); @@ -1070,6 +1200,10 @@ cx_test_register(suite, test_hash_map_difference); cx_test_register(suite, test_hash_map_difference_ptr); cx_test_register(suite, test_hash_map_list_difference); + cx_test_register(suite, test_hash_map_difference_alloc_fail); + cx_test_register(suite, test_hash_map_list_difference_alloc_fail); + cx_test_register(suite, test_hash_map_difference_non_empty_target); + cx_test_register(suite, test_hash_map_list_difference_non_empty_target); cx_test_register(suite, test_empty_map_no_ops); cx_test_register(suite, test_empty_map_size); cx_test_register(suite, test_empty_map_get);