add destructor functions for maps - fixes #253

Tue, 18 Apr 2023 19:10:45 +0200

author
Mike Becker <universe@uap-core.de>
date
Tue, 18 Apr 2023 19:10:45 +0200
changeset 686
64919f63f059
parent 685
2dd841e364af
child 687
612ed521b1c5

add destructor functions for maps - fixes #253

src/cx/map.h file | annotate | diff | comparison | revisions
src/hash_map.c file | annotate | diff | comparison | revisions
tests/test_map.cpp file | annotate | diff | comparison | revisions
--- a/src/cx/map.h	Tue Apr 18 18:01:41 2023 +0200
+++ b/src/cx/map.h	Tue Apr 18 19:10:45 2023 +0200
@@ -102,7 +102,8 @@
     __attribute__((__nonnull__))
     void *(*remove)(
             CxMap *map,
-            CxHashKey key
+            CxHashKey key,
+            bool destroy
     );
 
     /**
@@ -196,7 +197,6 @@
  */
 __attribute__((__nonnull__))
 static inline void cxMapDestroy(CxMap *map) {
-    // TODO: likely to add auto-free feature for contents in the future
     map->cl->destructor(map);
 }
 
@@ -246,42 +246,72 @@
 /**
  * Removes a key/value-pair from the map by using the key.
  *
- * If this map is storing pointers, you should make sure that the map
- * is not the last location where this pointer is stored.
- * Otherwise, use cxMapRemoveAndGet() to retrieve the pointer while
- * removing it from the map.
+ * Always invokes the destructor function, if any, on the removed element.
+ * If this map is storing pointers and you just want to retrieve the pointer
+ * without invoking the destructor, use cxMapRemoveAndGet().
+ * If you just want to detach the element from the map without invoking the
+ * destructor or returning the element, use cxMapDetach().
  *
  * @param map the map
  * @param key the key
  * @see cxMapRemoveAndGet()
+ * @see cxMapDetach()
  */
 __attribute__((__nonnull__))
 static inline void cxMapRemove(
         CxMap *map,
         CxHashKey key
 ) {
-    (void) map->cl->remove(map, key);
+    (void) map->cl->remove(map, key, true);
+}
+
+/**
+ * Detaches a key/value-pair from the map by using the key
+ * without invoking the destructor.
+ *
+ * In general, you should only use this function if the map does not own
+ * the data and there is a valid reference to the data somewhere else
+ * in the program. In all other cases it is prefarable to use
+ * cxMapRemove() or cxMapRemoveAndGet().
+ *
+ * @param map the map
+ * @param key the key
+ * @see cxMapRemove()
+ * @see cxMapRemoveAndGet()
+ */
+__attribute__((__nonnull__))
+static inline void cxMapDetach(
+        CxMap *map,
+        CxHashKey key
+) {
+    (void) map->cl->remove(map, key, false);
 }
 
 /**
  * Removes a key/value-pair from the map by using the key.
  *
- * This function should only be used when the map is storing pointers,
- * in order to retrieve the pointer you are about to remove.
- * In any other case, cxMapRemove() is sufficient.
+ * This function can be used when the map is storing pointers,
+ * in order to retrieve the pointer from the map without invoking
+ * any destructor function. Sometimes you do not want the pointer
+ * to be returned - in that case (instead of suppressing the "unused
+ * result" warning) you can use cxMapDetach().
+ *
+ * If this map is not storing pointers, this function behaves like
+ * cxMapRemove() and returns \c NULL.
  *
  * @param map the map
  * @param key the key
  * @return the stored pointer or \c NULL if either the key is not present
  * in the map or the map is not storing pointers
  * @see cxMapStorePointers()
+ * @see cxMapDetach()
  */
 __attribute__((__nonnull__, __warn_unused_result__))
 static inline void *cxMapRemoveAndGet(
         CxMap *map,
         CxHashKey key
 ) {
-    return map->cl->remove(map, key);
+    return map->cl->remove(map, key, !map->store_pointer);
 }
 
 // TODO: set-like map operations (union, intersect, difference)
