Merge pull request #7492 from keszybz/coverity-fixes

A few more cleanups based on coverity and valgrind

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);
 

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 */
 

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);

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) {

src/coredump/coredump-vacuum.c

@@ -52,16 +52,11 @@ static void vacuum_candidate_free(struct vacuum_candidate *c) {
 
 DEFINE_TRIVIAL_CLEANUP_FUNC(struct vacuum_candidate*, vacuum_candidate_free);
 
-static void vacuum_candidate_hasmap_free(Hashmap *h) {
-        struct vacuum_candidate *c;
-
-        while ((c = hashmap_steal_first(h)))
-                vacuum_candidate_free(c);
-
-        hashmap_free(h);
+static void vacuum_candidate_hashmap_free(Hashmap *h) {
+        hashmap_free_with_destructor(h, vacuum_candidate_free);
 }
 
-DEFINE_TRIVIAL_CLEANUP_FUNC(Hashmap*, vacuum_candidate_hasmap_free);
+DEFINE_TRIVIAL_CLEANUP_FUNC(Hashmap*, vacuum_candidate_hashmap_free);
 
 static int uid_from_file_name(const char *filename, uid_t *uid) {
         const char *p, *e, *u;
@@ -160,7 +155,7 @@ int coredump_vacuum(int exclude_fd, uint64_t keep_free, uint64_t max_use) {
         }
 
         for (;;) {
-                _cleanup_(vacuum_candidate_hasmap_freep) Hashmap *h = NULL;
+                _cleanup_(vacuum_candidate_hashmap_freep) Hashmap *h = NULL;
                 struct vacuum_candidate *worst = NULL;
                 struct dirent *de;
                 uint64_t sum = 0;

src/cryptsetup/cryptsetup-generator.c

@@ -219,18 +219,12 @@ static int create_disk(
         return 0;
 }
 
-static void free_arg_disks(void) {
-        crypto_device *d;
-
-        while ((d = hashmap_steal_first(arg_disks))) {
-                free(d->uuid);
-                free(d->keyfile);
-                free(d->name);
-                free(d->options);
-                free(d);
-        }
-
-        hashmap_free(arg_disks);
+static void crypt_device_free(crypto_device *d) {
+        free(d->uuid);
+        free(d->keyfile);
+        free(d->name);
+        free(d->options);
+        free(d);
 }
 
 static crypto_device *get_crypto_device(const char *uuid) {
@@ -492,7 +486,7 @@ int main(int argc, char *argv[]) {
         r = 0;
 
 finish:
-        free_arg_disks();
+        hashmap_free_with_destructor(arg_disks, crypt_device_free);
         free(arg_default_options);
         free(arg_default_keyfile);
 

src/journal-remote/journal-remote.c

@@ -1043,18 +1043,17 @@ static int remoteserver_init(RemoteServer *s,
         return 0;
 }
 
+static void MHDDaemonWrapper_free(MHDDaemonWrapper *d) {
+        MHD_stop_daemon(d->daemon);
+        sd_event_source_unref(d->io_event);
+        sd_event_source_unref(d->timer_event);
+        free(d);
+}
+
 static void server_destroy(RemoteServer *s) {
         size_t i;
-        MHDDaemonWrapper *d;
 
-        while ((d = hashmap_steal_first(s->daemons))) {
-                MHD_stop_daemon(d->daemon);
-                sd_event_source_unref(d->io_event);
-                sd_event_source_unref(d->timer_event);
-                free(d);
-        }
-
-        hashmap_free(s->daemons);
+        hashmap_free_with_destructor(s->daemons, MHDDaemonWrapper_free);
 
         assert(s->sources_size == 0 || s->sources);
         for (i = 0; i < s->sources_size; i++)

src/journal/compress.c

@@ -667,7 +667,7 @@ int decompress_stream_lz4(int in, int out, uint64_t max_bytes) {
 
         log_debug("LZ4 decompression finished (%zu -> %zu bytes, %.1f%%)",
                   total_in, total_out,
-                  (double) total_out / total_in * 100);
+                  total_in > 0 ? (double) total_out / total_in * 100 : 0.0);
  cleanup:
         munmap(src, st.st_size);
         return r;

src/journal/journal-file.c

@@ -400,15 +400,6 @@ JournalFile* journal_file_close(JournalFile *f) {
         return mfree(f);
 }
 
-void journal_file_close_set(Set *s) {
-        JournalFile *f;
-
-        assert(s);
-
-        while ((f = set_steal_first(s)))
-                (void) journal_file_close(f);
-}
-
 static int journal_file_init_header(JournalFile *f, JournalFile *template) {
         Header h = {};
         ssize_t k;
@@ -3371,8 +3362,7 @@ int journal_file_open(
         f->header = h;
 
         if (!newly_created) {
-                if (deferred_closes)
-                        journal_file_close_set(deferred_closes);
+                set_clear_with_destructor(deferred_closes, journal_file_close);
 
                 r = journal_file_verify_header(f);
                 if (r < 0)

src/journal/journal-file.h

@@ -161,7 +161,6 @@ int journal_file_open(
 int journal_file_set_offline(JournalFile *f, bool wait);
 bool journal_file_is_offlining(JournalFile *f);
 JournalFile* journal_file_close(JournalFile *j);
-void journal_file_close_set(Set *s);
 
 int journal_file_open_reliably(
                 const char *fname,

src/journal/journald-server.c

@@ -1899,13 +1899,9 @@ void server_maybe_append_tags(Server *s) {
 }
 
 void server_done(Server *s) {
-        JournalFile *f;
         assert(s);
 
-        if (s->deferred_closes) {
-                journal_file_close_set(s->deferred_closes);
-                set_free(s->deferred_closes);
-        }
+        set_free_with_destructor(s->deferred_closes, journal_file_close);
 
         while (s->stdout_streams)
                 stdout_stream_free(s->stdout_streams);
@@ -1918,10 +1914,7 @@ void server_done(Server *s) {
         if (s->runtime_journal)
                 (void) journal_file_close(s->runtime_journal);
 
-        while ((f = ordered_hashmap_steal_first(s->user_journals)))
-                (void) journal_file_close(f);
-
-        ordered_hashmap_free(s->user_journals);
+        ordered_hashmap_free_with_destructor(s->user_journals, journal_file_close);
 
         sd_event_source_unref(s->syslog_event_source);
         sd_event_source_unref(s->native_event_source);

src/journal/sd-journal.c

@@ -1977,18 +1977,13 @@ fail:
 
 _public_ void sd_journal_close(sd_journal *j) {
         Directory *d;
-        JournalFile *f;
-        char *p;
 
         if (!j)
                 return;
 
         sd_journal_flush_matches(j);
 
-        while ((f = ordered_hashmap_steal_first(j->files)))
-                (void) journal_file_close(f);
-
-        ordered_hashmap_free(j->files);
+        ordered_hashmap_free_with_destructor(j->files, journal_file_close);
 
         while ((d = hashmap_first(j->directories_by_path)))
                 remove_directory(j, d);
@@ -2006,9 +2001,7 @@ _public_ void sd_journal_close(sd_journal *j) {
                 mmap_cache_unref(j->mmap);
         }
 
-        while ((p = hashmap_steal_first(j->errors)))
-                free(p);
-        hashmap_free(j->errors);
+        hashmap_free_free(j->errors);
 
         free(j->path);
         free(j->prefix);

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;

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)

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;
 }
 

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);
 

src/network/networkd-manager.c

@@ -1223,7 +1223,6 @@ int manager_new(Manager **ret, sd_event *event) {
 }
 
 void manager_free(Manager *m) {
-        RoutingPolicyRule *rule;
         Network *network;
         NetDev *netdev;
         Link *link;
@@ -1253,10 +1252,7 @@ void manager_free(Manager *m) {
         set_free(m->rules);
         set_free(m->rules_foreign);
 
-        while ((rule = set_steal_first(m->rules_saved)))
-                free(rule);
-
-        set_free(m->rules_saved);
+        set_free_with_destructor(m->rules_saved, routing_policy_rule_free);
 
         sd_netlink_unref(m->rtnl);
         sd_event_unref(m->event);

src/network/test-routing-policy-rule.c

@@ -34,7 +34,7 @@ static void test_rule_serialization(const char *title, const char *ruleset, cons
         const char *cmd;
         int fd, fd2, fd3;
         _cleanup_fclose_ FILE *f = NULL, *f2 = NULL, *f3 = NULL;
-        _cleanup_set_free_free_ Set *rules = NULL;
+        Set *rules = NULL;
         _cleanup_free_ char *buf = NULL;
         size_t buf_size;
 
@@ -64,6 +64,8 @@ static void test_rule_serialization(const char *title, const char *ruleset, cons
         cmd = strjoina("diff -u ", pattern3, " ", pattern2);
         log_info("$ %s", cmd);
         assert_se(system(cmd) == 0);
+
+        set_free_with_destructor(rules, routing_policy_rule_free);
 }
 
 int main(int argc, char **argv) {

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);

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);
 }