From f8b02771019a52e9f4a60378b93bf496d54f2c83 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 11 Apr 2025 08:42:52 +0900 Subject: [PATCH 1/7] analyze: replace set_free_free() with set_free() set_put_strdup() uses &string_hash_ops_free, hence set_free() also frees stored contents. --- src/analyze/analyze-verify-util.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/analyze/analyze-verify-util.c b/src/analyze/analyze-verify-util.c index 97f5449b0b..04d9de381e 100644 --- a/src/analyze/analyze-verify-util.c +++ b/src/analyze/analyze-verify-util.c @@ -31,7 +31,7 @@ static void log_syntax_callback(const char *unit, int level, void *userdata) { r = set_put_strdup(s, unit); if (r < 0) { - set_free_free(*s); + set_free(*s); *s = POINTER_MAX; } } @@ -265,7 +265,7 @@ static int verify_unit(Unit *u, bool check_man, const char *root) { static void set_destroy_ignore_pointer_max(Set **s) { if (*s == POINTER_MAX) return; - set_free_free(*s); + set_free(*s); } int verify_units( From 855800aaecb54a45c416193066cea9059a4d647c Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 12 Apr 2025 11:16:26 +0900 Subject: [PATCH 2/7] coredump: replace custom cleanup function with specific hash_ops with destructor --- src/coredump/coredump-vacuum.c | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/src/coredump/coredump-vacuum.c b/src/coredump/coredump-vacuum.c index 8e2febd581..3f99d04cf0 100644 --- a/src/coredump/coredump-vacuum.c +++ b/src/coredump/coredump-vacuum.c @@ -38,11 +38,10 @@ static VacuumCandidate* vacuum_candidate_free(VacuumCandidate *c) { } DEFINE_TRIVIAL_CLEANUP_FUNC(VacuumCandidate*, vacuum_candidate_free); -static Hashmap* vacuum_candidate_hashmap_free(Hashmap *h) { - return hashmap_free_with_destructor(h, vacuum_candidate_free); -} - -DEFINE_TRIVIAL_CLEANUP_FUNC(Hashmap*, vacuum_candidate_hashmap_free); +DEFINE_PRIVATE_HASH_OPS_WITH_VALUE_DESTRUCTOR( + vacuum_candidate_hash_ops, + void, trivial_hash_func, trivial_compare_func, + VacuumCandidate, vacuum_candidate_free); static int uid_from_file_name(const char *filename, uid_t *uid) { const char *p, *e, *u; @@ -141,7 +140,7 @@ int coredump_vacuum(int exclude_fd, uint64_t keep_free, uint64_t max_use) { } for (;;) { - _cleanup_(vacuum_candidate_hashmap_freep) Hashmap *h = NULL; + _cleanup_hashmap_free_ Hashmap *h = NULL; VacuumCandidate *worst = NULL; uint64_t sum = 0; @@ -171,10 +170,6 @@ int coredump_vacuum(int exclude_fd, uint64_t keep_free, uint64_t max_use) { if (exclude_fd >= 0 && stat_inode_same(&exclude_st, &st)) continue; - r = hashmap_ensure_allocated(&h, NULL); - if (r < 0) - return log_oom(); - t = timespec_load(&st.st_mtim); c = hashmap_get(h, UID_TO_PTR(uid)); @@ -197,7 +192,7 @@ int coredump_vacuum(int exclude_fd, uint64_t keep_free, uint64_t max_use) { return r; n->oldest_mtime = t; - r = hashmap_put(h, UID_TO_PTR(uid), n); + r = hashmap_ensure_put(&h, &vacuum_candidate_hash_ops, UID_TO_PTR(uid), n); if (r < 0) return log_oom(); From f92fac7e9bf5cadba939b9d5a021e9142b081ff8 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 11 Apr 2025 10:48:32 +0900 Subject: [PATCH 3/7] sd-device: replace set_free_free() with set_free() These uses string_hash_ops_free, hence not set_free() is enough. --- src/libsystemd/sd-device/device-private.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libsystemd/sd-device/device-private.c b/src/libsystemd/sd-device/device-private.c index 00279ae01f..3ce87690b3 100644 --- a/src/libsystemd/sd-device/device-private.c +++ b/src/libsystemd/sd-device/device-private.c @@ -692,8 +692,8 @@ int device_clone_with_db(sd_device *device, sd_device **ret) { void device_cleanup_tags(sd_device *device) { assert(device); - device->all_tags = set_free_free(device->all_tags); - device->current_tags = set_free_free(device->current_tags); + device->all_tags = set_free(device->all_tags); + device->current_tags = set_free(device->current_tags); device->property_tags_outdated = true; device->tags_generation++; } @@ -701,7 +701,7 @@ void device_cleanup_tags(sd_device *device) { void device_cleanup_devlinks(sd_device *device) { assert(device); - set_free_free(device->devlinks); + set_free(device->devlinks); device->devlinks = NULL; device->property_devlinks_outdated = true; device->devlinks_generation++; From 70669fa2fe040f20ac0a5a8f554f0f26f1505aae Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 12 Apr 2025 11:47:22 +0900 Subject: [PATCH 4/7] sd-device-enumerator: use custom hash_ops with destructor --- src/libsystemd/sd-device/device-enumerator.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/libsystemd/sd-device/device-enumerator.c b/src/libsystemd/sd-device/device-enumerator.c index ca7e70393b..b72a2463da 100644 --- a/src/libsystemd/sd-device/device-enumerator.c +++ b/src/libsystemd/sd-device/device-enumerator.c @@ -91,7 +91,7 @@ static void device_unref_many(sd_device **devices, size_t n) { static void device_enumerator_unref_devices(sd_device_enumerator *enumerator) { assert(enumerator); - hashmap_clear_with_destructor(enumerator->devices_by_syspath, sd_device_unref); + hashmap_clear(enumerator->devices_by_syspath); device_unref_many(enumerator->devices, enumerator->n_devices); enumerator->devices = mfree(enumerator->devices); enumerator->n_devices = 0; @@ -471,6 +471,11 @@ failed: return r; } +DEFINE_PRIVATE_HASH_OPS_WITH_VALUE_DESTRUCTOR( + device_hash_ops_by_syspath, + char, path_hash_func, path_compare, + sd_device, sd_device_unref); + int device_enumerator_add_device(sd_device_enumerator *enumerator, sd_device *device) { const char *syspath; int r; @@ -482,7 +487,7 @@ int device_enumerator_add_device(sd_device_enumerator *enumerator, sd_device *de if (r < 0) return r; - r = hashmap_ensure_put(&enumerator->devices_by_syspath, &string_hash_ops, syspath, device); + r = hashmap_ensure_put(&enumerator->devices_by_syspath, &device_hash_ops_by_syspath, syspath, device); if (IN_SET(r, -EEXIST, 0)) return 0; if (r < 0) From 5f43554f906630acf02ab34c7697bc46b8fa95af Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 12 Apr 2025 11:53:57 +0900 Subject: [PATCH 5/7] sd-netlink: introduce custom hash_ops for GenericNetlinkFamily --- src/libsystemd/sd-netlink/netlink-genl.c | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/src/libsystemd/sd-netlink/netlink-genl.c b/src/libsystemd/sd-netlink/netlink-genl.c index 1dc62e89ba..412480e19a 100644 --- a/src/libsystemd/sd-netlink/netlink-genl.c +++ b/src/libsystemd/sd-netlink/netlink-genl.c @@ -46,11 +46,21 @@ static GenericNetlinkFamily *genl_family_free(GenericNetlinkFamily *f) { DEFINE_TRIVIAL_CLEANUP_FUNC(GenericNetlinkFamily*, genl_family_free); +DEFINE_PRIVATE_HASH_OPS_WITH_VALUE_DESTRUCTOR( + genl_family_hash_ops_by_name, + char, string_hash_func, string_compare_func, + GenericNetlinkFamily, genl_family_free); + +DEFINE_PRIVATE_HASH_OPS_WITH_VALUE_DESTRUCTOR( + genl_family_hash_ops_by_id, + void, trivial_hash_func, trivial_compare_func, + GenericNetlinkFamily, genl_family_free); + void genl_clear_family(sd_netlink *nl) { assert(nl); - nl->genl_family_by_name = hashmap_free_with_destructor(nl->genl_family_by_name, genl_family_free); - nl->genl_family_by_id = hashmap_free_with_destructor(nl->genl_family_by_id, genl_family_free); + nl->genl_family_by_name = hashmap_free(nl->genl_family_by_name); + nl->genl_family_by_id = hashmap_free(nl->genl_family_by_id); } static int genl_family_new_unsupported( @@ -80,7 +90,7 @@ static int genl_family_new_unsupported( if (!f->name) return -ENOMEM; - r = hashmap_ensure_put(&nl->genl_family_by_name, &string_hash_ops, f->name, f); + r = hashmap_ensure_put(&nl->genl_family_by_name, &genl_family_hash_ops_by_name, f->name, f); if (r < 0) return r; @@ -190,11 +200,11 @@ static int genl_family_new( return r; } - r = hashmap_ensure_put(&nl->genl_family_by_id, NULL, UINT_TO_PTR(f->id), f); + r = hashmap_ensure_put(&nl->genl_family_by_id, &genl_family_hash_ops_by_id, UINT_TO_PTR(f->id), f); if (r < 0) return r; - r = hashmap_ensure_put(&nl->genl_family_by_name, &string_hash_ops, f->name, f); + r = hashmap_ensure_put(&nl->genl_family_by_name, &genl_family_hash_ops_by_name, f->name, f); if (r < 0) { hashmap_remove(nl->genl_family_by_id, UINT_TO_PTR(f->id)); return r; From 765ffa12ee4e5e002e9c585dfcabf8e7617d3056 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 11 Apr 2025 11:37:14 +0900 Subject: [PATCH 6/7] sysusers: use trivial_hash_ops_free for storing user/group name --- src/sysusers/sysusers.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/sysusers/sysusers.c b/src/sysusers/sysusers.c index 24b20cbd67..bb14c6c43b 100644 --- a/src/sysusers/sysusers.c +++ b/src/sysusers/sysusers.c @@ -143,7 +143,7 @@ static void context_done(Context *c) { hashmap_free(c->database_by_gid); hashmap_free(c->database_by_groupname); - set_free_free(c->names); + set_free(c->names); uid_range_free(c->uid_range); } @@ -231,9 +231,8 @@ static int load_user_database(Context *c) { if (!n) return -ENOMEM; - /* Note that we use NULL hash_ops (i.e. trivial_hash_ops) here, so identical strings can - * exist in the set. */ - r = set_ensure_consume(&c->names, /* hash_ops= */ NULL, n); + /* Note that we use trivial_hash_ops_free here, so identical strings can exist in the set. */ + r = set_ensure_consume(&c->names, &trivial_hash_ops_free, n); if (r < 0) return r; assert(r > 0); /* The set uses pointer comparisons, so n must not be in the set. */ @@ -274,9 +273,8 @@ static int load_group_database(Context *c) { if (!n) return -ENOMEM; - /* Note that we use NULL hash_ops (i.e. trivial_hash_ops) here, so identical strings can - * exist in the set. */ - r = set_ensure_consume(&c->names, /* hash_ops= */ NULL, n); + /* Note that we use trivial_hash_ops_free here, so identical strings can exist in the set. */ + r = set_ensure_consume(&c->names, &trivial_hash_ops_free, n); if (r < 0) return r; assert(r > 0); /* The set uses pointer comparisons, so n must not be in the set. */ From 4100e0f207ec67c83a351cab34677489bc0784e6 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 12 Apr 2025 12:16:52 +0900 Subject: [PATCH 7/7] sysv-generator: introduce hash_ops for SysvStub This also renames free_sysvstub() -> sysvstub_free(), to follow our usual coding style. --- src/sysv-generator/sysv-generator.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/sysv-generator/sysv-generator.c b/src/sysv-generator/sysv-generator.c index 6520ac91e6..0878f85228 100644 --- a/src/sysv-generator/sysv-generator.c +++ b/src/sysv-generator/sysv-generator.c @@ -64,7 +64,7 @@ typedef struct SysvStub { bool loaded; } SysvStub; -static SysvStub* free_sysvstub(SysvStub *s) { +static SysvStub* sysvstub_free(SysvStub *s) { if (!s) return NULL; @@ -78,11 +78,12 @@ static SysvStub* free_sysvstub(SysvStub *s) { strv_free(s->wanted_by); return mfree(s); } -DEFINE_TRIVIAL_CLEANUP_FUNC(SysvStub*, free_sysvstub); +DEFINE_TRIVIAL_CLEANUP_FUNC(SysvStub*, sysvstub_free); -static void free_sysvstub_hashmapp(Hashmap **h) { - hashmap_free_with_destructor(*h, free_sysvstub); -} +DEFINE_PRIVATE_HASH_OPS_WITH_VALUE_DESTRUCTOR( + sysvstub_hash_ops, + char, string_hash_func, string_compare_func, + SysvStub, sysvstub_free); static int add_alias(const char *service, const char *alias) { _cleanup_free_ char *link = NULL; @@ -728,7 +729,7 @@ static int enumerate_sysv(const LookupPaths *lp, Hashmap *all_services) { FOREACH_DIRENT(de, d, log_error_errno(errno, "Failed to enumerate directory %s, ignoring: %m", *path)) { _cleanup_free_ char *fpath = NULL, *name = NULL; - _cleanup_(free_sysvstubp) SysvStub *service = NULL; + _cleanup_(sysvstub_freep) SysvStub *service = NULL; struct stat st; if (fstatat(dirfd(d), de->d_name, &st, 0) < 0) { @@ -894,7 +895,7 @@ finish: } static int run(const char *dest, const char *dest_early, const char *dest_late) { - _cleanup_(free_sysvstub_hashmapp) Hashmap *all_services = NULL; + _cleanup_hashmap_free_ Hashmap *all_services = NULL; _cleanup_(lookup_paths_done) LookupPaths lp = {}; SysvStub *service; int r; @@ -910,7 +911,7 @@ static int run(const char *dest, const char *dest_early, const char *dest_late) if (r < 0) return r; - all_services = hashmap_new(&string_hash_ops); + all_services = hashmap_new(&sysvstub_hash_ops); if (!all_services) return log_oom();