From 624d3698689150c2454e9dd69b04ce01fe65dc7c Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 11 Jun 2025 22:22:55 +0900 Subject: [PATCH] discover-image: make image_discover() allocate hashmap when necessary --- src/dissect/dissect.c | 11 +++-------- src/import/importd.c | 10 +++------- src/machine/image-dbus.c | 12 ++++-------- src/machine/image.c | 13 ++++--------- src/machine/machined-dbus.c | 12 ++++-------- src/machine/machined-varlink.c | 7 ++----- src/portable/portabled-bus.c | 6 +----- src/portable/portabled-image-bus.c | 6 +----- src/portable/portabled-image.c | 10 +++++++--- src/portable/portabled-image.h | 2 +- src/shared/discover-image.c | 12 ++++++------ src/shared/discover-image.h | 2 +- src/sysext/sysext.c | 31 +++++++++--------------------- src/sysupdate/sysupdated.c | 6 +----- 14 files changed, 47 insertions(+), 93 deletions(-) diff --git a/src/dissect/dissect.c b/src/dissect/dissect.c index 7b78a46d251..8dd8439ab8c 100644 --- a/src/dissect/dissect.c +++ b/src/dissect/dissect.c @@ -1973,16 +1973,10 @@ static int action_with(DissectedImage *m, LoopDevice *d) { static int action_discover(void) { _cleanup_hashmap_free_ Hashmap *images = NULL; - _cleanup_(table_unrefp) Table *t = NULL; - Image *img; int r; - images = hashmap_new(&image_hash_ops); - if (!images) - return log_oom(); - for (ImageClass cl = 0; cl < _IMAGE_CLASS_MAX; cl++) { - r = image_discover(arg_runtime_scope, cl, NULL, images); + r = image_discover(arg_runtime_scope, cl, NULL, &images); if (r < 0) return log_error_errno(r, "Failed to discover images: %m"); } @@ -1992,13 +1986,14 @@ static int action_discover(void) { return 0; } - t = table_new("name", "type", "class", "ro", "path", "time", "usage"); + _cleanup_(table_unrefp) Table *t = table_new("name", "type", "class", "ro", "path", "time", "usage"); if (!t) return log_oom(); table_set_align_percent(t, table_get_cell(t, 0, 6), 100); table_set_ersatz_string(t, TABLE_ERSATZ_DASH); + Image *img; HASHMAP_FOREACH(img, images) { if (!arg_all && startswith(img->name, ".")) diff --git a/src/import/importd.c b/src/import/importd.c index a361ea74788..2b59c965f75 100644 --- a/src/import/importd.c +++ b/src/import/importd.c @@ -1333,13 +1333,9 @@ static int method_list_images(sd_bus_message *msg, void *userdata, sd_bus_error class < 0 ? (c < _IMAGE_CLASS_MAX) : (c == class); c++) { - _cleanup_hashmap_free_ Hashmap *h = NULL; + _cleanup_hashmap_free_ Hashmap *images = NULL; - h = hashmap_new(&image_hash_ops); - if (!h) - return -ENOMEM; - - r = image_discover(m->runtime_scope, c, /* root= */ NULL, h); + r = image_discover(m->runtime_scope, c, /* root= */ NULL, &images); if (r < 0) { if (class >= 0) return r; @@ -1349,7 +1345,7 @@ static int method_list_images(sd_bus_message *msg, void *userdata, sd_bus_error } Image *i; - HASHMAP_FOREACH(i, h) { + HASHMAP_FOREACH(i, images) { r = sd_bus_message_append( reply, "(ssssbtttttt)", diff --git a/src/machine/image-dbus.c b/src/machine/image-dbus.c index c91cedf12de..9142157ea4b 100644 --- a/src/machine/image-dbus.c +++ b/src/machine/image-dbus.c @@ -396,24 +396,20 @@ char* image_bus_path(const char *name) { } static int image_node_enumerator(sd_bus *bus, const char *path, void *userdata, char ***nodes, sd_bus_error *error) { - _cleanup_hashmap_free_ Hashmap *images = NULL; - _cleanup_strv_free_ char **l = NULL; Manager *m = ASSERT_PTR(userdata); - Image *image; int r; assert(bus); assert(path); assert(nodes); - images = hashmap_new(&image_hash_ops); - if (!images) - return -ENOMEM; - - r = image_discover(m->runtime_scope, IMAGE_MACHINE, NULL, images); + _cleanup_hashmap_free_ Hashmap *images = NULL; + r = image_discover(m->runtime_scope, IMAGE_MACHINE, NULL, &images); if (r < 0) return r; + _cleanup_strv_free_ char **l = NULL; + Image *image; HASHMAP_FOREACH(image, images) { char *p; diff --git a/src/machine/image.c b/src/machine/image.c index 3e903b3f3f7..89e1c68e259 100644 --- a/src/machine/image.c +++ b/src/machine/image.c @@ -115,28 +115,23 @@ int image_clean_pool_operation(Manager *manager, ImageCleanPoolMode mode, Operat if (r < 0) return log_debug_errno(r, "Failed to fork(): %m"); if (r == 0) { - _cleanup_hashmap_free_ Hashmap *images = NULL; - bool success = true; - Image *image; - errno_pipe_fd[0] = safe_close(errno_pipe_fd[0]); - images = hashmap_new(&image_hash_ops); - if (!images) - report_errno_and_exit(errno_pipe_fd[1], ENOMEM); - - r = image_discover(manager->runtime_scope, IMAGE_MACHINE, /* root = */ NULL, images); + _cleanup_hashmap_free_ Hashmap *images = NULL; + r = image_discover(manager->runtime_scope, IMAGE_MACHINE, /* root = */ NULL, &images); if (r < 0) { log_debug_errno(r, "Failed to discover images: %m"); report_errno_and_exit(errno_pipe_fd[1], r); } + bool success = true; ssize_t n = loop_write(result_fd, &success, sizeof(success)); if (n < 0) { log_debug_errno(n, "Failed to write to tmp file: %m"); report_errno_and_exit(errno_pipe_fd[1], n); } + Image *image; HASHMAP_FOREACH(image, images) { /* We can't remove vendor images (i.e. those in /usr) */ if (image_is_vendor(image)) diff --git a/src/machine/machined-dbus.c b/src/machine/machined-dbus.c index b13546df095..222e3934d9c 100644 --- a/src/machine/machined-dbus.c +++ b/src/machine/machined-dbus.c @@ -490,18 +490,13 @@ static int method_get_machine_os_release(sd_bus_message *message, void *userdata static int method_list_images(sd_bus_message *message, void *userdata, sd_bus_error *error) { _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; - _cleanup_hashmap_free_ Hashmap *images = NULL; - _unused_ Manager *m = ASSERT_PTR(userdata); - Image *image; + Manager *m = ASSERT_PTR(userdata); int r; assert(message); - images = hashmap_new(&image_hash_ops); - if (!images) - return -ENOMEM; - - r = image_discover(m->runtime_scope, IMAGE_MACHINE, NULL, images); + _cleanup_hashmap_free_ Hashmap *images = NULL; + r = image_discover(m->runtime_scope, IMAGE_MACHINE, NULL, &images); if (r < 0) return r; @@ -513,6 +508,7 @@ static int method_list_images(sd_bus_message *message, void *userdata, sd_bus_er if (r < 0) return r; + Image *image; HASHMAP_FOREACH(image, images) { _cleanup_free_ char *p = NULL; diff --git a/src/machine/machined-varlink.c b/src/machine/machined-varlink.c index b429ceb974c..f3351724b41 100644 --- a/src/machine/machined-varlink.c +++ b/src/machine/machined-varlink.c @@ -700,11 +700,8 @@ static int vl_method_list_images(sd_varlink *link, sd_json_variant *parameters, if (!FLAGS_SET(flags, SD_VARLINK_METHOD_MORE)) return sd_varlink_error(link, SD_VARLINK_ERROR_EXPECTED_MORE, NULL); - _cleanup_hashmap_free_ Hashmap *images = hashmap_new(&image_hash_ops); - if (!images) - return -ENOMEM; - - r = image_discover(m->runtime_scope, IMAGE_MACHINE, /* root = */ NULL, images); + _cleanup_hashmap_free_ Hashmap *images = NULL; + r = image_discover(m->runtime_scope, IMAGE_MACHINE, /* root = */ NULL, &images); if (r < 0) return log_debug_errno(r, "Failed to discover images: %m"); diff --git a/src/portable/portabled-bus.c b/src/portable/portabled-bus.c index e347db5c0db..bc62d31c22b 100644 --- a/src/portable/portabled-bus.c +++ b/src/portable/portabled-bus.c @@ -141,11 +141,7 @@ static int method_list_images(sd_bus_message *message, void *userdata, sd_bus_er assert(message); - images = hashmap_new(&image_hash_ops); - if (!images) - return -ENOMEM; - - r = manager_image_cache_discover(m, images, error); + r = manager_image_cache_discover(m, &images, error); if (r < 0) return r; diff --git a/src/portable/portabled-image-bus.c b/src/portable/portabled-image-bus.c index 8972da0802f..caad5d4b575 100644 --- a/src/portable/portabled-image-bus.c +++ b/src/portable/portabled-image-bus.c @@ -1156,11 +1156,7 @@ int bus_image_node_enumerator(sd_bus *bus, const char *path, void *userdata, cha assert(path); assert(nodes); - images = hashmap_new(&image_hash_ops); - if (!images) - return -ENOMEM; - - r = manager_image_cache_discover(m, images, error); + r = manager_image_cache_discover(m, &images, error); if (r < 0) return r; diff --git a/src/portable/portabled-image.c b/src/portable/portabled-image.c index 961ca4588a6..dd6e405bda3 100644 --- a/src/portable/portabled-image.c +++ b/src/portable/portabled-image.c @@ -85,8 +85,7 @@ int manager_image_cache_add(Manager *m, Image *image) { return 0; } -int manager_image_cache_discover(Manager *m, Hashmap *images, sd_bus_error *error) { - Image *image; +int manager_image_cache_discover(Manager *m, Hashmap **ret_images, sd_bus_error *error) { int r; assert(m); @@ -94,12 +93,17 @@ int manager_image_cache_discover(Manager *m, Hashmap *images, sd_bus_error *erro /* A wrapper around image_discover() (for finding images in search path) and portable_discover_attached() (for * finding attached images). */ - r = image_discover(m->runtime_scope, IMAGE_PORTABLE, NULL, images); + _cleanup_hashmap_free_ Hashmap *images = NULL; + r = image_discover(m->runtime_scope, IMAGE_PORTABLE, NULL, &images); if (r < 0) return r; + Image *image; HASHMAP_FOREACH(image, images) (void) manager_image_cache_add(m, image); + if (ret_images) + *ret_images = TAKE_PTR(images); + return 0; } diff --git a/src/portable/portabled-image.h b/src/portable/portabled-image.h index ababa411a88..8497a42655d 100644 --- a/src/portable/portabled-image.h +++ b/src/portable/portabled-image.h @@ -7,4 +7,4 @@ Image *manager_image_cache_get(Manager *m, const char *name_or_path); int manager_image_cache_add(Manager *m, Image *image); -int manager_image_cache_discover(Manager *m, Hashmap *images, sd_bus_error *error); +int manager_image_cache_discover(Manager *m, Hashmap **ret_images, sd_bus_error *error); diff --git a/src/shared/discover-image.c b/src/shared/discover-image.c index 099f5e03f33..089d656f400 100644 --- a/src/shared/discover-image.c +++ b/src/shared/discover-image.c @@ -887,7 +887,7 @@ int image_discover( RuntimeScope scope, ImageClass class, const char *root, - Hashmap *h) { + Hashmap **images) { /* As mentioned above, we follow symlinks on this fstatat(), because we want to permit people to * symlink block devices into the search path. (For now, we disable that when operating relative to @@ -897,7 +897,7 @@ int image_discover( assert(scope < _RUNTIME_SCOPE_MAX && scope != RUNTIME_SCOPE_GLOBAL); assert(class >= 0); assert(class < _IMAGE_CLASS_MAX); - assert(h); + assert(images); _cleanup_strv_free_ char **search = NULL; r = pick_image_search_path(scope, class, &search); @@ -1039,7 +1039,7 @@ int image_discover( continue; } - if (hashmap_contains(h, pretty)) + if (hashmap_contains(*images, pretty)) continue; r = image_make(class, pretty, dirfd(d), resolved, fname, fd, &st, &image); @@ -1050,7 +1050,7 @@ int image_discover( image->discoverable = true; - r = hashmap_put(h, image->name, image); + r = hashmap_ensure_put(images, &image_hash_ops, image->name, image); if (r < 0) return r; @@ -1058,7 +1058,7 @@ int image_discover( } } - if (scope == RUNTIME_SCOPE_SYSTEM && class == IMAGE_MACHINE && !hashmap_contains(h, ".host")) { + if (scope == RUNTIME_SCOPE_SYSTEM && class == IMAGE_MACHINE && !hashmap_contains(*images, ".host")) { _cleanup_(image_unrefp) Image *image = NULL; r = image_make(IMAGE_MACHINE, @@ -1074,7 +1074,7 @@ int image_discover( image->discoverable = true; - r = hashmap_put(h, image->name, image); + r = hashmap_ensure_put(images, &image_hash_ops, image->name, image); if (r < 0) return r; diff --git a/src/shared/discover-image.h b/src/shared/discover-image.h index 142c001f53a..60f5a4dce13 100644 --- a/src/shared/discover-image.h +++ b/src/shared/discover-image.h @@ -53,7 +53,7 @@ DEFINE_TRIVIAL_CLEANUP_FUNC(Image*, image_unref); int image_find(RuntimeScope scope, ImageClass class, const char *name, const char *root, Image **ret); int image_from_path(const char *path, Image **ret); int image_find_harder(RuntimeScope scope, ImageClass class, const char *name_or_path, const char *root, Image **ret); -int image_discover(RuntimeScope scope, ImageClass class, const char *root, Hashmap *map); +int image_discover(RuntimeScope scope, ImageClass class, const char *root, Hashmap **images); int image_remove(Image *i); int image_rename(Image *i, const char *new_name, RuntimeScope scope); diff --git a/src/sysext/sysext.c b/src/sysext/sysext.c index a4aea64380c..ffc3ebcf2cb 100644 --- a/src/sysext/sysext.c +++ b/src/sysext/sysext.c @@ -2082,20 +2082,14 @@ static int merge(ImageClass image_class, return 1; } -static int image_discover_and_read_metadata( - ImageClass image_class, - Hashmap **ret_images) { +static int image_discover_and_read_metadata(ImageClass image_class, Hashmap **ret_images) { _cleanup_hashmap_free_ Hashmap *images = NULL; Image *img; int r; assert(ret_images); - images = hashmap_new(&image_hash_ops); - if (!images) - return log_oom(); - - r = image_discover(RUNTIME_SCOPE_SYSTEM, image_class, arg_root, images); + r = image_discover(RUNTIME_SCOPE_SYSTEM, image_class, arg_root, &images); if (r < 0) return log_error_errno(r, "Failed to discover images: %m"); @@ -2105,7 +2099,8 @@ static int image_discover_and_read_metadata( return log_error_errno(r, "Failed to read metadata for image %s: %m", img->name); } - *ret_images = TAKE_PTR(images); + if (ret_images) + *ret_images = TAKE_PTR(images); return 0; } @@ -2338,11 +2333,7 @@ static int verb_list(int argc, char **argv, void *userdata) { Image *img; int r; - images = hashmap_new(&image_hash_ops); - if (!images) - return log_oom(); - - r = image_discover(RUNTIME_SCOPE_SYSTEM, arg_image_class, arg_root, images); + r = image_discover(RUNTIME_SCOPE_SYSTEM, arg_image_class, arg_root, &images); if (r < 0) return log_error_errno(r, "Failed to discover images: %m"); @@ -2384,9 +2375,6 @@ static int vl_method_list(sd_varlink *link, sd_json_variant *parameters, sd_varl MethodListParameters p = { }; _cleanup_(sd_json_variant_unrefp) sd_json_variant *v = NULL; - _cleanup_hashmap_free_ Hashmap *images = NULL; - ImageClass image_class = arg_image_class; - Image *img; int r; assert(link); @@ -2395,18 +2383,17 @@ static int vl_method_list(sd_varlink *link, sd_json_variant *parameters, sd_varl if (r != 0) return r; + ImageClass image_class = arg_image_class; r = parse_image_class_parameter(link, p.class, &image_class, NULL); if (r < 0) return r; - images = hashmap_new(&image_hash_ops); - if (!images) - return -ENOMEM; - - r = image_discover(RUNTIME_SCOPE_SYSTEM, image_class, arg_root, images); + _cleanup_hashmap_free_ Hashmap *images = NULL; + r = image_discover(RUNTIME_SCOPE_SYSTEM, image_class, arg_root, &images); if (r < 0) return r; + Image *img; HASHMAP_FOREACH(img, images) { if (v) { /* Send previous item with more=true */ diff --git a/src/sysupdate/sysupdated.c b/src/sysupdate/sysupdated.c index fa04515feec..3d65d99389b 100644 --- a/src/sysupdate/sysupdated.c +++ b/src/sysupdate/sysupdated.c @@ -1762,11 +1762,7 @@ static int manager_enumerate_image_class(Manager *m, TargetClass class) { Image *image; int r; - images = hashmap_new(&image_hash_ops); - if (!images) - return -ENOMEM; - - r = image_discover(m->runtime_scope, (ImageClass) class, NULL, images); + r = image_discover(m->runtime_scope, (ImageClass) class, NULL, &images); if (r < 0) return r; -- 2.47.3