diff --git a/src/core/dbus-manager.c b/src/core/dbus-manager.c index 8fdca2da22..2feec0bd7d 100644 --- a/src/core/dbus-manager.c +++ b/src/core/dbus-manager.c @@ -2094,7 +2094,7 @@ static int list_unit_files_by_patterns(sd_bus_message *message, void *userdata, _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; Manager *m = ASSERT_PTR(userdata); UnitFileList *item; - Hashmap *h; + _cleanup_(hashmap_freep) Hashmap *h = NULL; int r; assert(message); @@ -2109,36 +2109,30 @@ static int list_unit_files_by_patterns(sd_bus_message *message, void *userdata, if (r < 0) return r; - h = hashmap_new(&string_hash_ops); + 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); if (r < 0) - goto fail; + return r; r = sd_bus_message_open_container(reply, 'a', "(ss)"); if (r < 0) - goto fail; + return r; HASHMAP_FOREACH(item, h) { r = sd_bus_message_append(reply, "(ss)", item->path, unit_file_state_to_string(item->state)); if (r < 0) - goto fail; + return r; } - unit_file_list_free(h); - r = sd_bus_message_close_container(reply); if (r < 0) return r; return sd_bus_send(NULL, reply, NULL); - -fail: - unit_file_list_free(h); - return r; } static int method_list_unit_files(sd_bus_message *message, void *userdata, sd_bus_error *error) { diff --git a/src/shared/install.c b/src/shared/install.c index 79f9ea9b0e..964092456f 100644 --- a/src/shared/install.c +++ b/src/shared/install.c @@ -3611,7 +3611,7 @@ int unit_file_preset_all( return execute_preset(file_flags, &plus, &minus, &lp, config_path, NULL, mode, changes, n_changes); } -static UnitFileList* unit_file_list_free_one(UnitFileList *f) { +static UnitFileList* unit_file_list_free(UnitFileList *f) { if (!f) return NULL; @@ -3619,11 +3619,15 @@ static UnitFileList* unit_file_list_free_one(UnitFileList *f) { return mfree(f); } -Hashmap* unit_file_list_free(Hashmap *h) { - return hashmap_free_with_destructor(h, unit_file_list_free_one); -} +DEFINE_TRIVIAL_CLEANUP_FUNC(UnitFileList*, unit_file_list_free); -DEFINE_TRIVIAL_CLEANUP_FUNC(UnitFileList*, unit_file_list_free_one); +DEFINE_HASH_OPS_WITH_VALUE_DESTRUCTOR( + unit_file_list_hash_ops_free, + char, + string_hash_func, + string_compare_func, + UnitFileList, + unit_file_list_free); int unit_file_get_list( RuntimeScope scope, @@ -3659,7 +3663,7 @@ int unit_file_get_list( } FOREACH_DIRENT(de, d, return -errno) { - _cleanup_(unit_file_list_free_onep) UnitFileList *f = NULL; + _cleanup_(unit_file_list_freep) UnitFileList *f = NULL; if (!unit_name_is_valid(de->d_name, UNIT_NAME_ANY)) continue; diff --git a/src/shared/install.h b/src/shared/install.h index 0f9724f999..2067969f83 100644 --- a/src/shared/install.h +++ b/src/shared/install.h @@ -195,7 +195,8 @@ int unit_file_get_state(RuntimeScope scope, const char *root_dir, const char *fi int unit_file_exists(RuntimeScope scope, const LookupPaths *paths, const char *name); int unit_file_get_list(RuntimeScope scope, const char *root_dir, Hashmap *h, char **states, char **patterns); -Hashmap* unit_file_list_free(Hashmap *h); + +extern const struct hash_ops unit_file_list_hash_ops_free; 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-is-enabled.c b/src/systemctl/systemctl-is-enabled.c index bc4fc54c5a..8d791deaf1 100644 --- a/src/systemctl/systemctl-is-enabled.c +++ b/src/systemctl/systemctl-is-enabled.c @@ -14,6 +14,8 @@ static int show_installation_targets_client_side(const char *name) { char **p; int r; + CLEANUP_ARRAY(changes, n_changes, install_changes_free); + p = STRV_MAKE(name); flags = UNIT_FILE_DRY_RUN | (arg_runtime ? UNIT_FILE_RUNTIME : 0); diff --git a/src/systemctl/systemctl-list-unit-files.c b/src/systemctl/systemctl-list-unit-files.c index 72b377cde6..69f0d977c1 100644 --- a/src/systemctl/systemctl-list-unit-files.c +++ b/src/systemctl/systemctl-list-unit-files.c @@ -136,6 +136,7 @@ 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_freep) Hashmap *h = NULL; unsigned c = 0; const char *state; char *path; @@ -143,40 +144,31 @@ int verb_list_unit_files(int argc, char *argv[], void *userdata) { bool fallback = false; if (install_client_side()) { - Hashmap *h; UnitFileList *u; unsigned n_units; - h = hashmap_new(&string_hash_ops); + 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)); - if (r < 0) { - unit_file_list_free(h); + if (r < 0) return log_error_errno(r, "Failed to get unit file list: %m"); - } n_units = hashmap_size(h); - units = new(UnitFileList, n_units ?: 1); /* avoid malloc(0) */ - if (!units) { - unit_file_list_free(h); + units = new(UnitFileList, n_units); + if (!units) return log_oom(); - } HASHMAP_FOREACH(u, h) { if (!output_show_unit_file(u, NULL, NULL)) continue; units[c++] = *u; - free(u); } assert(c <= n_units); - hashmap_free(h); - - r = 0; } else { _cleanup_(sd_bus_message_unrefp) sd_bus_message *m = NULL; _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; @@ -262,10 +254,6 @@ int verb_list_unit_files(int argc, char *argv[], void *userdata) { if (r < 0) return r; - if (install_client_side()) - for (UnitFileList *unit = units; unit < units + c; unit++) - free(unit->path); - if (c == 0) return -ENOENT; diff --git a/src/test/test-install-root.c b/src/test/test-install-root.c index ea8015d33d..c866cff022 100644 --- a/src/test/test-install-root.c +++ b/src/test/test-install-root.c @@ -578,7 +578,7 @@ TEST(preset_and_list) { UnitFileState state; bool got_yes = false, got_no = false; UnitFileList *fl; - Hashmap *h; + _cleanup_(hashmap_freep) Hashmap *h = NULL; assert_se(unit_file_get_state(RUNTIME_SCOPE_SYSTEM, root, "preset-yes.service", &state) == -ENOENT); assert_se(unit_file_get_state(RUNTIME_SCOPE_SYSTEM, root, "preset-no.service", &state) == -ENOENT); @@ -653,7 +653,7 @@ TEST(preset_and_list) { assert_se(unit_file_get_state(RUNTIME_SCOPE_SYSTEM, root, "preset-yes.service", &state) >= 0 && state == UNIT_FILE_ENABLED); assert_se(unit_file_get_state(RUNTIME_SCOPE_SYSTEM, root, "preset-no.service", &state) >= 0 && state == UNIT_FILE_DISABLED); - assert_se(h = hashmap_new(&string_hash_ops)); + 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); p = strjoina(root, "/usr/lib/systemd/system/preset-yes.service"); @@ -673,8 +673,6 @@ TEST(preset_and_list) { assert_se(IN_SET(fl->state, UNIT_FILE_DISABLED, UNIT_FILE_STATIC, UNIT_FILE_INDIRECT, UNIT_FILE_ALIAS)); } - unit_file_list_free(h); - assert_se(got_yes && got_no); } diff --git a/src/test/test-install.c b/src/test/test-install.c index eb02795096..9fba0a8ca8 100644 --- a/src/test/test-install.c +++ b/src/test/test-install.c @@ -20,7 +20,7 @@ static void dump_changes(InstallChange *c, unsigned n) { } int main(int argc, char* argv[]) { - Hashmap *h; + _cleanup_(hashmap_freep) Hashmap *h = NULL; UnitFileList *p; int r; const char *const files[] = { "avahi-daemon.service", NULL }; @@ -31,7 +31,7 @@ int main(int argc, char* argv[]) { test_setup_logging(LOG_DEBUG); - h = hashmap_new(&string_hash_ops); + 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); @@ -48,8 +48,6 @@ int main(int argc, char* argv[]) { unit_file_state_to_string(p->state)); } - unit_file_list_free(h); - log_info("/*** enable **/"); r = unit_file_enable(RUNTIME_SCOPE_SYSTEM, 0, NULL, (char**) files, &changes, &n_changes); diff --git a/src/test/test-load-fragment.c b/src/test/test-load-fragment.c index 39fcdcd94a..708abb7026 100644 --- a/src/test/test-load-fragment.c +++ b/src/test/test-load-fragment.c @@ -41,10 +41,10 @@ STATIC_DESTRUCTOR_REGISTER(runtime_dir, rm_rf_physical_and_freep); TEST_RET(unit_file_get_set) { int r; - Hashmap *h; + _cleanup_(hashmap_freep) Hashmap *h = NULL; UnitFileList *p; - h = hashmap_new(&string_hash_ops); + h = hashmap_new(&unit_file_list_hash_ops_free); assert_se(h); r = unit_file_get_list(RUNTIME_SCOPE_SYSTEM, NULL, h, NULL, NULL); @@ -59,8 +59,6 @@ TEST_RET(unit_file_get_set) { HASHMAP_FOREACH(p, h) printf("%s = %s\n", p->path, unit_file_state_to_string(p->state)); - unit_file_list_free(h); - return 0; } diff --git a/test/units/testsuite-26.sh b/test/units/testsuite-26.sh index 7c046932fb..dc8e1e47c6 100755 --- a/test/units/testsuite-26.sh +++ b/test/units/testsuite-26.sh @@ -92,8 +92,6 @@ systemctl list-sockets --legend=no -a "*journal*" systemctl list-sockets --show-types systemctl list-sockets --state=listening systemctl list-timers -a -l -systemctl list-unit-files -systemctl list-unit-files "*journal*" systemctl list-jobs systemctl list-jobs --after systemctl list-jobs --before @@ -105,6 +103,14 @@ systemctl list-dependencies sysinit.target --state=mounted --all systemctl list-paths systemctl list-paths --legend=no -a "systemd*" +test_list_unit_files() { + systemctl list-unit-files "$@" + systemctl list-unit-files "$@" "*journal*" +} + +test_list_unit_files +test_list_unit_files --root=/ + # is-* verbs # Should return 4 for a missing unit file assert_rc 4 systemctl --quiet is-active not-found.service @@ -132,45 +138,55 @@ systemctl stop "$UNIT_NAME" (! systemctl is-active "$UNIT_NAME") # enable/disable/preset -(! systemctl is-enabled "$UNIT_NAME") -systemctl enable "$UNIT_NAME" -systemctl is-enabled -l "$UNIT_NAME" -# We created a preset file for this unit above with a "disable" policy -systemctl preset "$UNIT_NAME" -(! systemctl is-enabled "$UNIT_NAME") -systemctl reenable "$UNIT_NAME" -systemctl is-enabled "$UNIT_NAME" -systemctl preset --preset-mode=enable-only "$UNIT_NAME" -systemctl is-enabled "$UNIT_NAME" -systemctl preset --preset-mode=disable-only "$UNIT_NAME" -(! systemctl is-enabled "$UNIT_NAME") -systemctl enable --runtime "$UNIT_NAME" -[[ -e "/run/systemd/system/multi-user.target.wants/$UNIT_NAME" ]] -systemctl is-enabled "$UNIT_NAME" -systemctl disable "$UNIT_NAME" -# The unit should be still enabled, as we didn't use the --runtime switch -systemctl is-enabled "$UNIT_NAME" -systemctl disable --runtime "$UNIT_NAME" -(! systemctl is-enabled "$UNIT_NAME") +test_enable_disable_preset() { + (! systemctl is-enabled "$@" "$UNIT_NAME") + systemctl enable "$@" "$UNIT_NAME" + systemctl is-enabled "$@" -l "$UNIT_NAME" + # We created a preset file for this unit above with a "disable" policy + systemctl preset "$@" "$UNIT_NAME" + (! systemctl is-enabled "$@" "$UNIT_NAME") + systemctl reenable "$@" "$UNIT_NAME" + systemctl is-enabled "$@" "$UNIT_NAME" + systemctl preset "$@" --preset-mode=enable-only "$UNIT_NAME" + systemctl is-enabled "$@" "$UNIT_NAME" + systemctl preset "$@" --preset-mode=disable-only "$UNIT_NAME" + (! systemctl is-enabled "$@" "$UNIT_NAME") + systemctl enable "$@" --runtime "$UNIT_NAME" + [[ -e "/run/systemd/system/multi-user.target.wants/$UNIT_NAME" ]] + systemctl is-enabled "$@" "$UNIT_NAME" + systemctl disable "$@" "$UNIT_NAME" + # The unit should be still enabled, as we didn't use the --runtime switch + systemctl is-enabled "$@" "$UNIT_NAME" + systemctl disable "$@" --runtime "$UNIT_NAME" + (! systemctl is-enabled "$@" "$UNIT_NAME") +} + +test_enable_disable_preset +test_enable_disable_preset --root=/ # mask/unmask/revert -systemctl disable "$UNIT_NAME" -[[ "$(systemctl is-enabled "$UNIT_NAME")" == disabled ]] -systemctl mask "$UNIT_NAME" -[[ "$(systemctl is-enabled "$UNIT_NAME")" == masked ]] -systemctl unmask "$UNIT_NAME" -[[ "$(systemctl is-enabled "$UNIT_NAME")" == disabled ]] -systemctl mask "$UNIT_NAME" -[[ "$(systemctl is-enabled "$UNIT_NAME")" == masked ]] -systemctl revert "$UNIT_NAME" -[[ "$(systemctl is-enabled "$UNIT_NAME")" == disabled ]] -systemctl mask --runtime "$UNIT_NAME" -[[ "$(systemctl is-enabled "$UNIT_NAME")" == masked-runtime ]] -# This should be a no-op without the --runtime switch -systemctl unmask "$UNIT_NAME" -[[ "$(systemctl is-enabled "$UNIT_NAME")" == masked-runtime ]] -systemctl unmask --runtime "$UNIT_NAME" -[[ "$(systemctl is-enabled "$UNIT_NAME")" == disabled ]] +test_mask_unmask_revert() { + systemctl disable "$@" "$UNIT_NAME" + [[ "$(systemctl is-enabled "$@" "$UNIT_NAME")" == disabled ]] + systemctl mask "$@" "$UNIT_NAME" + [[ "$(systemctl is-enabled "$@" "$UNIT_NAME")" == masked ]] + systemctl unmask "$@" "$UNIT_NAME" + [[ "$(systemctl is-enabled "$@" "$UNIT_NAME")" == disabled ]] + systemctl mask "$@" "$UNIT_NAME" + [[ "$(systemctl is-enabled "$@" "$UNIT_NAME")" == masked ]] + systemctl revert "$@" "$UNIT_NAME" + [[ "$(systemctl is-enabled "$@" "$UNIT_NAME")" == disabled ]] + systemctl mask "$@" --runtime "$UNIT_NAME" + [[ "$(systemctl is-enabled "$@" "$UNIT_NAME")" == masked-runtime ]] + # This should be a no-op without the --runtime switch + systemctl unmask "$@" "$UNIT_NAME" + [[ "$(systemctl is-enabled "$@" "$UNIT_NAME")" == masked-runtime ]] + systemctl unmask "$@" --runtime "$UNIT_NAME" + [[ "$(systemctl is-enabled "$@" "$UNIT_NAME")" == disabled ]] +} + +test_mask_unmask_revert +test_mask_unmask_revert --root=/ # add-wants/add-requires (! systemctl show -P Wants "$UNIT_NAME" | grep "systemd-journald.service") @@ -229,11 +245,16 @@ for value in pretty us µs utc us+utc µs+utc; do done # set-default/get-default -target="$(systemctl get-default)" -systemctl set-default emergency.target -[[ "$(systemctl get-default)" == emergency.target ]] -systemctl set-default "$target" -[[ "$(systemctl get-default)" == "$target" ]] +test_get_set_default() { + target="$(systemctl get-default "$@")" + systemctl set-default "$@" emergency.target + [[ "$(systemctl get-default "$@")" == emergency.target ]] + systemctl set-default "$@" "$target" + [[ "$(systemctl get-default "$@")" == "$target" ]] +} + +test_get_set_default +test_get_set_default --root=/ # show/status systemctl show --property ""