--- a/src/hash_map.c	Tue Apr 18 18:01:41 2023 +0200
+++ b/src/hash_map.c	Tue Apr 18 19:10:45 2023 +0200
@@ -48,6 +48,8 @@
         if (elem != NULL) {
             do {
                 struct cx_hash_map_element_s *next = elem->next;
+                // invoke the destructor
+                cx_invoke_destructor(map, elem->data);
                 // free the key data
                 cxFree(map->allocator, elem->key.data.obj);
                 // free the node
@@ -80,7 +82,7 @@
         void *value
 ) {
     struct cx_hash_map_s *hash_map = (struct cx_hash_map_s *) map;
-    CxAllocator *allocator = map->allocator;
+    CxAllocator const *allocator = map->allocator;
 
     unsigned hash = key.hash;
     if (hash == 0) {
@@ -172,12 +174,14 @@
  * @param map the map
  * @param key the key to look up
  * @param remove flag indicating whether the looked up entry shall be removed
+ * @param destroy flag indicating whether the destructor shall be invoked
  * @return a pointer to the value corresponding to the key or \c NULL
  */
 static void *cx_hash_map_get_remove(
         CxMap *map,
         CxHashKey key,
-        bool remove
+        bool remove,
+        bool destroy
 ) {
     struct cx_hash_map_s *hash_map = (struct cx_hash_map_s *) map;
 
@@ -194,10 +198,14 @@
         if (elm->key.hash == hash && elm->key.len == key.len) {
             if (memcmp(elm->key.data.obj, key.data.obj, key.len) == 0) {
                 void *data = NULL;
-                if (map->store_pointer) {
-                    data = *(void **) elm->data;
-                } else if (!remove) {
-                    data = elm->data;
+                if (destroy) {
+                    cx_invoke_destructor(map, elm->data);
+                } else {
+                    if (map->store_pointer) {
+                        data = *(void **) elm->data;
+                    } else {
+                        data = elm->data;
+                    }
                 }
                 if (remove) {
                     cx_hash_map_unlink(hash_map, slot, prev, elm);
@@ -217,14 +225,15 @@
         CxHashKey key
 ) {
     // we can safely cast, because we know when remove=false, the map stays untouched
-    return cx_hash_map_get_remove((CxMap *) map, key, false);
+    return cx_hash_map_get_remove((CxMap *) map, key, false, false);
 }
 
 static void *cx_hash_map_remove(
         CxMap *map,
-        CxHashKey key
+        CxHashKey key,
+        bool destroy
 ) {
-    return cx_hash_map_get_remove(map, key, true);
+    return cx_hash_map_get_remove(map, key, true, destroy);
 }
 
 static void *cx_hash_map_iter_current_entry(void const *it) {
@@ -280,6 +289,9 @@
             }
         }
 
+        // destroy
+        cx_invoke_destructor((struct cx_map_s *) map, elm->data);
+
         // unlink
         cx_hash_map_unlink(map, iter->slot, prev, elm);
 
--- a/tests/test_map.cpp	Tue Apr 18 18:01:41 2023 +0200
+++ b/tests/test_map.cpp	Tue Apr 18 19:10:45 2023 +0200
@@ -282,7 +282,7 @@
 TEST(CxHashMap, Clear) {
     CxTestingAllocator allocator;
     auto map = cxHashMapCreate(&allocator, CX_STORE_POINTERS, 0);
-    
+
     cxMapPut(map, cx_hash_key_str("key 1"), (void *) "val 1");
     cxMapPut(map, cx_hash_key_str("key 2"), (void *) "val 2");
     cxMapPut(map, cx_hash_key_str("key 3"), (void *) "val 3");
@@ -344,3 +344,100 @@
     EXPECT_TRUE(allocator.verify());
 }
 
+static void test_simple_destructor(void *data) {
+    strcpy((char *) data, "OK");
+}
+
+static void test_advanced_destructor(
+        [[maybe_unused]] void *unused,
+        void *data
+) {
+    strcpy((char *) data, "OK");
+}
+
+static void verify_any_destructor(CxMap *map) {
+    auto k1 = cx_hash_key_str("key 1");
+    auto k2 = cx_hash_key_str("key 2");
+    auto k3 = cx_hash_key_str("key 3");
+    auto k4 = cx_hash_key_str("key 4");
+    auto k5 = cx_hash_key_str("key 5");
+
+    char v1[] = "val 1";
+    char v2[] = "val 2";
+    char v3[] = "val 3";
+    char v4[] = "val 4";
+    char v5[] = "val 5";
+
+    cxMapPut(map, k1, (void *) v1);
+    cxMapPut(map, k2, (void *) v2);
+    cxMapPut(map, k3, (void *) v3);
+    cxMapPut(map, k4, (void *) v4);
+
+    cxMapRemove(map, k2);
+    auto r = cxMapRemoveAndGet(map, k3);
+    cxMapDetach(map, k1);
+
+    EXPECT_STREQ(v1, "val 1");
+    EXPECT_STREQ(v2, "OK");
+    EXPECT_STREQ(v3, "val 3");
+    EXPECT_STREQ(v4, "val 4");
+    EXPECT_STREQ(v5, "val 5");
+    EXPECT_EQ(r, v3);
+
+    cxMapClear(map);
+
+    EXPECT_STREQ(v1, "val 1");
+    EXPECT_STREQ(v2, "OK");
+    EXPECT_STREQ(v3, "val 3");
+    EXPECT_STREQ(v4, "OK");
+    EXPECT_STREQ(v5, "val 5");
+
+    cxMapPut(map, k1, (void *) v1);
+    cxMapPut(map, k3, (void *) v3);
+    cxMapPut(map, k5, (void *) v5);
+
+    {
+        auto iter = cxMapMutIteratorKeys(map);
+        cx_foreach(CxHashKey*, key, iter) {
+            if (key->data.cstr[4] == '1') cxIteratorFlagRemoval(iter);
+        }
+    }
+    {
+        auto iter = cxMapMutIteratorValues(map);
+        cx_foreach(char*, v, iter) {
+            if (v[4] == '5') cxIteratorFlagRemoval(iter);
+        }
+    }
+
+    EXPECT_STREQ(v1, "OK");
+    EXPECT_STREQ(v2, "OK");
+    EXPECT_STREQ(v3, "val 3");
+    EXPECT_STREQ(v4, "OK");
+    EXPECT_STREQ(v5, "OK");
+
+    v1[0] = v2[0] = v4[0] = v5[0] = 'c';
+
+    cxMapDestroy(map);
+
+    EXPECT_STREQ(v1, "cK");
+    EXPECT_STREQ(v2, "cK");
+    EXPECT_STREQ(v3, "OK");
+    EXPECT_STREQ(v4, "cK");
+    EXPECT_STREQ(v5, "cK");
+}
+
+TEST(CxHashMap, SimpleDestructor) {
+    CxTestingAllocator allocator;
+    auto map = cxHashMapCreate(&allocator, CX_STORE_POINTERS, 0);
+    map->simple_destructor = test_simple_destructor;
+    verify_any_destructor(map);
+    EXPECT_TRUE(allocator.verify());
+}
+
+TEST(CxHashMap, AdvancedDestructor) {
+    CxTestingAllocator allocator;
+    auto map = cxHashMapCreate(&allocator, CX_STORE_POINTERS, 0);
+    map->advanced_destructor = test_advanced_destructor;
+    verify_any_destructor(map);
+    EXPECT_TRUE(allocator.verify());
+}

mercurial