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 2fc4161b813dcd15587d7463ed337c65596dc137..6001dccd6eac16cb81ede768cd6acf42554c7dfb 100644 (file)
--- 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 cad30b10e5b2f8f725b1f2d4ac933f5c93ba7908..ae0d254f0816b8430b46b2b2c981e7a4ef928a91 100644 (file)
--- 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 c8308837ffa788b238bcd5ff166e7942cdeab27d..fdd3f363542f611cc2c34cf724b3b25f491b8873 100644 (file)
--- 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 b8b1531834f9957c30bf450304d3852b35c7370d..943d7ffec5262fb3809d2b56968e120440ed7fa7 100644 (file)
--- 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 1e7ed27f63b4fb90a7a6710f286d24fe0d8a24f2..fbfab6d69bfe76912ad6769e4f17029054efb8ab 100644 (file)
--- 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 b54252efc8bd2259ada506b3d77cd44c73df3d27..5747b84ba52af6b1c878a90e9f43440b749185a6 100644 (file)
--- 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 94904e531e724029c0cdd4e71250fdb555e2ad04..d1a51756ed3df5358509db3c6965e9dcdcc34002 100644 (file)
--- 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");