From: Mike Yuan Date: Wed, 1 May 2024 08:26:05 +0000 (+0800) Subject: shared/install: modernize unit_file_get_list, use key destructor X-Git-Tag: v257-rc1~1198^2~4 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=ca3c95c1c532d12a8d3a3ffca8b7400313b982b2;p=thirdparty%2Fsystemd.git shared/install: modernize unit_file_get_list, use key destructor The rest of the basename()s are easy to drop. --- diff --git a/src/core/dbus-manager.c b/src/core/dbus-manager.c index 2fc4161b813..6001dccd6ea 100644 --- a/src/core/dbus-manager.c +++ b/src/core/dbus-manager.c @@ -2187,9 +2187,8 @@ static int method_enqueue_marked_jobs(sd_bus_message *message, void *userdata, s } static int list_unit_files_by_patterns(sd_bus_message *message, void *userdata, sd_bus_error *error, char **states, char **patterns) { - _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; Manager *m = ASSERT_PTR(userdata); - UnitFileList *item; + _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; _cleanup_hashmap_free_ Hashmap *h = NULL; int r; @@ -2205,11 +2204,7 @@ static int list_unit_files_by_patterns(sd_bus_message *message, void *userdata, if (r < 0) return r; - h = hashmap_new(&unit_file_list_hash_ops_free); - if (!h) - return -ENOMEM; - - r = unit_file_get_list(m->runtime_scope, NULL, h, states, patterns); + r = unit_file_get_list(m->runtime_scope, /* root_dir = */ NULL, states, patterns, &h); if (r < 0) return r; @@ -2217,8 +2212,8 @@ static int list_unit_files_by_patterns(sd_bus_message *message, void *userdata, if (r < 0) return r; + UnitFileList *item; HASHMAP_FOREACH(item, h) { - r = sd_bus_message_append(reply, "(ss)", item->path, unit_file_state_to_string(item->state)); if (r < 0) return r; diff --git a/src/shared/install.c b/src/shared/install.c index cad30b10e5b..ae0d254f081 100644 --- a/src/shared/install.c +++ b/src/shared/install.c @@ -3714,27 +3714,24 @@ static UnitFileList* unit_file_list_free(UnitFileList *f) { DEFINE_TRIVIAL_CLEANUP_FUNC(UnitFileList*, unit_file_list_free); -DEFINE_HASH_OPS_WITH_VALUE_DESTRUCTOR( - unit_file_list_hash_ops_free, - char, - string_hash_func, - string_compare_func, - UnitFileList, - unit_file_list_free); +DEFINE_PRIVATE_HASH_OPS_FULL(unit_file_list_hash_ops_free_free, + char, string_hash_func, string_compare_func, free, + UnitFileList, unit_file_list_free); int unit_file_get_list( RuntimeScope scope, const char *root_dir, - Hashmap *h, - char **states, - char **patterns) { + char * const *states, + char * const *patterns, + Hashmap **ret) { _cleanup_(lookup_paths_done) LookupPaths lp = {}; + _cleanup_hashmap_free_ Hashmap *h = NULL; int r; assert(scope >= 0); assert(scope < _RUNTIME_SCOPE_MAX); - assert(h); + assert(ret); r = lookup_paths_init(&lp, scope, 0, root_dir); if (r < 0) @@ -3756,7 +3753,11 @@ int unit_file_get_list( } FOREACH_DIRENT(de, d, return -errno) { - _cleanup_(unit_file_list_freep) UnitFileList *f = NULL; + if (!IN_SET(de->d_type, DT_LNK, DT_REG)) + continue; + + if (hashmap_contains(h, de->d_name)) + continue; if (!unit_name_is_valid(de->d_name, UNIT_NAME_ANY)) continue; @@ -3764,36 +3765,42 @@ int unit_file_get_list( if (!strv_fnmatch_or_empty(patterns, de->d_name, FNM_NOESCAPE)) continue; - if (hashmap_get(h, de->d_name)) - continue; + UnitFileState state; - if (!IN_SET(de->d_type, DT_LNK, DT_REG)) + r = unit_file_lookup_state(scope, &lp, de->d_name, &state); + if (r < 0) + state = UNIT_FILE_BAD; + + if (!strv_isempty(states) && + !strv_contains(states, unit_file_state_to_string(state))) continue; - f = new0(UnitFileList, 1); + _cleanup_(unit_file_list_freep) UnitFileList *f = new(UnitFileList, 1); if (!f) return -ENOMEM; - f->path = path_make_absolute(de->d_name, *dirname); + *f = (UnitFileList) { + .path = path_make_absolute(de->d_name, *dirname), + .state = state, + }; if (!f->path) return -ENOMEM; - r = unit_file_lookup_state(scope, &lp, de->d_name, &f->state); - if (r < 0) - f->state = UNIT_FILE_BAD; - - if (!strv_isempty(states) && - !strv_contains(states, unit_file_state_to_string(f->state))) - continue; + _cleanup_free_ char *unit_name = strdup(de->d_name); + if (!unit_name) + return -ENOMEM; - r = hashmap_put(h, basename(f->path), f); + r = hashmap_ensure_put(&h, &unit_file_list_hash_ops_free_free, unit_name, f); if (r < 0) return r; + assert(r > 0); - f = NULL; /* prevent cleanup */ + TAKE_PTR(unit_name); + TAKE_PTR(f); } } + *ret = TAKE_PTR(h); return 0; } diff --git a/src/shared/install.h b/src/shared/install.h index c8308837ffa..fdd3f363542 100644 --- a/src/shared/install.h +++ b/src/shared/install.h @@ -199,9 +199,7 @@ static inline int unit_file_exists(RuntimeScope scope, const LookupPaths *paths, return unit_file_exists_full(scope, paths, name, NULL); } -int unit_file_get_list(RuntimeScope scope, const char *root_dir, Hashmap *h, char **states, char **patterns); - -extern const struct hash_ops unit_file_list_hash_ops_free; +int unit_file_get_list(RuntimeScope scope, const char *root_dir, char * const *states, char * const *patterns, Hashmap **ret); InstallChangeType install_changes_add(InstallChange **changes, size_t *n_changes, InstallChangeType type, const char *path, const char *source); void install_changes_free(InstallChange *changes, size_t n_changes); diff --git a/src/systemctl/systemctl-list-unit-files.c b/src/systemctl/systemctl-list-unit-files.c index b8b1531834f..943d7ffec52 100644 --- a/src/systemctl/systemctl-list-unit-files.c +++ b/src/systemctl/systemctl-list-unit-files.c @@ -142,23 +142,15 @@ static int output_unit_file_list(const UnitFileList *units, unsigned c) { int verb_list_unit_files(int argc, char *argv[], void *userdata) { _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; - _cleanup_free_ UnitFileList *units = NULL; _cleanup_hashmap_free_ Hashmap *h = NULL; + _cleanup_free_ UnitFileList *units = NULL; unsigned c = 0; - const char *state; - char *path; int r; - bool fallback = false; if (install_client_side()) { - UnitFileList *u; unsigned n_units; - h = hashmap_new(&unit_file_list_hash_ops_free); - if (!h) - return log_oom(); - - r = unit_file_get_list(arg_runtime_scope, arg_root, h, arg_states, strv_skip(argv, 1)); + r = unit_file_get_list(arg_runtime_scope, arg_root, arg_states, strv_skip(argv, 1), &h); if (r < 0) return log_error_errno(r, "Failed to get unit file list: %m"); @@ -168,6 +160,7 @@ int verb_list_unit_files(int argc, char *argv[], void *userdata) { if (!units) return log_oom(); + UnitFileList *u; HASHMAP_FOREACH(u, h) { if (!output_show_unit_file(u, NULL, NULL)) continue; @@ -179,6 +172,8 @@ int verb_list_unit_files(int argc, char *argv[], void *userdata) { } else { _cleanup_(sd_bus_message_unrefp) sd_bus_message *m = NULL; _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; + const char *path, *state; + bool fallback = false; sd_bus *bus; r = acquire_bus(BUS_MANAGER, &bus); @@ -201,19 +196,17 @@ int verb_list_unit_files(int argc, char *argv[], void *userdata) { return log_error_errno(r, "Failed to append unit dependencies: %m"); r = sd_bus_message_append_strv(m, names_with_deps); - if (r < 0) - return bus_log_create_error(r); - } else { + } else r = sd_bus_message_append_strv(m, strv_skip(argv, 1)); - if (r < 0) - return bus_log_create_error(r); - } + if (r < 0) + return bus_log_create_error(r); r = sd_bus_call(bus, m, 0, &error, &reply); if (r < 0 && sd_bus_error_has_name(&error, SD_BUS_ERROR_UNKNOWN_METHOD)) { /* Fallback to legacy ListUnitFiles method */ + log_debug_errno(r, "Unable to list unit files through ListUnitFilesByPatterns, falling back to ListUnitsFiles method."); + fallback = true; - log_debug_errno(r, "Failed to list unit files: %s Falling back to ListUnitsFiles method.", bus_error_message(&error, r)); m = sd_bus_message_unref(m); sd_bus_error_free(&error); @@ -235,16 +228,15 @@ int verb_list_unit_files(int argc, char *argv[], void *userdata) { if (!GREEDY_REALLOC(units, c + 1)) return log_oom(); - units[c] = (struct UnitFileList) { - path, - unit_file_state_from_string(state) + units[c] = (UnitFileList) { + .path = (char*) path, + .state = unit_file_state_from_string(state), }; if (output_show_unit_file(&units[c], fallback ? arg_states : NULL, fallback ? strv_skip(argv, 1) : NULL)) c++; - } if (r < 0) return bus_log_parse_error(r); diff --git a/src/test/test-install-root.c b/src/test/test-install-root.c index 1e7ed27f63b..fbfab6d69bf 100644 --- a/src/test/test-install-root.c +++ b/src/test/test-install-root.c @@ -680,8 +680,7 @@ TEST(preset_and_list) { assert_se(unit_file_get_state(RUNTIME_SCOPE_SYSTEM, root, "preset-no.service", &state) >= 0 && state == UNIT_FILE_DISABLED); assert_se(unit_file_get_state(RUNTIME_SCOPE_SYSTEM, root, "preset-ignore.service", &state) >= 0 && state == UNIT_FILE_DISABLED); - assert_se(h = hashmap_new(&unit_file_list_hash_ops_free)); - assert_se(unit_file_get_list(RUNTIME_SCOPE_SYSTEM, root, h, NULL, NULL) >= 0); + ASSERT_OK(unit_file_get_list(RUNTIME_SCOPE_SYSTEM, root, NULL, NULL, &h)); p = strjoina(root, "/usr/lib/systemd/system/preset-yes.service"); q = strjoina(root, "/usr/lib/systemd/system/preset-no.service"); diff --git a/src/test/test-install.c b/src/test/test-install.c index b54252efc8b..5747b84ba52 100644 --- a/src/test/test-install.c +++ b/src/test/test-install.c @@ -31,9 +31,7 @@ int main(int argc, char* argv[]) { test_setup_logging(LOG_DEBUG); - h = hashmap_new(&unit_file_list_hash_ops_free); - r = unit_file_get_list(RUNTIME_SCOPE_SYSTEM, NULL, h, NULL, NULL); - assert_se(r == 0); + ASSERT_OK(unit_file_get_list(RUNTIME_SCOPE_SYSTEM, NULL, NULL, NULL, &h)); HASHMAP_FOREACH(p, h) { UnitFileState s = _UNIT_FILE_STATE_INVALID; diff --git a/src/test/test-load-fragment.c b/src/test/test-load-fragment.c index 94904e531e7..d1a51756ed3 100644 --- a/src/test/test-load-fragment.c +++ b/src/test/test-load-fragment.c @@ -42,15 +42,12 @@ STATIC_DESTRUCTOR_REGISTER(runtime_dir, rm_rf_physical_and_freep); /* For testing type compatibility. */ _unused_ ConfigPerfItemLookup unused_lookup = load_fragment_gperf_lookup; -TEST_RET(unit_file_get_set) { +TEST_RET(unit_file_get_list) { int r; _cleanup_hashmap_free_ Hashmap *h = NULL; UnitFileList *p; - h = hashmap_new(&unit_file_list_hash_ops_free); - assert_se(h); - - r = unit_file_get_list(RUNTIME_SCOPE_SYSTEM, NULL, h, NULL, NULL); + r = unit_file_get_list(RUNTIME_SCOPE_SYSTEM, NULL, NULL, NULL, &h); if (IN_SET(r, -EPERM, -EACCES)) return log_tests_skipped_errno(r, "unit_file_get_list");