Add set/hashmap helpers for non-trivial freeing and use where straighforward

A macro is needed because otherwise we couldn't ensure type safety.
Some simple tests are included.
No functional change intended.

diff --git a/src/basic/hashmap.h b/src/basic/hashmap.h
index 222da8f6a39388c988a45050c0ed157a724d420c..0eb763944cd4932ee2a5e19aa2a223b09cba5b8a 100644 (file)
--- a/src/basic/hashmap.h
+++ b/src/basic/hashmap.h
@@ -329,6 +329,29 @@ static inline void *ordered_hashmap_first(OrderedHashmap *h) {
         return internal_hashmap_first(HASHMAP_BASE(h));
 }
 
+#define hashmap_clear_with_destructor(_s, _f)                   \
+        ({                                                      \
+                void *_item;                                    \
+                while ((_item = hashmap_steal_first(_s)))       \
+                        _f(_item);                              \
+        })
+#define hashmap_free_with_destructor(_s, _f)                    \
+        ({                                                      \
+                hashmap_clear_with_destructor(_s, _f);          \
+                hashmap_free(_s);                               \
+        })
+#define ordered_hashmap_clear_with_destructor(_s, _f)                   \
+        ({                                                              \
+                void *_item;                                            \
+                while ((_item = ordered_hashmap_steal_first(_s)))       \
+                        _f(_item);                                      \
+        })
+#define ordered_hashmap_free_with_destructor(_s, _f)                    \
+        ({                                                              \
+                ordered_hashmap_clear_with_destructor(_s, _f);          \
+                ordered_hashmap_free(_s);                               \
+        })
+
 /* no hashmap_next */
 void *ordered_hashmap_next(OrderedHashmap *h, const void *key);
 

diff --git a/src/basic/set.h b/src/basic/set.h
index e798d5fb3571644cf72f36928054373fac7deccf..156ab4b0816db79087d178c87c6a60958c190979 100644 (file)
--- a/src/basic/set.h
+++ b/src/basic/set.h
@@ -108,6 +108,18 @@ static inline void *set_steal_first(Set *s) {
         return internal_hashmap_steal_first(HASHMAP_BASE(s));
 }
 
+#define set_clear_with_destructor(_s, _f)               \
+        ({                                              \
+                void *_item;                            \
+                while ((_item = set_steal_first(_s)))   \
+                        _f(_item);                      \
+        })
+#define set_free_with_destructor(_s, _f)                \
+        ({                                              \
+                set_clear_with_destructor(_s, _f);      \
+                set_free(_s);                           \
+        })
+
 /* no set_steal_first_key */
 /* no set_first_key */
 

