From d654b9dcfd325fd1fabe78a61123f9098069301e Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 11 Jun 2025 21:54:35 +0900 Subject: [PATCH 1/3] discover-image: coding style fixlets --- src/shared/discover-image.c | 6 +++--- src/shared/discover-image.h | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/shared/discover-image.c b/src/shared/discover-image.c index 71d64bed8d..099f5e03f3 100644 --- a/src/shared/discover-image.c +++ b/src/shared/discover-image.c @@ -117,7 +117,7 @@ static const char *const image_root_runtime_table[_IMAGE_CLASS_MAX] = { DEFINE_STRING_TABLE_LOOKUP_TO_STRING(image_root_runtime, ImageClass); -static Image *image_free(Image *i) { +static Image* image_free(Image *i) { assert(i); free(i->name); @@ -136,7 +136,7 @@ DEFINE_TRIVIAL_REF_UNREF_FUNC(Image, image, image_free); DEFINE_HASH_OPS_WITH_VALUE_DESTRUCTOR(image_hash_ops, char, string_hash_func, string_compare_func, Image, image_unref); -static char **image_settings_path(Image *image) { +static char** image_settings_path(Image *image) { _cleanup_strv_free_ char **l = NULL; _cleanup_free_ char *fn = NULL; size_t i = 0; @@ -653,7 +653,7 @@ static int pick_image_search_path( return 0; } -static char **make_possible_filenames(ImageClass class, const char *image_name) { +static char** make_possible_filenames(ImageClass class, const char *image_name) { _cleanup_strv_free_ char **l = NULL; assert(image_name); diff --git a/src/shared/discover-image.h b/src/shared/discover-image.h index 4b586f68dd..142c001f53 100644 --- a/src/shared/discover-image.h +++ b/src/shared/discover-image.h @@ -45,8 +45,8 @@ typedef struct Image { void *userdata; } Image; -Image *image_unref(Image *i); -Image *image_ref(Image *i); +Image* image_unref(Image *i); +Image* image_ref(Image *i); DEFINE_TRIVIAL_CLEANUP_FUNC(Image*, image_unref); @@ -73,7 +73,7 @@ int image_read_metadata(Image *i, const ImagePolicy *image_policy); bool image_in_search_path(RuntimeScope scope, ImageClass class, const char *root, const char *image); -static inline char **image_extension_release(Image *image, ImageClass class) { +static inline char** image_extension_release(Image *image, ImageClass class) { assert(image); if (class == IMAGE_SYSEXT) From 624d3698689150c2454e9dd69b04ce01fe65dc7c Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 11 Jun 2025 22:22:55 +0900 Subject: [PATCH 2/3] 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 7b78a46d25..8dd8439ab8 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 a361ea7478..2b59c965f7 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 c91cedf12d..9142157ea4 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 3e903b3f3f..89e1c68e25 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 b13546df09..222e3934d9 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 b429ceb974..f3351724b4 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 e347db5c0d..bc62d31c22 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 8972da0802..caad5d4b57 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 961ca4588a..dd6e405bda 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 ababa411a8..8497a42655 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 099f5e03f3..089d656f40 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 142c001f53..60f5a4dce1 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 a4aea64380..ffc3ebcf2c 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 fa04515fee..3d65d99389 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; From d604341ca5c16a994f6fc8826c095deddfe3b2ba Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 11 Jun 2025 22:26:04 +0900 Subject: [PATCH 3/3] sysext: drop unnecessary struct MethodListParameters --- src/sysext/sysext.c | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/src/sysext/sysext.c b/src/sysext/sysext.c index ffc3ebcf2c..b232ee4fc1 100644 --- a/src/sysext/sysext.c +++ b/src/sysext/sysext.c @@ -2362,29 +2362,24 @@ static int verb_list(int argc, char **argv, void *userdata) { return table_print_with_pager(t, arg_json_format_flags, arg_pager_flags, arg_legend); } -typedef struct MethodListParameters { - const char *class; -} MethodListParameters; - static int vl_method_list(sd_varlink *link, sd_json_variant *parameters, sd_varlink_method_flags_t flags, void *userdata) { static const sd_json_dispatch_field dispatch_table[] = { - { "class", SD_JSON_VARIANT_STRING, sd_json_dispatch_const_string, offsetof(MethodListParameters, class), 0 }, + { "class", SD_JSON_VARIANT_STRING, sd_json_dispatch_const_string, 0, 0 }, {} }; - MethodListParameters p = { - }; _cleanup_(sd_json_variant_unrefp) sd_json_variant *v = NULL; int r; assert(link); - r = sd_varlink_dispatch(link, parameters, dispatch_table, &p); + const char *class = NULL; + r = sd_varlink_dispatch(link, parameters, dispatch_table, &class); if (r != 0) return r; ImageClass image_class = arg_image_class; - r = parse_image_class_parameter(link, p.class, &image_class, NULL); + r = parse_image_class_parameter(link, class, &image_class, NULL); if (r < 0) return r;