Tue, 18 Apr 2023 19:10:45 +0200
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()); +}