Mercurial > hg > ucx / changeset

--- a/src/cx/map.h	Tue Jan 07 18:37:07 2025 +0100
+++ b/src/cx/map.h	Tue Jan 07 19:16:03 2025 +0100
@@ -494,6 +494,8 @@
  * Puts a key/value-pair into the map.
  *
  * A possible existing value will be overwritten.
+ * If destructor functions are specified, they are called for
+ * the overwritten element.
  *
  * If this map is storing pointers, the @p value pointer is written
  * to the map. Otherwise, the memory is copied from @p value with
@@ -705,7 +707,7 @@
  * Removes a key/value-pair from the map by using the key.
  *
  * This function will copy the contents of the removed element
- * to the target buffer must be guaranteed to be large enough
+ * to the target buffer, which must be guaranteed to be large enough
  * to hold the element (the map's element size).
  * The destructor functions, if any, will @em not be called.
  *

--- a/src/hash_map.c	Tue Jan 07 18:37:07 2025 +0100
+++ b/src/hash_map.c	Tue Jan 07 19:16:03 2025 +0100
@@ -103,7 +103,8 @@
 
     if (elm != NULL && elm->key.hash == hash && elm->key.len == key.len &&
         memcmp(elm->key.data, key.data, key.len) == 0) {
-        // overwrite existing element
+        // overwrite existing element, but call destructors first
+        cx_invoke_destructor(map, elm->data);
         if (map->collection.store_pointer) {
             memcpy(elm->data, &value, sizeof(void *));
         } else {

--- a/tests/test_hash_map.c	Tue Jan 07 18:37:07 2025 +0100
+++ b/tests/test_hash_map.c	Tue Jan 07 19:16:03 2025 +0100
@@ -257,15 +257,20 @@
     cx_testing_allocator_destroy(&talloc);
 }
 
-static void test_simple_destructor(void *data) {
-    strcpy(data, "OK");
+struct test_destr_struct {
+    char *str;
+};
+
+static void test_simple_destructor(void *d) {
+    struct test_destr_struct *data = d;
+    strcpy(data->str, "OK");
 }
 
 static void test_advanced_destructor(
         cx_attr_unused void *unused,
-        void *data
+        void *d
 ) {
-    strcpy(data, "OK");
+    test_simple_destructor(d);
 }
 
 static CX_TEST_SUBROUTINE(verify_any_destructor, CxMap *map) {
@@ -280,35 +285,58 @@
     char v3[] = "val 3";
     char v4[] = "val 4";
     char v5[] = "val 5";
+    char v6[] = "val 6";
+    char v7[] = "val 7";
 
-    cxMapPut(map, k1, v1);
-    cxMapPut(map, k2, v2);
-    cxMapPut(map, k3, v3);
-    cxMapPut(map, k4, v4);
+    struct test_destr_struct d1 = {v1};
+    struct test_destr_struct d2 = {v2};
+    struct test_destr_struct d3 = {v3};
+    struct test_destr_struct d4 = {v4};
+    struct test_destr_struct d5 = {v5};
+    struct test_destr_struct d6 = {v6};
+    struct test_destr_struct d7 = {v7};
+
+    void *v1data = &d1;
+    void *v2data = &d2;
+    void *v3data = &d3;
+    void *v4data = &d4;
+    void *v5data = &d5;
+    void *v6data = &d6;
+    void *v7data = &d7;
+
+    cxMapPut(map, k1, v1data);
+    cxMapPut(map, k2, v2data);
+    cxMapPut(map, k3, v3data);
+    cxMapPut(map, k4, v4data);
 
     CX_TEST_ASSERT(0 == cxMapRemove(map, k2));
-    char *r;
-    CX_TEST_ASSERT(0 == cxMapRemoveAndGet(map, k3, &r));
+    if (map->collection.store_pointer) {
+        struct test_destr_struct *r;
+        CX_TEST_ASSERT(0 == cxMapRemoveAndGet(map, k3, &r));
+        CX_TEST_ASSERT(r == &d3);
+    } else {
+        struct test_destr_struct r;
+        CX_TEST_ASSERT(0 == cxMapRemoveAndGet(map, k3, &r));
+        CX_TEST_ASSERT(r.str == v3);
+    }
 
     CX_TEST_ASSERT(0 == strcmp(v1, "val 1"));
     CX_TEST_ASSERT(0 == strcmp(v2, "OK"));
     CX_TEST_ASSERT(0 == strcmp(v3, "val 3"));
     CX_TEST_ASSERT(0 == strcmp(v4, "val 4"));
     CX_TEST_ASSERT(0 == strcmp(v5, "val 5"));
-    CX_TEST_ASSERT(r == v3);
 
-    cxMapClear(map);
+    // put k5, overwrite k1, put new k3
+    cxMapPut(map, k5, v5data);
+    cxMapPut(map, k1, v6data);
+    cxMapPut(map, k3, v7data);
 
-    CX_TEST_ASSERT(0 == strcmp(v1, "val 1"));
-    CX_TEST_ASSERT(0 == strcmp(v2, "OK"));
+    // destructor the value behind k1 was called, but for k3 not
+    CX_TEST_ASSERT(0 == strcmp(v1, "OK"));
     CX_TEST_ASSERT(0 == strcmp(v3, "val 3"));
-    CX_TEST_ASSERT(0 == strcmp(v4, "OK"));
-    CX_TEST_ASSERT(0 == strcmp(v5, "val 5"));
 
-    cxMapPut(map, k1, (void *) v1);
-    cxMapPut(map, k3, (void *) v3);
-    cxMapPut(map, k5, (void *) v5);
-
+    // now remove k1 via key iterator and k5 (val 5) via value iterator
+    v1[0] = 'y'; // mark v1 and check that destr is not called for previous val
     {
         CxIterator iter = cxMapMutIteratorKeys(map);
         cx_foreach(CxHashKey*, key, iter) {
@@ -317,29 +345,63 @@
     }
     {
         CxIterator iter = cxMapMutIteratorValues(map);
-        cx_foreach(char*, v, iter) {
-            if (v[4] == '5') cxIteratorFlagRemoval(iter);
+        cx_foreach(struct test_destr_struct*, v, iter) {
+            if (v->str[4] == '5') cxIteratorFlagRemoval(iter);
         }
     }
 
-    CX_TEST_ASSERT(0 == strcmp(v1, "OK"));
+    CX_TEST_ASSERT(0 == strcmp(v1, "yK"));
     CX_TEST_ASSERT(0 == strcmp(v2, "OK"));
     CX_TEST_ASSERT(0 == strcmp(v3, "val 3"));
-    CX_TEST_ASSERT(0 == strcmp(v4, "OK"));
+    CX_TEST_ASSERT(0 == strcmp(v4, "val 4"));
     CX_TEST_ASSERT(0 == strcmp(v5, "OK"));
+    CX_TEST_ASSERT(0 == strcmp(v6, "OK"));
+    CX_TEST_ASSERT(0 == strcmp(v7, "val 7"));
 
+    // mark the already destroyed items
+    // and check that they are not destroyed again
     v1[0] = v2[0] = v4[0] = v5[0] = 'c';
 
     cxMapFree(map);
 
     CX_TEST_ASSERT(0 == strcmp(v1, "cK"));
     CX_TEST_ASSERT(0 == strcmp(v2, "cK"));
-    CX_TEST_ASSERT(0 == strcmp(v3, "OK"));
-    CX_TEST_ASSERT(0 == strcmp(v4, "cK"));
+    CX_TEST_ASSERT(0 == strcmp(v3, "val 3"));
+    CX_TEST_ASSERT(0 == strcmp(v4, "OK"));
     CX_TEST_ASSERT(0 == strcmp(v5, "cK"));
+    CX_TEST_ASSERT(0 == strcmp(v6, "OK"));
+    CX_TEST_ASSERT(0 == strcmp(v7, "OK"));
 }
 
-CX_TEST(test_hash_map_simple_destructor) {
+CX_TEST(test_hash_map_simple_destructor_objects) {
+    CxTestingAllocator talloc;
+    cx_testing_allocator_init(&talloc);
+    CxAllocator *allocator = &talloc.base;
+    CX_TEST_DO {
+        CxMap *map = cxHashMapCreate(allocator,
+            sizeof(struct test_destr_struct), 0);
+        map->collection.simple_destructor = test_simple_destructor;
+        CX_TEST_CALL_SUBROUTINE(verify_any_destructor, map);
+        CX_TEST_ASSERT(cx_testing_allocator_verify(&talloc));
+    }
+    cx_testing_allocator_destroy(&talloc);
+}
+
+CX_TEST(test_hash_map_advanced_destructor_objects) {
+    CxTestingAllocator talloc;
+    cx_testing_allocator_init(&talloc);
+    CxAllocator *allocator = &talloc.base;
+    CX_TEST_DO {
+        CxMap *map = cxHashMapCreate(allocator,
+            sizeof(struct test_destr_struct), 0);
+        map->collection.advanced_destructor = test_advanced_destructor;
+        CX_TEST_CALL_SUBROUTINE(verify_any_destructor, map);
+        CX_TEST_ASSERT(cx_testing_allocator_verify(&talloc));
+    }
+    cx_testing_allocator_destroy(&talloc);
+}
+
+CX_TEST(test_hash_map_simple_destructor_pointers) {
     CxTestingAllocator talloc;
     cx_testing_allocator_init(&talloc);
     CxAllocator *allocator = &talloc.base;
@@ -352,7 +414,7 @@
     cx_testing_allocator_destroy(&talloc);
 }
 
-CX_TEST(test_hash_map_advanced_destructor) {
+CX_TEST(test_hash_map_advanced_destructor_pointers) {
     CxTestingAllocator talloc;
     cx_testing_allocator_init(&talloc);
     CxAllocator *allocator = &talloc.base;
@@ -674,6 +736,10 @@
     cx_test_register(suite, test_hash_map_clear);
     cx_test_register(suite, test_hash_map_store_ucx_strings);
     cx_test_register(suite, test_hash_map_remove_via_iterator);
+    cx_test_register(suite, test_hash_map_simple_destructor_objects);
+    cx_test_register(suite, test_hash_map_advanced_destructor_objects);
+    cx_test_register(suite, test_hash_map_simple_destructor_pointers);
+    cx_test_register(suite, test_hash_map_advanced_destructor_pointers);
     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);