Merge pull request #27027 from dtardon/unit-file-list-cleanup

Use _cleanup_ for UnitFileList hash
This commit is contained in:
Lennart Poettering
2023-04-13 09:10:17 +02:00
committed by GitHub
9 changed files with 95 additions and 91 deletions
+5 -11
View File
@@ -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) {
+10 -6
View File
@@ -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;
+2 -1
View File
@@ -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);
+2
View File
@@ -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);
+5 -17
View File
@@ -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;
+2 -4
View File
@@ -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);
}
+2 -4
View File
@@ -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);
+2 -4
View File
@@ -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;
}
+65 -44
View File
@@ -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 ""