From 3bee193141bdf3106732a2c925ffaf5ce48f0ecb Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 27 Nov 2018 22:25:40 +0900 Subject: [PATCH] hash-func: add destructors for key and value If they are set, then they are called in hashmap_clear() or hashmap_free(). (cherry picked from commit 59a5cda7b904cd7ef9853bda15b498bbc0577524) Resolves: #2037807 --- src/basic/hash-funcs.h | 54 ++++++++++++++++++++++++++--- src/basic/hashmap.c | 76 +++++++++++------------------------------ src/basic/hashmap.h | 50 ++++++++++++++++++--------- src/basic/ordered-set.h | 6 ++-- src/basic/set.h | 10 +++--- 5 files changed, 109 insertions(+), 87 deletions(-) diff --git a/src/basic/hash-funcs.h b/src/basic/hash-funcs.h index 2ff687e5f9..2d3125d0f9 100644 --- a/src/basic/hash-funcs.h +++ b/src/basic/hash-funcs.h @@ -1,7 +1,7 @@ /* SPDX-License-Identifier: LGPL-2.1+ */ #pragma once - +#include "alloc-util.h" #include "macro.h" #include "siphash24.h" @@ -11,21 +11,67 @@ typedef int (*compare_func_t)(const void *a, const void *b); struct hash_ops { hash_func_t hash; compare_func_t compare; + free_func_t free_key; + free_func_t free_value; }; -#define _DEFINE_HASH_OPS(uq, name, type, hash_func, compare_func, scope) \ +#define _DEFINE_HASH_OPS(uq, name, type, hash_func, compare_func, free_key_func, free_value_func, scope) \ _unused_ static void (* UNIQ_T(static_hash_wrapper, uq))(const type *, struct siphash *) = hash_func; \ _unused_ static int (* UNIQ_T(static_compare_wrapper, uq))(const type *, const type *) = compare_func; \ scope const struct hash_ops name = { \ .hash = (hash_func_t) hash_func, \ .compare = (compare_func_t) compare_func, \ + .free_key = free_key_func, \ + .free_value = free_value_func, \ + } + +#define _DEFINE_FREE_FUNC(uq, type, wrapper_name, func) \ + /* Type-safe free function */ \ + static void UNIQ_T(wrapper_name, uq)(void *a) { \ + type *_a = a; \ + func(_a); \ } +#define _DEFINE_HASH_OPS_WITH_KEY_DESTRUCTOR(uq, name, type, hash_func, compare_func, free_func, scope) \ + _DEFINE_FREE_FUNC(uq, type, static_free_wrapper, free_func); \ + _DEFINE_HASH_OPS(uq, name, type, hash_func, compare_func, \ + UNIQ_T(static_free_wrapper, uq), NULL, scope) + +#define _DEFINE_HASH_OPS_WITH_VALUE_DESTRUCTOR(uq, name, type, hash_func, compare_func, type_value, free_func, scope) \ + _DEFINE_FREE_FUNC(uq, type_value, static_free_wrapper, free_func); \ + _DEFINE_HASH_OPS(uq, name, type, hash_func, compare_func, \ + NULL, UNIQ_T(static_free_wrapper, uq), scope) + +#define _DEFINE_HASH_OPS_FULL(uq, name, type, hash_func, compare_func, free_key_func, type_value, free_value_func, scope) \ + _DEFINE_FREE_FUNC(uq, type, static_free_key_wrapper, free_key_func); \ + _DEFINE_FREE_FUNC(uq, type_value, static_free_value_wrapper, free_value_func); \ + _DEFINE_HASH_OPS(uq, name, type, hash_func, compare_func, \ + UNIQ_T(static_free_key_wrapper, uq), \ + UNIQ_T(static_free_value_wrapper, uq), scope) + #define DEFINE_HASH_OPS(name, type, hash_func, compare_func) \ - _DEFINE_HASH_OPS(UNIQ, name, type, hash_func, compare_func,) + _DEFINE_HASH_OPS(UNIQ, name, type, hash_func, compare_func, NULL, NULL,) #define DEFINE_PRIVATE_HASH_OPS(name, type, hash_func, compare_func) \ - _DEFINE_HASH_OPS(UNIQ, name, type, hash_func, compare_func, static) + _DEFINE_HASH_OPS(UNIQ, name, type, hash_func, compare_func, NULL, NULL, static) + +#define DEFINE_HASH_OPS_WITH_KEY_DESTRUCTOR(name, type, hash_func, compare_func, free_func) \ + _DEFINE_HASH_OPS_WITH_KEY_DESTRUCTOR(UNIQ, name, type, hash_func, compare_func, free_func,) + +#define DEFINE_PRIVATE_HASH_OPS_WITH_KEY_DESTRUCTOR(name, type, hash_func, compare_func, free_func) \ + _DEFINE_HASH_OPS_WITH_KEY_DESTRUCTOR(UNIQ, name, type, hash_func, compare_func, free_func, static) + +#define DEFINE_HASH_OPS_WITH_VALUE_DESTRUCTOR(name, type, hash_func, compare_func, value_type, free_func) \ + _DEFINE_HASH_OPS_WITH_VALUE_DESTRUCTOR(UNIQ, name, type, hash_func, compare_func, value_type, free_func,) + +#define DEFINE_PRIVATE_HASH_OPS_WITH_VALUE_DESTRUCTOR(name, type, hash_func, compare_func, value_type, free_func) \ + _DEFINE_HASH_OPS_WITH_VALUE_DESTRUCTOR(UNIQ, name, type, hash_func, compare_func, value_type, free_func, static) + +#define DEFINE_HASH_OPS_FULL(name, type, hash_func, compare_func, free_key_func, value_type, free_value_func) \ + _DEFINE_HASH_OPS_FULL(UNIQ, name, type, hash_func, compare_func, free_key_func, value_type, free_value_func,) + +#define DEFINE_PRIVATE_HASH_OPS_FULL(name, type, hash_func, compare_func, free_key_func, value_type, free_value_func) \ + _DEFINE_HASH_OPS_FULL(UNIQ, name, type, hash_func, compare_func, free_key_func, value_type, free_value_func, static) void string_hash_func(const void *p, struct siphash *state); int string_compare_func(const void *a, const void *b) _pure_; diff --git a/src/basic/hashmap.c b/src/basic/hashmap.c index 69a7d70b04..7c508086f0 100644 --- a/src/basic/hashmap.c +++ b/src/basic/hashmap.c @@ -863,47 +863,38 @@ static void hashmap_free_no_clear(HashmapBase *h) { 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) { +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; - /* Free the hashmap and all data objects in it, but not the - * keys */ + free_key = h->hash_ops->free_key ?: default_free_key; + free_value = h->hash_ops->free_value ?: default_free_value; - if (h) { - internal_hashmap_clear_free(h); - hashmap_free_no_clear(h); - } - - return NULL; -} + if (free_key || free_value) { + unsigned idx; -Hashmap *hashmap_free_free_free(Hashmap *h) { + for (idx = skip_free_buckets(h, 0); idx != IDX_NIL; + idx = skip_free_buckets(h, idx + 1)) { + struct hashmap_base_entry *e = bucket_at(h, idx); - /* Free the hashmap and all data and key objects in it */ + if (free_key) + free_key((void *) e->key); - if (h) { - hashmap_clear_free_free(h); - hashmap_free_no_clear(HASHMAP_BASE(h)); + if (free_value) + free_value(entry_value(h, e)); + } } - return NULL; -} - -void internal_hashmap_clear(HashmapBase *h) { - if (!h) - return; - if (h->has_indirect) { free(h->indirect.storage); h->has_indirect = false; @@ -920,35 +911,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); /* @@ -1771,7 +1733,7 @@ HashmapBase *internal_hashmap_copy(HashmapBase *h) { } if (r < 0) { - internal_hashmap_free(copy); + internal_hashmap_free(copy, false, false); return NULL; } diff --git a/src/basic/hashmap.h b/src/basic/hashmap.h index 5c70c102d7..9e4772b497 100644 --- a/src/basic/hashmap.h +++ b/src/basic/hashmap.h @@ -23,6 +23,8 @@ #define HASH_KEY_SIZE 16 +typedef void* (*hashmap_destroy_t)(void *p); + /* The base type for all hashmap and set types. Many functions in the * implementation take (HashmapBase*) parameters and are run-time polymorphic, * though the API is not meant to be polymorphic (do not call functions @@ -88,25 +90,33 @@ OrderedHashmap *internal_ordered_hashmap_new(const struct hash_ops *hash_ops HA #define hashmap_new(ops) internal_hashmap_new(ops HASHMAP_DEBUG_SRC_ARGS) #define ordered_hashmap_new(ops) internal_ordered_hashmap_new(ops HASHMAP_DEBUG_SRC_ARGS) -HashmapBase *internal_hashmap_free(HashmapBase *h); +HashmapBase *internal_hashmap_free(HashmapBase *h, free_func_t default_free_key, free_func_t default_free_value); static inline Hashmap *hashmap_free(Hashmap *h) { - return (void*)internal_hashmap_free(HASHMAP_BASE(h)); + return (void*) internal_hashmap_free(HASHMAP_BASE(h), NULL, NULL); } static inline OrderedHashmap *ordered_hashmap_free(OrderedHashmap *h) { - return (void*)internal_hashmap_free(HASHMAP_BASE(h)); + return (void*) internal_hashmap_free(HASHMAP_BASE(h), NULL, NULL); } -HashmapBase *internal_hashmap_free_free(HashmapBase *h); static inline Hashmap *hashmap_free_free(Hashmap *h) { - return (void*)internal_hashmap_free_free(HASHMAP_BASE(h)); + return (void*) internal_hashmap_free(HASHMAP_BASE(h), NULL, free); } static inline OrderedHashmap *ordered_hashmap_free_free(OrderedHashmap *h) { - return (void*)internal_hashmap_free_free(HASHMAP_BASE(h)); + return (void*) internal_hashmap_free(HASHMAP_BASE(h), NULL, free); } -Hashmap *hashmap_free_free_free(Hashmap *h); +static inline Hashmap *hashmap_free_free_key(Hashmap *h) { + return (void*) internal_hashmap_free(HASHMAP_BASE(h), free, NULL); +} +static inline OrderedHashmap *ordered_hashmap_free_free_key(OrderedHashmap *h) { + return (void*) internal_hashmap_free(HASHMAP_BASE(h), free, NULL); +} + +static inline Hashmap *hashmap_free_free_free(Hashmap *h) { + return (void*) internal_hashmap_free(HASHMAP_BASE(h), free, free); +} static inline OrderedHashmap *ordered_hashmap_free_free_free(OrderedHashmap *h) { - return (void*)hashmap_free_free_free(PLAIN_HASHMAP(h)); + return (void*) internal_hashmap_free(HASHMAP_BASE(h), free, free); } IteratedCache *iterated_cache_free(IteratedCache *cache); @@ -259,25 +269,33 @@ static inline bool ordered_hashmap_iterate(OrderedHashmap *h, Iterator *i, void return internal_hashmap_iterate(HASHMAP_BASE(h), i, value, key); } -void internal_hashmap_clear(HashmapBase *h); +void internal_hashmap_clear(HashmapBase *h, free_func_t default_free_key, free_func_t default_free_value); static inline void hashmap_clear(Hashmap *h) { - internal_hashmap_clear(HASHMAP_BASE(h)); + internal_hashmap_clear(HASHMAP_BASE(h), NULL, NULL); } static inline void ordered_hashmap_clear(OrderedHashmap *h) { - internal_hashmap_clear(HASHMAP_BASE(h)); + internal_hashmap_clear(HASHMAP_BASE(h), NULL, NULL); } -void internal_hashmap_clear_free(HashmapBase *h); static inline void hashmap_clear_free(Hashmap *h) { - internal_hashmap_clear_free(HASHMAP_BASE(h)); + internal_hashmap_clear(HASHMAP_BASE(h), NULL, free); } static inline void ordered_hashmap_clear_free(OrderedHashmap *h) { - internal_hashmap_clear_free(HASHMAP_BASE(h)); + internal_hashmap_clear(HASHMAP_BASE(h), NULL, free); } -void hashmap_clear_free_free(Hashmap *h); +static inline void hashmap_clear_free_key(Hashmap *h) { + internal_hashmap_clear(HASHMAP_BASE(h), free, NULL); +} +static inline void ordered_hashmap_clear_free_key(OrderedHashmap *h) { + internal_hashmap_clear(HASHMAP_BASE(h), free, NULL); +} + +static inline void hashmap_clear_free_free(Hashmap *h) { + internal_hashmap_clear(HASHMAP_BASE(h), free, free); +} static inline void ordered_hashmap_clear_free_free(OrderedHashmap *h) { - hashmap_clear_free_free(PLAIN_HASHMAP(h)); + internal_hashmap_clear(HASHMAP_BASE(h), free, free); } /* diff --git a/src/basic/ordered-set.h b/src/basic/ordered-set.h index e7c054d8e4..7cbb71819b 100644 --- a/src/basic/ordered-set.h +++ b/src/basic/ordered-set.h @@ -21,13 +21,11 @@ static inline int ordered_set_ensure_allocated(OrderedSet **s, const struct hash } static inline OrderedSet* ordered_set_free(OrderedSet *s) { - ordered_hashmap_free((OrderedHashmap*) s); - return NULL; + return (OrderedSet*) ordered_hashmap_free((OrderedHashmap*) s); } static inline OrderedSet* ordered_set_free_free(OrderedSet *s) { - ordered_hashmap_free_free((OrderedHashmap*) s); - return NULL; + return (OrderedSet*) ordered_hashmap_free_free((OrderedHashmap*) s); } static inline int ordered_set_put(OrderedSet *s, void *p) { diff --git a/src/basic/set.h b/src/basic/set.h index 664713810d..8e12670a6e 100644 --- a/src/basic/set.h +++ b/src/basic/set.h @@ -9,13 +9,11 @@ Set *internal_set_new(const struct hash_ops *hash_ops HASHMAP_DEBUG_PARAMS); #define set_new(ops) internal_set_new(ops HASHMAP_DEBUG_SRC_ARGS) static inline Set *set_free(Set *s) { - internal_hashmap_free(HASHMAP_BASE(s)); - return NULL; + return (Set*) internal_hashmap_free(HASHMAP_BASE(s), NULL, NULL); } static inline Set *set_free_free(Set *s) { - internal_hashmap_free_free(HASHMAP_BASE(s)); - return NULL; + return (Set*) internal_hashmap_free(HASHMAP_BASE(s), free, NULL); } /* no set_free_free_free */ @@ -76,11 +74,11 @@ static inline unsigned set_buckets(Set *s) { bool set_iterate(Set *s, Iterator *i, void **value); static inline void set_clear(Set *s) { - internal_hashmap_clear(HASHMAP_BASE(s)); + internal_hashmap_clear(HASHMAP_BASE(s), NULL, NULL); } static inline void set_clear_free(Set *s) { - internal_hashmap_clear_free(HASHMAP_BASE(s)); + internal_hashmap_clear(HASHMAP_BASE(s), free, NULL); } /* no set_clear_free_free */