Skip to content

Commit 2a6ca54

Browse files
yuwatayuwata
authored
hashmap: kill hashmap_free_with_destructor() and friends (systemd#37111)
Now destructor is always set in hash_ops when necessary. Hence, hashmap_free_with_destructor() and friends are not necessary anymore. Let's kill them.
2 parents 39dd06d + bdf4f20 commit 2a6ca54

File tree

10 files changed

+25
-127
lines changed

10 files changed

+25
-127
lines changed

‎src/basic/hashmap.c‎

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -912,24 +912,20 @@ static void hashmap_free_no_clear(HashmapBase *h) {
912912
free(h);
913913
}
914914

915-
HashmapBase* _hashmap_free(HashmapBase *h, free_func_t default_free_key, free_func_t default_free_value) {
915+
HashmapBase* _hashmap_free(HashmapBase *h) {
916916
if (h) {
917-
_hashmap_clear(h, default_free_key, default_free_value);
917+
_hashmap_clear(h);
918918
hashmap_free_no_clear(h);
919919
}
920920

921921
return NULL;
922922
}
923923

924-
void _hashmap_clear(HashmapBase *h, free_func_t default_free_key, free_func_t default_free_value) {
925-
free_func_t free_key, free_value;
924+
void _hashmap_clear(HashmapBase *h) {
926925
if (!h)
927926
return;
928927

929-
free_key = h->hash_ops->free_key ?: default_free_key;
930-
free_value = h->hash_ops->free_value ?: default_free_value;
931-
932-
if (free_key || free_value) {
928+
if (h->hash_ops->free_key || h->hash_ops->free_value) {
933929

934930
/* If destructor calls are defined, let's destroy things defensively: let's take the item out of the
935931
* hash table, and only then call the destructor functions. If these destructors then try to unregister
@@ -941,11 +937,11 @@ void _hashmap_clear(HashmapBase *h, free_func_t default_free_key, free_func_t de
941937

942938
v = _hashmap_first_key_and_value(h, true, &k);
943939

944-
if (free_key)
945-
free_key(k);
940+
if (h->hash_ops->free_key)
941+
h->hash_ops->free_key(k);
946942

947-
if (free_value)
948-
free_value(v);
943+
if (h->hash_ops->free_value)
944+
h->hash_ops->free_value(v);
949945
}
950946
}
951947

@@ -1780,7 +1776,7 @@ HashmapBase* _hashmap_copy(HashmapBase *h HASHMAP_DEBUG_PARAMS) {
17801776
}
17811777

17821778
if (r < 0)
1783-
return _hashmap_free(copy, NULL, NULL);
1779+
return _hashmap_free(copy);
17841780

17851781
return copy;
17861782
}

‎src/basic/hashmap.h‎

Lines changed: 6 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -93,12 +93,12 @@ OrderedHashmap* _ordered_hashmap_new(const struct hash_ops *hash_ops HASHMAP_DE
9393
#define ordered_hashmap_free_and_replace(a, b) \
9494
free_and_replace_full(a, b, ordered_hashmap_free)
9595

96-
HashmapBase* _hashmap_free(HashmapBase *h, free_func_t default_free_key, free_func_t default_free_value);
96+
HashmapBase* _hashmap_free(HashmapBase *h);
9797
static inline Hashmap* hashmap_free(Hashmap *h) {
98-
return (void*) _hashmap_free(HASHMAP_BASE(h), NULL, NULL);
98+
return (void*) _hashmap_free(HASHMAP_BASE(h));
9999
}
100100
static inline OrderedHashmap* ordered_hashmap_free(OrderedHashmap *h) {
101-
return (void*) _hashmap_free(HASHMAP_BASE(h), NULL, NULL);
101+
return (void*) _hashmap_free(HASHMAP_BASE(h));
102102
}
103103

104104
IteratedCache* iterated_cache_free(IteratedCache *cache);
@@ -266,12 +266,12 @@ static inline bool ordered_hashmap_iterate(OrderedHashmap *h, Iterator *i, void
266266
return _hashmap_iterate(HASHMAP_BASE(h), i, value, key);
267267
}
268268

269-
void _hashmap_clear(HashmapBase *h, free_func_t default_free_key, free_func_t default_free_value);
269+
void _hashmap_clear(HashmapBase *h);
270270
static inline void hashmap_clear(Hashmap *h) {
271-
_hashmap_clear(HASHMAP_BASE(h), NULL, NULL);
271+
_hashmap_clear(HASHMAP_BASE(h));
272272
}
273273
static inline void ordered_hashmap_clear(OrderedHashmap *h) {
274-
_hashmap_clear(HASHMAP_BASE(h), NULL, NULL);
274+
_hashmap_clear(HASHMAP_BASE(h));
275275
}
276276

277277
/*
@@ -331,27 +331,6 @@ static inline void *ordered_hashmap_first_key(OrderedHashmap *h) {
331331
return _hashmap_first_key(HASHMAP_BASE(h), false);
332332
}
333333

334-
#define hashmap_clear_with_destructor(h, f) \
335-
({ \
336-
Hashmap *_h = (h); \
337-
void *_item; \
338-
while ((_item = hashmap_steal_first(_h))) \
339-
f(_item); \
340-
_h; \
341-
})
342-
#define hashmap_free_with_destructor(h, f) \
343-
hashmap_free(hashmap_clear_with_destructor(h, f))
344-
#define ordered_hashmap_clear_with_destructor(h, f) \
345-
({ \
346-
OrderedHashmap *_h = (h); \
347-
void *_item; \
348-
while ((_item = ordered_hashmap_steal_first(_h))) \
349-
f(_item); \
350-
_h; \
351-
})
352-
#define ordered_hashmap_free_with_destructor(h, f) \
353-
ordered_hashmap_free(ordered_hashmap_clear_with_destructor(h, f))
354-
355334
/* no hashmap_next */
356335
void* ordered_hashmap_next(OrderedHashmap *h, const void *key);
357336

‎src/basic/ordered-set.h‎

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -83,17 +83,6 @@ void ordered_set_print(FILE *f, const char *field, OrderedSet *s);
8383
#define ORDERED_SET_FOREACH(e, s) \
8484
_ORDERED_SET_FOREACH(e, s, UNIQ_T(i, UNIQ))
8585

86-
#define ordered_set_clear_with_destructor(s, f) \
87-
({ \
88-
OrderedSet *_s = (s); \
89-
void *_item; \
90-
while ((_item = ordered_set_steal_first(_s))) \
91-
f(_item); \
92-
_s; \
93-
})
94-
#define ordered_set_free_with_destructor(s, f) \
95-
ordered_set_free(ordered_set_clear_with_destructor(s, f))
96-
9786
DEFINE_TRIVIAL_CLEANUP_FUNC(OrderedSet*, ordered_set_free);
9887

9988
#define _cleanup_ordered_set_free_ _cleanup_(ordered_set_freep)

‎src/basic/set.h‎

Lines changed: 2 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -12,15 +12,9 @@ Set* _set_new(const struct hash_ops *hash_ops HASHMAP_DEBUG_PARAMS);
1212
#define set_new(ops) _set_new(ops HASHMAP_DEBUG_SRC_ARGS)
1313

1414
static inline Set* set_free(Set *s) {
15-
return (Set*) _hashmap_free(HASHMAP_BASE(s), NULL, NULL);
15+
return (Set*) _hashmap_free(HASHMAP_BASE(s));
1616
}
1717

18-
static inline Set* set_free_free(Set *s) {
19-
return (Set*) _hashmap_free(HASHMAP_BASE(s), free, NULL);
20-
}
21-
22-
/* no set_free_free_free */
23-
2418
#define set_copy(s) ((Set*) _hashmap_copy(HASHMAP_BASE(s) HASHMAP_DEBUG_SRC_ARGS))
2519

2620
int _set_ensure_allocated(Set **s, const struct hash_ops *hash_ops HASHMAP_DEBUG_PARAMS);
@@ -77,30 +71,13 @@ static inline bool set_iterate(const Set *s, Iterator *i, void **value) {
7771
}
7872

7973
static inline void set_clear(Set *s) {
80-
_hashmap_clear(HASHMAP_BASE(s), NULL, NULL);
74+
_hashmap_clear(HASHMAP_BASE(s));
8175
}
8276

83-
static inline void set_clear_free(Set *s) {
84-
_hashmap_clear(HASHMAP_BASE(s), free, NULL);
85-
}
86-
87-
/* no set_clear_free_free */
88-
8977
static inline void *set_steal_first(Set *s) {
9078
return _hashmap_first_key_and_value(HASHMAP_BASE(s), true, NULL);
9179
}
9280

93-
#define set_clear_with_destructor(s, f) \
94-
({ \
95-
Set *_s = (s); \
96-
void *_item; \
97-
while ((_item = set_steal_first(_s))) \
98-
f(_item); \
99-
_s; \
100-
})
101-
#define set_free_with_destructor(s, f) \
102-
set_free(set_clear_with_destructor(s, f))
103-
10481
/* no set_steal_first_key */
10582
/* no set_first_key */
10683

@@ -145,10 +122,8 @@ int set_put_strsplit(Set *s, const char *v, const char *separators, ExtractFlags
145122
for (; ({ e = set_first(s); assert_se(!e || set_move_one(d, s, e) >= 0); e; }); )
146123

147124
DEFINE_TRIVIAL_CLEANUP_FUNC(Set*, set_free);
148-
DEFINE_TRIVIAL_CLEANUP_FUNC(Set*, set_free_free);
149125

150126
#define _cleanup_set_free_ _cleanup_(set_freep)
151-
#define _cleanup_set_free_free_ _cleanup_(set_free_freep)
152127

153128
int set_strjoin(Set *s, const char *separator, bool wrap_with_separator, char **ret);
154129

‎src/network/netdev/netdev.c‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1163,8 +1163,7 @@ int netdev_reload(Manager *manager) {
11631163
}
11641164

11651165
/* Detach old NetDev objects from Manager.
1166-
* Note, the same object may be registered with multiple names, and netdev_detach() may drop multiple
1167-
* entries. Hence, hashmap_free_with_destructor() cannot be used. */
1166+
* The same object may be registered with multiple names, and netdev_detach() may drop multiple entries. */
11681167
for (NetDev *n; (n = hashmap_first(manager->netdevs)); )
11691168
netdev_detach(n);
11701169

‎src/network/networkd-manager.c‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -683,8 +683,7 @@ Manager* manager_free(Manager *m) {
683683
m->dhcp_pd_subnet_ids = set_free(m->dhcp_pd_subnet_ids);
684684
m->networks = ordered_hashmap_free(m->networks);
685685

686-
/* The same object may be registered with multiple names, and netdev_detach() may drop multiple
687-
* entries. Hence, hashmap_free_with_destructor() cannot be used. */
686+
/* The same object may be registered with multiple names, and netdev_detach() may drop multiple entries. */
688687
for (NetDev *n; (n = hashmap_first(m->netdevs)); )
689688
netdev_detach(n);
690689
m->netdevs = hashmap_free(m->netdevs);

‎src/test/test-hashmap-plain.c‎

Lines changed: 0 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -764,29 +764,6 @@ TEST(hashmap_free) {
764764
}
765765
}
766766

767-
typedef struct Item {
768-
int seen;
769-
} Item;
770-
static void item_seen(Item *item) {
771-
item->seen++;
772-
}
773-
774-
TEST(hashmap_free_with_destructor) {
775-
Hashmap *m;
776-
struct Item items[4] = {};
777-
unsigned i;
778-
779-
assert_se(m = hashmap_new(NULL));
780-
for (i = 0; i < ELEMENTSOF(items) - 1; i++)
781-
assert_se(hashmap_put(m, INT_TO_PTR(i), items + i) == 1);
782-
783-
m = hashmap_free_with_destructor(m, item_seen);
784-
assert_se(items[0].seen == 1);
785-
assert_se(items[1].seen == 1);
786-
assert_se(items[2].seen == 1);
787-
assert_se(items[3].seen == 0);
788-
}
789-
790767
TEST(hashmap_first) {
791768
_cleanup_hashmap_free_ Hashmap *m = NULL;
792769

‎src/test/test-nulstr-util.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ TEST(strv_make_nulstr) {
110110
}
111111

112112
TEST(set_make_nulstr) {
113-
_cleanup_set_free_free_ Set *set = NULL;
113+
_cleanup_set_free_ Set *set = NULL;
114114
size_t len = 0;
115115
int r;
116116

@@ -130,7 +130,7 @@ TEST(set_make_nulstr) {
130130
static const char expect[] = { 0x00, 0x00 };
131131
_cleanup_free_ char *nulstr = NULL;
132132

133-
set = set_new(NULL);
133+
set = set_new(&string_hash_ops_free);
134134
assert_se(set);
135135

136136
r = set_make_nulstr(set, &nulstr, &len);

‎src/test/test-serialize.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -227,14 +227,14 @@ TEST(serialize_item_base64mem) {
227227
TEST(serialize_string_set) {
228228
_cleanup_(unlink_tempfilep) char fn[] = "/tmp/test-serialize.XXXXXX";
229229
_cleanup_fclose_ FILE *f = NULL;
230-
_cleanup_set_free_free_ Set *s = NULL;
230+
_cleanup_set_free_ Set *s = NULL;
231231
_cleanup_free_ char *line1 = NULL, *line2 = NULL;
232232
char *p, *q;
233233

234234
assert_se(fmkostemp_safe(fn, "r+", &f) == 0);
235235
log_info("/* %s (%s) */", __func__, fn);
236236

237-
assert_se(set_ensure_allocated(&s, &string_hash_ops) >= 0);
237+
assert_se(set_ensure_allocated(&s, &string_hash_ops_free) >= 0);
238238

239239
assert_se(serialize_string_set(f, "a", s) == 0);
240240

‎src/test/test-set.c‎

Lines changed: 2 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -32,21 +32,6 @@ static void item_seen(Item *item) {
3232
item->seen++;
3333
}
3434

35-
TEST(set_free_with_destructor) {
36-
Set *m;
37-
struct Item items[4] = {};
38-
39-
assert_se(m = set_new(NULL));
40-
FOREACH_ARRAY(item, items, ELEMENTSOF(items) - 1)
41-
assert_se(set_put(m, item) == 1);
42-
43-
m = set_free_with_destructor(m, item_seen);
44-
assert_se(items[0].seen == 1);
45-
assert_se(items[1].seen == 1);
46-
assert_se(items[2].seen == 1);
47-
assert_se(items[3].seen == 0);
48-
}
49-
5035
DEFINE_PRIVATE_HASH_OPS_WITH_VALUE_DESTRUCTOR(item_hash_ops, void, trivial_hash_func, trivial_compare_func, Item, item_seen);
5136

5237
TEST(set_free_with_hash_ops) {
@@ -145,9 +130,8 @@ TEST(set_ensure_allocated) {
145130
}
146131

147132
TEST(set_copy) {
148-
_cleanup_set_free_ Set *s = NULL;
149-
_cleanup_set_free_free_ Set *copy = NULL;
150-
char *key1, *key2, *key3, *key4;
133+
_cleanup_set_free_ Set *s = NULL, *copy = NULL;
134+
_cleanup_free_ char *key1 = NULL, *key2 = NULL, *key3 = NULL, *key4 = NULL;
151135

152136
key1 = strdup("key1");
153137
assert_se(key1);

0 commit comments

Comments
 (0)