From 8d190e990ee2da124eb36e0bbeefb798aa8ee993 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 11 Apr 2025 11:47:53 +0900 Subject: [PATCH 1/6] install: use path_hash_ops_free --- src/shared/install.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/shared/install.c b/src/shared/install.c index cbbf8b0706..fb784e50a5 100644 --- a/src/shared/install.c +++ b/src/shared/install.c @@ -634,7 +634,7 @@ static int mark_symlink_for_removal( assert(p); - r = set_ensure_allocated(remove_symlinks_to, &path_hash_ops); + r = set_ensure_allocated(remove_symlinks_to, &path_hash_ops_free); if (r < 0) return r; @@ -2371,7 +2371,7 @@ int unit_file_unmask( size_t *n_changes) { _cleanup_(lookup_paths_done) LookupPaths lp = {}; - _cleanup_set_free_free_ Set *remove_symlinks_to = NULL; + _cleanup_set_free_ Set *remove_symlinks_to = NULL; _cleanup_strv_free_ char **todo = NULL; const char *config_path; size_t n_todo = 0; @@ -2587,7 +2587,7 @@ int unit_file_revert( InstallChange **changes, size_t *n_changes) { - _cleanup_set_free_free_ Set *remove_symlinks_to = NULL; + _cleanup_set_free_ Set *remove_symlinks_to = NULL; _cleanup_(lookup_paths_done) LookupPaths lp = {}; _cleanup_strv_free_ char **todo = NULL; size_t n_todo = 0; @@ -2874,7 +2874,6 @@ static int do_unit_file_disable( size_t *n_changes) { _cleanup_(install_context_done) InstallContext ctx = { .scope = scope }; - _cleanup_set_free_free_ Set *remove_symlinks_to = NULL; bool has_install_info = false; int r; @@ -2901,6 +2900,7 @@ static int do_unit_file_disable( has_install_info = has_install_info || install_info_has_rules(info) || install_info_has_also(info); } + _cleanup_set_free_ Set *remove_symlinks_to = NULL; r = install_context_mark_for_removal(&ctx, lp, &remove_symlinks_to, config_path, changes, n_changes); if (r >= 0) r = remove_marked_symlinks(remove_symlinks_to, config_path, lp, flags & UNIT_FILE_DRY_RUN, changes, n_changes); @@ -3538,7 +3538,7 @@ static int execute_preset( assert(config_path); if (mode != UNIT_FILE_PRESET_ENABLE_ONLY) { - _cleanup_set_free_free_ Set *remove_symlinks_to = NULL; + _cleanup_set_free_ Set *remove_symlinks_to = NULL; r = install_context_mark_for_removal(minus, lp, &remove_symlinks_to, config_path, changes, n_changes); if (r < 0) From f9ae1f5cf52e3917cf8697c1b6b13e02e01d3c71 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 12 Apr 2025 22:41:25 +0900 Subject: [PATCH 2/6] install: use hash_ops with destructor for InstallInfo --- src/shared/install.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/shared/install.c b/src/shared/install.c index fb784e50a5..1f1c66d6d8 100644 --- a/src/shared/install.c +++ b/src/shared/install.c @@ -1126,11 +1126,16 @@ static InstallInfo* install_info_free(InstallInfo *i) { DEFINE_TRIVIAL_CLEANUP_FUNC(InstallInfo*, install_info_free); +DEFINE_PRIVATE_HASH_OPS_WITH_VALUE_DESTRUCTOR( + install_info_hash_ops, + char, string_hash_func, string_compare_func, + InstallInfo, install_info_free); + static void install_context_done(InstallContext *ctx) { assert(ctx); - ctx->will_process = ordered_hashmap_free_with_destructor(ctx->will_process, install_info_free); - ctx->have_processed = ordered_hashmap_free_with_destructor(ctx->have_processed, install_info_free); + ctx->will_process = ordered_hashmap_free(ctx->will_process); + ctx->have_processed = ordered_hashmap_free(ctx->have_processed); } static InstallInfo *install_info_find(InstallContext *ctx, const char *name) { @@ -1229,7 +1234,7 @@ static int install_info_add( if (r < 0) return r; - r = ordered_hashmap_ensure_put(&ctx->will_process, &string_hash_ops, new_info->name, new_info); + r = ordered_hashmap_ensure_put(&ctx->will_process, &install_info_hash_ops, new_info->name, new_info); if (r < 0) return r; i = TAKE_PTR(new_info); @@ -2195,7 +2200,7 @@ static int install_context_apply( if (ordered_hashmap_isempty(ctx->will_process)) return 0; - r = ordered_hashmap_ensure_allocated(&ctx->have_processed, &string_hash_ops); + r = ordered_hashmap_ensure_allocated(&ctx->have_processed, &install_info_hash_ops); if (r < 0) return r; @@ -2267,7 +2272,7 @@ static int install_context_mark_for_removal( if (ordered_hashmap_isempty(ctx->will_process)) return 0; - r = ordered_hashmap_ensure_allocated(&ctx->have_processed, &string_hash_ops); + r = ordered_hashmap_ensure_allocated(&ctx->have_processed, &install_info_hash_ops); if (r < 0) return r; From c4c4ea2fb660f3b439321359720a7e1a0d1f6f11 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 11 Apr 2025 10:19:07 +0900 Subject: [PATCH 3/6] core: replace unnecessary use of set_free_free() with set_free() These variables are assigned with set_put_strv(), hence string_hash_ops_free is used. This also drops unnecessary call of set_ensure_allocated() with wrong hash ops. --- src/core/cgroup.c | 2 +- src/core/dbus-cgroup.c | 2 +- src/core/dbus-execute.c | 6 +++--- src/core/execute-serialize.c | 8 -------- src/core/execute.c | 6 +++--- src/core/load-fragment.c | 8 ++++---- 6 files changed, 12 insertions(+), 20 deletions(-) diff --git a/src/core/cgroup.c b/src/core/cgroup.c index 3e7bf74b69..c55813d662 100644 --- a/src/core/cgroup.c +++ b/src/core/cgroup.c @@ -620,7 +620,7 @@ void cgroup_context_done(CGroupContext *c) { while (c->bpf_foreign_programs) cgroup_context_remove_bpf_foreign_program(c, c->bpf_foreign_programs); - c->restrict_network_interfaces = set_free_free(c->restrict_network_interfaces); + c->restrict_network_interfaces = set_free(c->restrict_network_interfaces); cpu_set_reset(&c->cpuset_cpus); cpu_set_reset(&c->startup_cpuset_cpus); diff --git a/src/core/dbus-cgroup.c b/src/core/dbus-cgroup.c index c99f1e29ac..bd355281e7 100644 --- a/src/core/dbus-cgroup.c +++ b/src/core/dbus-cgroup.c @@ -2218,7 +2218,7 @@ int bus_cgroup_set_property( if (strv_isempty(l)) { c->restrict_network_interfaces_is_allow_list = false; - c->restrict_network_interfaces = set_free_free(c->restrict_network_interfaces); + c->restrict_network_interfaces = set_free(c->restrict_network_interfaces); unit_write_settingf(u, flags, name, "%s=", name); return 1; diff --git a/src/core/dbus-execute.c b/src/core/dbus-execute.c index 2e79fdf276..2e1527811c 100644 --- a/src/core/dbus-execute.c +++ b/src/core/dbus-execute.c @@ -1862,8 +1862,8 @@ int bus_exec_context_set_transient_property( if (!UNIT_WRITE_FLAGS_NOOP(flags)) { if (strv_isempty(allow_list) && strv_isempty(deny_list)) { - c->log_filter_allowed_patterns = set_free_free(c->log_filter_allowed_patterns); - c->log_filter_denied_patterns = set_free_free(c->log_filter_denied_patterns); + c->log_filter_allowed_patterns = set_free(c->log_filter_allowed_patterns); + c->log_filter_denied_patterns = set_free(c->log_filter_denied_patterns); unit_write_settingf(u, flags, name, "%s=", name); } else { r = set_put_strdupv(&c->log_filter_allowed_patterns, allow_list); @@ -2240,7 +2240,7 @@ int bus_exec_context_set_transient_property( if (strv_isempty(l)) { c->restrict_filesystems_allow_list = false; - c->restrict_filesystems = set_free_free(c->restrict_filesystems); + c->restrict_filesystems = set_free(c->restrict_filesystems); unit_write_setting(u, flags, name, "RestrictFileSystems="); return 1; diff --git a/src/core/execute-serialize.c b/src/core/execute-serialize.c index 2a9c619f2c..a13d612080 100644 --- a/src/core/execute-serialize.c +++ b/src/core/execute-serialize.c @@ -1089,10 +1089,6 @@ static int exec_cgroup_context_deserialize(CGroupContext *c, FILE *f) { LIST_PREPEND(socket_bind_items, c->socket_bind_deny, item); } else if ((val = startswith(l, "exec-cgroup-context-restrict-network-interfaces="))) { - r = set_ensure_allocated(&c->restrict_network_interfaces, &string_hash_ops); - if (r < 0) - return r; - r = set_put_strdup(&c->restrict_network_interfaces, val); if (r < 0) return r; @@ -3547,10 +3543,6 @@ static int exec_context_deserialize(ExecContext *c, FILE *f) { if (r < 0) return r; } else if ((val = startswith(l, "exec-context-restrict-filesystems="))) { - r = set_ensure_allocated(&c->restrict_filesystems, &string_hash_ops); - if (r < 0) - return r; - r = set_put_strdup(&c->restrict_filesystems, val); if (r < 0) return r; diff --git a/src/core/execute.c b/src/core/execute.c index cca42583d1..dade528b8c 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -705,7 +705,7 @@ void exec_context_done(ExecContext *c) { c->apparmor_profile = mfree(c->apparmor_profile); c->smack_process_label = mfree(c->smack_process_label); - c->restrict_filesystems = set_free_free(c->restrict_filesystems); + c->restrict_filesystems = set_free(c->restrict_filesystems); c->syscall_filter = hashmap_free(c->syscall_filter); c->syscall_archs = set_free(c->syscall_archs); @@ -718,8 +718,8 @@ void exec_context_done(ExecContext *c) { c->log_level_max = -1; exec_context_free_log_extra_fields(c); - c->log_filter_allowed_patterns = set_free_free(c->log_filter_allowed_patterns); - c->log_filter_denied_patterns = set_free_free(c->log_filter_denied_patterns); + c->log_filter_allowed_patterns = set_free(c->log_filter_allowed_patterns); + c->log_filter_denied_patterns = set_free(c->log_filter_denied_patterns); c->log_ratelimit = (RateLimit) {}; diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c index 9600cb46af..5c1bc8dd96 100644 --- a/src/core/load-fragment.c +++ b/src/core/load-fragment.c @@ -3653,7 +3653,7 @@ int config_parse_restrict_filesystems( if (isempty(rvalue)) { /* Empty assignment resets the list */ - c->restrict_filesystems = set_free_free(c->restrict_filesystems); + c->restrict_filesystems = set_free(c->restrict_filesystems); c->restrict_filesystems_allow_list = false; return 0; } @@ -6056,7 +6056,7 @@ int config_parse_restrict_network_interfaces( if (isempty(rvalue)) { /* Empty assignment resets the list */ - c->restrict_network_interfaces = set_free_free(c->restrict_network_interfaces); + c->restrict_network_interfaces = set_free(c->restrict_network_interfaces); return 0; } @@ -6681,8 +6681,8 @@ int config_parse_log_filter_patterns( if (isempty(pattern)) { /* Empty assignment resets the lists. */ - c->log_filter_allowed_patterns = set_free_free(c->log_filter_allowed_patterns); - c->log_filter_denied_patterns = set_free_free(c->log_filter_denied_patterns); + c->log_filter_allowed_patterns = set_free(c->log_filter_allowed_patterns); + c->log_filter_denied_patterns = set_free(c->log_filter_denied_patterns); return 0; } From 5793a39dd8de9f3ed689aacb75db2528d5acd8e1 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 11 Apr 2025 10:28:54 +0900 Subject: [PATCH 4/6] core,systemctl: unit_file_find_fragment() provides names with string_hash_ops_free names are assigned with set_put_strdup(), hence string_hash_ops_free is used and we can use set_free() rather than set_free_free(). --- src/core/load-fragment.c | 4 ++-- src/systemctl/systemctl-util.c | 5 ++--- src/test/test-unit-file.c | 2 +- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c index 5c1bc8dd96..e09415e1d3 100644 --- a/src/core/load-fragment.c +++ b/src/core/load-fragment.c @@ -6178,8 +6178,6 @@ static int merge_by_names(Unit *u, Set *names, const char *id) { } int unit_load_fragment(Unit *u) { - const char *fragment; - _cleanup_set_free_free_ Set *names = NULL; int r; assert(u); @@ -6201,6 +6199,8 @@ int unit_load_fragment(Unit *u) { if (r < 0) return log_error_errno(r, "Failed to rebuild name map: %m"); + const char *fragment; + _cleanup_set_free_ Set *names = NULL; r = unit_file_find_fragment(u->manager->unit_id_map, u->manager->unit_name_map, u->id, diff --git a/src/systemctl/systemctl-util.c b/src/systemctl/systemctl-util.c index ecff2453e6..a6771601a1 100644 --- a/src/systemctl/systemctl-util.c +++ b/src/systemctl/systemctl-util.c @@ -569,15 +569,14 @@ int unit_find_paths( return log_error_errno(r, "Failed to get DropInPaths: %s", bus_error_message(&error, r)); } } else { - const char *_path; - _cleanup_set_free_free_ Set *names = NULL; - if (!*cached_name_map) { r = unit_file_build_name_map(lp, NULL, cached_id_map, cached_name_map, NULL); if (r < 0) return r; } + const char *_path; + _cleanup_set_free_ Set *names = NULL; r = unit_file_find_fragment(*cached_id_map, *cached_name_map, unit_name, &_path, &names); if (r < 0) return log_error_errno(r, "Failed to find fragment for '%s': %m", unit_name); diff --git a/src/test/test-unit-file.c b/src/test/test-unit-file.c index c7779237b5..06aee82bdf 100644 --- a/src/test/test-unit-file.c +++ b/src/test/test-unit-file.c @@ -63,7 +63,7 @@ TEST(unit_file_build_name_map) { STRV_FOREACH(id, ids) { const char *fragment, *name; - _cleanup_set_free_free_ Set *names = NULL; + _cleanup_set_free_ Set *names = NULL; log_info("*** %s ***", *id); r = unit_file_find_fragment(unit_ids, unit_names, From 5c6ea302097c1a18f96866cd31c5008927c0b4b0 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 11 Apr 2025 10:40:20 +0900 Subject: [PATCH 5/6] core/unit: use string_hash_ops_free for Unit.aliases --- src/core/unit.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/core/unit.c b/src/core/unit.c index b8febd4a21..557630368c 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -219,7 +219,7 @@ static int unit_add_alias(Unit *u, char *donated_name) { /* Make sure that u->names is allocated. We may leave u->names * empty if we fail later, but this is not a problem. */ - r = set_ensure_put(&u->aliases, &string_hash_ops, donated_name); + r = set_ensure_put(&u->aliases, &string_hash_ops_free, donated_name); if (r < 0) return r; assert(r > 0); @@ -860,7 +860,7 @@ Unit* unit_free(Unit *u) { free(u->access_selinux_context); - set_free_free(u->aliases); + set_free(u->aliases); free(u->id); activation_details_unref(u->activation_details); @@ -905,7 +905,7 @@ static int unit_merge_names(Unit *u, Unit *other) { } TAKE_PTR(other->id); - other->aliases = set_free_free(other->aliases); + other->aliases = set_free(other->aliases); SET_FOREACH(name, u->aliases) assert_se(hashmap_replace(u->manager->units, name, u) == 0); From d502816b3e8ce22d2500d270f4d0a6e9c4bd1800 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 11 Apr 2025 11:39:49 +0900 Subject: [PATCH 6/6] unit-file: use set_free() to free paths As it uses path_hash_ops_free. --- src/shared/unit-file.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/shared/unit-file.c b/src/shared/unit-file.c index 77985c7917..95ba31e3c6 100644 --- a/src/shared/unit-file.c +++ b/src/shared/unit-file.c @@ -376,7 +376,7 @@ int unit_file_build_name_map( */ _cleanup_hashmap_free_ Hashmap *ids = NULL, *names = NULL; - _cleanup_set_free_free_ Set *paths = NULL; + _cleanup_set_free_ Set *paths = NULL; _cleanup_strv_free_ char **expanded_search_path = NULL; uint64_t timestamp_hash; int r;