diff --git a/src/busctl/busctl.c b/src/busctl/busctl.c
index 3b2b5f3fc227972c046558955964fb662b679c5d..44110b63f9bac036ce3e2a404def71847918d660 100644 (file)
--- a/src/busctl/busctl.c
+++ b/src/busctl/busctl.c
@@ -692,12 +692,7 @@ static void member_free(Member *m) {
 DEFINE_TRIVIAL_CLEANUP_FUNC(Member*, member_free);
 
 static void member_set_free(Set *s) {
-        Member *m;
-
-        while ((m = set_steal_first(s)))
-                member_free(m);
-
-        set_free(s);
+        set_free_with_destructor(s, member_free);
 }
 
 DEFINE_TRIVIAL_CLEANUP_FUNC(Set*, member_set_free);

diff --git a/src/cgtop/cgtop.c b/src/cgtop/cgtop.c
index 8add7e73e4e1c9dd8e23ae6391361d0882648487..fe339eb493e7ff5d531cff55c14dcdc77492849c 100644 (file)
--- a/src/cgtop/cgtop.c
+++ b/src/cgtop/cgtop.c
@@ -105,10 +105,7 @@ static void group_free(Group *g) {
 }
 
 static void group_hashmap_clear(Hashmap *h) {
-        Group *g;
-
-        while ((g = hashmap_steal_first(h)))
-                group_free(g);
+        hashmap_clear_with_destructor(h, group_free);
 }
 
 static void group_hashmap_free(Hashmap *h) {

diff --git a/src/libsystemd-network/sd-dhcp-server.c b/src/libsystemd-network/sd-dhcp-server.c
index 660358e857e2d4ce8450415402e4658e3512806f..63fb355e85a22ee31e9aeb708a2c54aff44d812c 100644 (file)
--- a/src/libsystemd-network/sd-dhcp-server.c
+++ b/src/libsystemd-network/sd-dhcp-server.c
@@ -87,7 +87,6 @@ int sd_dhcp_server_configure_pool(sd_dhcp_server *server, struct in_addr *addres
                 size = size_max;
 
         if (server->address != address->s_addr || server->netmask != netmask || server->pool_size != size || server->pool_offset != offset) {
-                DHCPLease *lease;
 
                 free(server->bound_leases);
                 server->bound_leases = new0(DHCPLease*, size);
@@ -105,8 +104,7 @@ int sd_dhcp_server_configure_pool(sd_dhcp_server *server, struct in_addr *addres
                         server->bound_leases[server_off - offset] = &server->invalid_lease;
 
                 /* Drop any leases associated with the old address range */
-                while ((lease = hashmap_steal_first(server->leases_by_client_id)))
-                        dhcp_lease_free(lease);
+                hashmap_clear_with_destructor(server->leases_by_client_id, dhcp_lease_free);
         }
 
         return 0;

diff --git a/src/libsystemd/sd-bus/bus-track.c b/src/libsystemd/sd-bus/bus-track.c
index 75930f33bab9dbce951c648c88b15b2f804f2773..5b55f304de1d18b5f67d1523aedc509ed07ea75a 100644 (file)
--- a/src/libsystemd/sd-bus/bus-track.c
+++ b/src/libsystemd/sd-bus/bus-track.c
@@ -184,8 +184,6 @@ _public_ sd_bus_track* sd_bus_track_ref(sd_bus_track *track) {
 }
 
 _public_ sd_bus_track* sd_bus_track_unref(sd_bus_track *track) {
-        struct track_item *i;
-
         if (!track)
                 return NULL;
 
@@ -196,14 +194,11 @@ _public_ sd_bus_track* sd_bus_track_unref(sd_bus_track *track) {
                 return NULL;
         }
 
-        while ((i = hashmap_steal_first(track->names)))
-                track_item_free(i);
-
         if (track->in_list)
                 LIST_REMOVE(tracks, track->bus->tracks, track);
 
         bus_track_remove_from_queue(track);
-        hashmap_free(track->names);
+        hashmap_free_with_destructor(track->names, track_item_free);
         sd_bus_unref(track->bus);
         return mfree(track);
 }
@@ -429,8 +424,6 @@ void bus_track_dispatch(sd_bus_track *track) {
 }
 
 void bus_track_close(sd_bus_track *track) {
-        struct track_item *i;
-
         assert(track);
 
         /* Called whenever our bus connected is closed. If so, and our track object is non-empty, dispatch it
@@ -448,8 +441,7 @@ void bus_track_close(sd_bus_track *track) {
                 return;
 
         /* Let's flush out all names */
-        while ((i = hashmap_steal_first(track->names)))
-                track_item_free(i);
+        hashmap_clear_with_destructor(track->names, track_item_free);
 
         /* Invoke handler */
         if (track->handler)

diff --git a/src/machine/image-dbus.c b/src/machine/image-dbus.c
index aebfc388634289340be7e00091f614c32c777a95..10d1b06016eca4ff7faff22f6bda07972029acae 100644 (file)
--- a/src/machine/image-dbus.c
+++ b/src/machine/image-dbus.c
@@ -396,14 +396,11 @@ const sd_bus_vtable image_vtable[] = {
 
 static int image_flush_cache(sd_event_source *s, void *userdata) {
         Manager *m = userdata;
-        Image *i;
 
         assert(s);
         assert(m);
 
-        while ((i = hashmap_steal_first(m->image_cache)))
-                image_unref(i);
-
+        hashmap_clear_with_destructor(m->image_cache, image_unref);
         return 0;
 }
 

diff --git a/src/machine/machined.c b/src/machine/machined.c
index 17142a68af479d64346ef80cceddd69d53786df7..d4810208936483205bc377bdeab3a881fbe8c4ea 100644 (file)
--- a/src/machine/machined.c
+++ b/src/machine/machined.c
@@ -68,7 +68,6 @@ Manager *manager_new(void) {
 
 void manager_free(Manager *m) {
         Machine *machine;
-        Image *i;
 
         assert(m);
 
@@ -84,10 +83,7 @@ void manager_free(Manager *m) {
         hashmap_free(m->machine_units);
         hashmap_free(m->machine_leaders);
 
-        while ((i = hashmap_steal_first(m->image_cache)))
-                image_unref(i);
-
-        hashmap_free(m->image_cache);
+        hashmap_free_with_destructor(m->image_cache, image_unref);
 
         sd_event_source_unref(m->image_cache_defer_event);
 

diff --git a/src/resolve/resolved-dns-scope.c b/src/resolve/resolved-dns-scope.c
index 4be6040e39d666926b2a39b5fc7e3f935d33d778..a9071ee73e3ef581ead115f22484ad261a7b7b53 100644 (file)
--- a/src/resolve/resolved-dns-scope.c
+++ b/src/resolve/resolved-dns-scope.c
@@ -103,8 +103,6 @@ static void dns_scope_abort_transactions(DnsScope *s) {
 }
 
 DnsScope* dns_scope_free(DnsScope *s) {
-        DnsResourceRecord *rr;
-
         if (!s)
                 return NULL;
 
@@ -119,10 +117,7 @@ DnsScope* dns_scope_free(DnsScope *s) {
 
         hashmap_free(s->transactions_by_key);
 
-        while ((rr = ordered_hashmap_steal_first(s->conflict_queue)))
-                dns_resource_record_unref(rr);
-
-        ordered_hashmap_free(s->conflict_queue);
+        ordered_hashmap_free_with_destructor(s->conflict_queue, dns_resource_record_unref);
         sd_event_source_unref(s->conflict_event_source);
 
         sd_event_source_unref(s->announce_event_source);

diff --git a/src/resolve/resolved-dns-trust-anchor.c b/src/resolve/resolved-dns-trust-anchor.c
index f2bd9ef7e49ec6a880e3774b3e0ba430ee7b0327..c6e47ed0e902f9de01ea5f323083e77f32d7773c 100644 (file)
--- a/src/resolve/resolved-dns-trust-anchor.c
+++ b/src/resolve/resolved-dns-trust-anchor.c
@@ -542,19 +542,10 @@ int dns_trust_anchor_load(DnsTrustAnchor *d) {
 }
 
 void dns_trust_anchor_flush(DnsTrustAnchor *d) {
-        DnsAnswer *a;
-        DnsResourceRecord *rr;
-
         assert(d);
 
-        while ((a = hashmap_steal_first(d->positive_by_key)))
-                dns_answer_unref(a);
-        d->positive_by_key = hashmap_free(d->positive_by_key);
-
-        while ((rr = set_steal_first(d->revoked_by_rr)))
-                dns_resource_record_unref(rr);
-        d->revoked_by_rr = set_free(d->revoked_by_rr);
-
+        d->positive_by_key = hashmap_free_with_destructor(d->positive_by_key, dns_answer_unref);
+        d->revoked_by_rr = set_free_with_destructor(d->revoked_by_rr, dns_resource_record_unref);
         d->negative_by_name = set_free_free(d->negative_by_name);
 }
 

diff --git a/src/shared/bus-util.c b/src/shared/bus-util.c
index c6844f5ec2c1833a7c82be6d97e9d5889c185811..7d5ea0c0acf53019bdd7c1cd9a61991ee8cb6884 100644 (file)
--- a/src/shared/bus-util.c
+++ b/src/shared/bus-util.c
@@ -554,12 +554,7 @@ int bus_verify_polkit_async(
 
 void bus_verify_polkit_async_registry_free(Hashmap *registry) {
 #if ENABLE_POLKIT
-        AsyncPolkitQuery *q;
-
-        while ((q = hashmap_steal_first(registry)))
-                async_polkit_query_free(q);
-
-        hashmap_free(registry);
+        hashmap_free_with_destructor(registry, async_polkit_query_free);
 #endif
 }
 

diff --git a/src/shared/install.c b/src/shared/install.c
index a75a045d43d1ad1d43c01ec037f6fc4a4ad4d6f1..05ccc586a94131fb1392defe06ff827ddbe21c63 100644 (file)
--- a/src/shared/install.c
+++ b/src/shared/install.c
@@ -992,23 +992,11 @@ static void install_info_free(UnitFileInstallInfo *i) {
         free(i);
 }
 
-static OrderedHashmap* install_info_hashmap_free(OrderedHashmap *m) {
-        UnitFileInstallInfo *i;
-
-        if (!m)
-                return NULL;
-
-        while ((i = ordered_hashmap_steal_first(m)))
-                install_info_free(i);
-
-        return ordered_hashmap_free(m);
-}
-
 static void install_context_done(InstallContext *c) {
         assert(c);
 
-        c->will_process = install_info_hashmap_free(c->will_process);
-        c->have_processed = install_info_hashmap_free(c->have_processed);
+        c->will_process = ordered_hashmap_free_with_destructor(c->will_process, install_info_free);
+        c->have_processed = ordered_hashmap_free_with_destructor(c->have_processed, install_info_free);
 }
 
 static UnitFileInstallInfo *install_info_find(InstallContext *c, const char *name) {
@@ -3141,12 +3129,7 @@ static void unit_file_list_free_one(UnitFileList *f) {
 }
 
 Hashmap* unit_file_list_free(Hashmap *h) {
-        UnitFileList *i;
-
-        while ((i = hashmap_steal_first(h)))
-                unit_file_list_free_one(i);
-
-        return hashmap_free(h);
+        return hashmap_free_with_destructor(h, unit_file_list_free_one);
 }
 
 DEFINE_TRIVIAL_CLEANUP_FUNC(UnitFileList*, unit_file_list_free_one);

diff --git a/src/socket-proxy/socket-proxyd.c b/src/socket-proxy/socket-proxyd.c
index 8887e2f989044c7888c3c0e5f2f4c744c30751c2..d4f401d99630c838f3b928ab94f03d4d91574f8a 100644 (file)
--- a/src/socket-proxy/socket-proxyd.c
+++ b/src/socket-proxy/socket-proxyd.c
@@ -92,19 +92,10 @@ static void connection_free(Connection *c) {
 }
 
 static void context_free(Context *context) {
-        sd_event_source *es;
-        Connection *c;
-
         assert(context);
 
-        while ((es = set_steal_first(context->listen)))
-                sd_event_source_unref(es);
-
-        while ((c = set_first(context->connections)))
-                connection_free(c);
-
-        set_free(context->listen);
-        set_free(context->connections);
+        set_free_with_destructor(context->listen, sd_event_source_unref);
+        set_free_with_destructor(context->connections, connection_free);
 
         sd_event_unref(context->event);
         sd_resolve_unref(context->resolve);

diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c
index 4fd9b1063406909590c646d8fa2f228cf4040a52..0d67905cdc24cc4c76772eefe6a8f57ffe8dd143 100644 (file)
--- a/src/systemctl/systemctl.c
+++ b/src/systemctl/systemctl.c
@@ -699,12 +699,7 @@ static int get_unit_list(
 }
 
 static void message_set_freep(Set **set) {
-        sd_bus_message *m;
-
-        while ((m = set_steal_first(*set)))
-                sd_bus_message_unref(m);
-
-        set_free(*set);
+        set_free_with_destructor(*set, sd_bus_message_unref);
 }
 
 static int get_unit_list_recursive(

diff --git a/src/sysusers/sysusers.c b/src/sysusers/sysusers.c
index 909a4ca542eefb8a3a0475333a5c10315328cef4..d8009458ee3ec8a2443511234a9b936e61434782 100644 (file)
--- a/src/sysusers/sysusers.c
+++ b/src/sysusers/sysusers.c
@@ -1864,20 +1864,15 @@ int main(int argc, char *argv[]) {
                 log_error_errno(r, "Failed to write files: %m");
 
 finish:
-        while ((i = hashmap_steal_first(groups)))
-                item_free(i);
-
-        while ((i = hashmap_steal_first(users)))
-                item_free(i);
+        hashmap_free_with_destructor(groups, item_free);
+        hashmap_free_with_destructor(users, item_free);
 
         while ((n = hashmap_first_key(members))) {
                 strv_free(hashmap_steal_first(members));
                 free(n);
         }
-
-        hashmap_free(groups);
-        hashmap_free(users);
         hashmap_free(members);
+
         hashmap_free(todo_uids);
         hashmap_free(todo_gids);
 

diff --git a/src/sysv-generator/sysv-generator.c b/src/sysv-generator/sysv-generator.c
index cfb34c8f0b5998ad8fd42d758fbcc79a16e7e8be..f59c277e13ed9d9e0c35370fe17251ad60ab122e 100644 (file)
--- a/src/sysv-generator/sysv-generator.c
+++ b/src/sysv-generator/sysv-generator.c
@@ -95,12 +95,7 @@ static void free_sysvstub(SysvStub *s) {
 DEFINE_TRIVIAL_CLEANUP_FUNC(SysvStub*, free_sysvstub);
 
 static void free_sysvstub_hashmapp(Hashmap **h) {
-        SysvStub *stub;
-
-        while ((stub = hashmap_steal_first(*h)))
-                free_sysvstub(stub);
-
-        hashmap_free(*h);
+        hashmap_free_with_destructor(*h, free_sysvstub);
 }
 
 static int add_alias(const char *service, const char *alias) {

diff --git a/src/test/test-hashmap.c b/src/test/test-hashmap.c
index 6ea1780cf254629289f169603ef22d1c2e6e21ae..dd9195425efd9f7e690fdbdb003e73cae5b503db 100644 (file)
--- a/src/test/test-hashmap.c
+++ b/src/test/test-hashmap.c
@@ -38,6 +38,29 @@ static void test_ordered_hashmap_next(void) {
         assert_se(!ordered_hashmap_next(m, INT_TO_PTR(3)));
 }
 
+typedef struct Item {
+        int seen;
+} Item;
+static void item_seen(Item *item) {
+        item->seen++;
+}
+
+static void test_hashmap_free_with_destructor(void) {
+        Hashmap *m;
+        struct Item items[4] = {};
+        unsigned i;
+
+        assert_se(m = hashmap_new(NULL));
+        for (i = 0; i < ELEMENTSOF(items) - 1; i++)
+                assert_se(hashmap_put(m, INT_TO_PTR(i), items + i) == 1);
+
+        m = hashmap_free_with_destructor(m, item_seen);
+        assert_se(items[0].seen == 1);
+        assert_se(items[1].seen == 1);
+        assert_se(items[2].seen == 1);
+        assert_se(items[3].seen == 0);
+}
+
 static void test_uint64_compare_func(void) {
         const uint64_t a = 0x100, b = 0x101;
 
@@ -62,6 +85,7 @@ int main(int argc, const char *argv[]) {
         test_ordered_hashmap_funcs();
 
         test_ordered_hashmap_next();
+        test_hashmap_free_with_destructor();
         test_uint64_compare_func();
         test_trivial_compare_func();
         test_string_compare_func();

diff --git a/src/test/test-set.c b/src/test/test-set.c
index 7bae95bd2bccced2252fd192911670f0808528b5..0a29a62621095ce32bc85f222664c4fc3d66067c 100644 (file)
--- a/src/test/test-set.c
+++ b/src/test/test-set.c
@@ -40,6 +40,29 @@ static void test_set_steal_first(void) {
         assert_se(set_isempty(m));
 }
 
+typedef struct Item {
+        int seen;
+} Item;
+static void item_seen(Item *item) {
+        item->seen++;
+}
+
+static void test_set_free_with_destructor(void) {
+        Set *m;
+        struct Item items[4] = {};
+        unsigned i;
+
+        assert_se(m = set_new(NULL));
+        for (i = 0; i < ELEMENTSOF(items) - 1; i++)
+                assert_se(set_put(m, items + i) == 1);
+
+        m = set_free_with_destructor(m, item_seen);
+        assert_se(items[0].seen == 1);
+        assert_se(items[1].seen == 1);
+        assert_se(items[2].seen == 1);
+        assert_se(items[3].seen == 0);
+}
+
 static void test_set_put(void) {
         _cleanup_set_free_ Set *m = NULL;
 
@@ -102,6 +125,7 @@ static void test_set_make(void) {
 
 int main(int argc, const char *argv[]) {
         test_set_steal_first();
+        test_set_free_with_destructor();
         test_set_put();
         test_set_make();
 

diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c
index 970ea3b05f812cad4502e98f18e9c2b807bd8d43..ad89b46f010d0a2573e0bbe9db5488c476f4a96a 100644 (file)
--- a/src/tmpfiles/tmpfiles.c
+++ b/src/tmpfiles/tmpfiles.c
@@ -2393,14 +2393,8 @@ int main(int argc, char *argv[]) {
         }
 
 finish:
-        while ((a = ordered_hashmap_steal_first(items)))
-                item_array_free(a);
-
-        while ((a = ordered_hashmap_steal_first(globs)))
-                item_array_free(a);
-
-        ordered_hashmap_free(items);
-        ordered_hashmap_free(globs);
+        ordered_hashmap_free_with_destructor(items, item_array_free);
+        ordered_hashmap_free_with_destructor(globs, item_array_free);
 
         free(arg_include_prefixes);
         free(arg_exclude_prefixes);