]> git.ipfire.org Git - thirdparty/systemd.git/blobdiff - src/basic/hashmap.c
hashmap: rework hashmap_clear() to be more defensive
[thirdparty/systemd.git] / src / basic / hashmap.c
index 784fbe31062aa22a68415a29f145f39d562ea67f..0dd9f8ddd426aa6da69b8ed12c0da2bc038a866d 100644 (file)
@@ -1,7 +1,4 @@
 /* SPDX-License-Identifier: LGPL-2.1+ */
-/***
-  Copyright 2014 Michal Schmidt
-***/
 
 #include <errno.h>
 #include <stdint.h>
@@ -9,8 +6,8 @@
 #include <string.h>
 
 #include "alloc-util.h"
-#include "hashmap.h"
 #include "fileio.h"
+#include "hashmap.h"
 #include "macro.h"
 #include "mempool.h"
 #include "process-util.h"
@@ -279,7 +276,7 @@ static const struct hashmap_type_info hashmap_type_info[_HASHMAP_TYPE_MAX] = {
 };
 
 #if VALGRIND
-__attribute__((destructor)) static void cleanup_pools(void) {
+_destructor_ static void cleanup_pools(void) {
         _cleanup_free_ char *t = NULL;
         int r;
 
@@ -772,17 +769,16 @@ static void reset_direct_storage(HashmapBase *h) {
 static struct HashmapBase *hashmap_base_new(const struct hash_ops *hash_ops, enum HashmapType type HASHMAP_DEBUG_PARAMS) {
         HashmapBase *h;
         const struct hashmap_type_info *hi = &hashmap_type_info[type];
-        bool use_pool;
-
-        use_pool = is_main_thread();
+        bool up;
 
-        h = use_pool ? mempool_alloc0_tile(hi->mempool) : malloc0(hi->head_size);
+        up = mempool_enabled();
 
+        h = up ? mempool_alloc0_tile(hi->mempool) : malloc0(hi->head_size);
         if (!h)
                 return NULL;
 
         h->type = type;
-        h->from_pool = use_pool;
+        h->from_pool = up;
         h->hash_ops = hash_ops ? hash_ops : &trivial_hash_ops;
 
         if (type == HASHMAP_TYPE_ORDERED) {
@@ -852,7 +848,7 @@ int internal_set_ensure_allocated(Set **s, const struct hash_ops *hash_ops  HASH
 
 static void hashmap_free_no_clear(HashmapBase *h) {
         assert(!h->has_indirect);
-        assert(!h->n_direct_entries);
+        assert(h->n_direct_entries == 0);
 
 #if ENABLE_DEBUG_HASHMAP
         assert_se(pthread_mutex_lock(&hashmap_debug_list_mutex) == 0);
@@ -860,52 +856,49 @@ static void hashmap_free_no_clear(HashmapBase *h) {
         assert_se(pthread_mutex_unlock(&hashmap_debug_list_mutex) == 0);
 #endif
 
-        if (h->from_pool)
+        if (h->from_pool) {
+                /* Ensure that the object didn't get migrated between threads. */
+                assert_se(is_main_thread());
                 mempool_free_tile(hashmap_type_info[h->type].mempool, h);
-        else
+        else
                 free(h);
 }
 
-HashmapBase *internal_hashmap_free(HashmapBase *h) {
-
-        /* Free the hashmap, but nothing in it */
-
+HashmapBase *internal_hashmap_free(HashmapBase *h, free_func_t default_free_key, free_func_t default_free_value) {
         if (h) {
-                internal_hashmap_clear(h);
+                internal_hashmap_clear(h, default_free_key, default_free_value);
                 hashmap_free_no_clear(h);
         }
 
         return NULL;
 }
 
-HashmapBase *internal_hashmap_free_free(HashmapBase *h) {
-
-        /* Free the hashmap and all data objects in it, but not the
-         * keys */
+void internal_hashmap_clear(HashmapBase *h, free_func_t default_free_key, free_func_t default_free_value) {
+        free_func_t free_key, free_value;
+        if (!h)
+                return;
 
-        if (h) {
-                internal_hashmap_clear_free(h);
-                hashmap_free_no_clear(h);
-        }
+        free_key = h->hash_ops->free_key ?: default_free_key;
+        free_value = h->hash_ops->free_value ?: default_free_value;
 
-        return NULL;
-}
+        if (free_key || free_value) {
 
-Hashmap *hashmap_free_free_free(Hashmap *h) {
+                /* If destructor calls are defined, let's destroy things defensively: let's take the item out of the
+                 * hash table, and only then call the destructor functions. If these destructors then try to unregister
+                 * themselves from our hash table a second time, the entry is already gone. */
 
-        /* Free the hashmap and all data and key objects in it */
+                while (internal_hashmap_size(h) > 0) {
+                        void *v, *k;
 
-        if (h) {
-                hashmap_clear_free_free(h);
-                hashmap_free_no_clear(HASHMAP_BASE(h));
-        }
+                        v = internal_hashmap_first_key_and_value(h, true, &k);
 
-        return NULL;
-}
+                        if (free_key)
+                                free_key(k);
 
-void internal_hashmap_clear(HashmapBase *h) {
-        if (!h)
-                return;
+                        if (free_value)
+                                free_value(v);
+                }
+        }
 
         if (h->has_indirect) {
                 free(h->indirect.storage);
@@ -923,35 +916,6 @@ void internal_hashmap_clear(HashmapBase *h) {
         base_set_dirty(h);
 }
 
-void internal_hashmap_clear_free(HashmapBase *h) {
-        unsigned idx;
-
-        if (!h)
-                return;
-
-        for (idx = skip_free_buckets(h, 0); idx != IDX_NIL;
-             idx = skip_free_buckets(h, idx + 1))
-                free(entry_value(h, bucket_at(h, idx)));
-
-        internal_hashmap_clear(h);
-}
-
-void hashmap_clear_free_free(Hashmap *h) {
-        unsigned idx;
-
-        if (!h)
-                return;
-
-        for (idx = skip_free_buckets(HASHMAP_BASE(h), 0); idx != IDX_NIL;
-             idx = skip_free_buckets(HASHMAP_BASE(h), idx + 1)) {
-                struct plain_hashmap_entry *e = plain_bucket_at(h, idx);
-                free((void*)e->b.key);
-                free(e->value);
-        }
-
-        internal_hashmap_clear(HASHMAP_BASE(h));
-}
-
 static int resize_buckets(HashmapBase *h, unsigned entries_add);
 
 /*
@@ -1515,8 +1479,8 @@ int hashmap_remove_and_replace(Hashmap *h, const void *old_key, const void *new_
         return 0;
 }
 
-void *hashmap_remove_value(Hashmap *h, const void *key, void *value) {
-        struct plain_hashmap_entry *e;
+void *internal_hashmap_remove_value(HashmapBase *h, const void *key, void *value) {
+        struct hashmap_base_entry *e;
         unsigned hash, idx;
 
         if (!h)
@@ -1527,8 +1491,8 @@ void *hashmap_remove_value(Hashmap *h, const void *key, void *value) {
         if (idx == IDX_NIL)
                 return NULL;
 
-        e = plain_bucket_at(h, idx);
-        if (e->value != value)
+        e = bucket_at(h, idx);
+        if (entry_value(h, e) != value)
                 return NULL;
 
         remove_entry(h, idx);
@@ -1545,18 +1509,9 @@ static unsigned find_first_entry(HashmapBase *h) {
         return hashmap_iterate_entry(h, &i);
 }
 
-void *internal_hashmap_first(HashmapBase *h) {
-        unsigned idx;
-
-        idx = find_first_entry(h);
-        if (idx == IDX_NIL)
-                return NULL;
-
-        return entry_value(h, bucket_at(h, idx));
-}
-
-void *internal_hashmap_first_key(HashmapBase *h) {
+void *internal_hashmap_first_key_and_value(HashmapBase *h, bool remove, void **ret_key) {
         struct hashmap_base_entry *e;
+        void *key, *data;
         unsigned idx;
 
         idx = find_first_entry(h);
@@ -1564,39 +1519,16 @@ void *internal_hashmap_first_key(HashmapBase *h) {
                 return NULL;
 
         e = bucket_at(h, idx);
-        return (void*) e->key;
-}
-
-void *internal_hashmap_steal_first(HashmapBase *h) {
-        struct hashmap_base_entry *e;
-        void *data;
-        unsigned idx;
-
-        idx = find_first_entry(h);
-        if (idx == IDX_NIL)
-                return NULL;
-
-        e = bucket_at(h, idx);
+        key = (void*) e->key;
         data = entry_value(h, e);
-        remove_entry(h, idx);
 
-        return data;
-}
+        if (remove)
+                remove_entry(h, idx);
 
-void *internal_hashmap_steal_first_key(HashmapBase *h) {
-        struct hashmap_base_entry *e;
-        void *key;
-        unsigned idx;
+        if (ret_key)
+                *ret_key = key;
 
-        idx = find_first_entry(h);
-        if (idx == IDX_NIL)
-                return NULL;
-
-        e = bucket_at(h, idx);
-        key = (void*) e->key;
-        remove_entry(h, idx);
-
-        return key;
+        return data;
 }
 
 unsigned internal_hashmap_size(HashmapBase *h) {
@@ -1774,7 +1706,7 @@ HashmapBase *internal_hashmap_copy(HashmapBase *h) {
         }
 
         if (r < 0) {
-                internal_hashmap_free(copy);
+                internal_hashmap_free(copy, false, false);
                 return NULL;
         }