From 0284fcc0ab3da57cabed4060a5c1d8a9e697034a Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 22 Jan 2025 06:05:53 +0900 Subject: [PATCH 01/15] hash-funcs: introduce several basic hash_ops with value destructor --- src/basic/hash-funcs.c | 78 ++++++++++++++++++++++++------------------ src/basic/hash-funcs.h | 3 ++ 2 files changed, 48 insertions(+), 33 deletions(-) diff --git a/src/basic/hash-funcs.c b/src/basic/hash-funcs.c index 251ee4f069..b122c300b8 100644 --- a/src/basic/hash-funcs.c +++ b/src/basic/hash-funcs.c @@ -10,15 +10,23 @@ void string_hash_func(const char *p, struct siphash *state) { siphash24_compress(p, strlen(p) + 1, state); } -DEFINE_HASH_OPS(string_hash_ops, char, string_hash_func, string_compare_func); -DEFINE_HASH_OPS_WITH_KEY_DESTRUCTOR(string_hash_ops_free, - char, string_hash_func, string_compare_func, free); -DEFINE_HASH_OPS_FULL(string_hash_ops_free_free, - char, string_hash_func, string_compare_func, free, - void, free); -DEFINE_HASH_OPS_FULL(string_hash_ops_free_strv_free, - char, string_hash_func, string_compare_func, free, - char*, strv_free); +DEFINE_HASH_OPS(string_hash_ops, + char, string_hash_func, string_compare_func); +DEFINE_HASH_OPS_WITH_KEY_DESTRUCTOR( + string_hash_ops_free, + char, string_hash_func, string_compare_func, free); +DEFINE_HASH_OPS_WITH_VALUE_DESTRUCTOR( + string_hash_ops_value_free, + char, string_hash_func, string_compare_func, + void, free); +DEFINE_HASH_OPS_FULL( + string_hash_ops_free_free, + char, string_hash_func, string_compare_func, free, + void, free); +DEFINE_HASH_OPS_FULL( + string_hash_ops_free_strv_free, + char, string_hash_func, string_compare_func, free, + char*, strv_free); void path_hash_func(const char *q, struct siphash *state) { bool add_slash = false; @@ -59,12 +67,15 @@ void path_hash_func(const char *q, struct siphash *state) { } } -DEFINE_HASH_OPS(path_hash_ops, char, path_hash_func, path_compare); -DEFINE_HASH_OPS_WITH_KEY_DESTRUCTOR(path_hash_ops_free, - char, path_hash_func, path_compare, free); -DEFINE_HASH_OPS_FULL(path_hash_ops_free_free, - char, path_hash_func, path_compare, free, - void, free); +DEFINE_HASH_OPS(path_hash_ops, + char, path_hash_func, path_compare); +DEFINE_HASH_OPS_WITH_KEY_DESTRUCTOR( + path_hash_ops_free, + char, path_hash_func, path_compare, free); +DEFINE_HASH_OPS_FULL( + path_hash_ops_free_free, + char, path_hash_func, path_compare, free, + void, free); void trivial_hash_func(const void *p, struct siphash *state) { siphash24_compress_typesafe(p, state); @@ -74,23 +85,19 @@ int trivial_compare_func(const void *a, const void *b) { return CMP(a, b); } -const struct hash_ops trivial_hash_ops = { - .hash = trivial_hash_func, - .compare = trivial_compare_func, -}; - -const struct hash_ops trivial_hash_ops_free = { - .hash = trivial_hash_func, - .compare = trivial_compare_func, - .free_key = free, -}; - -const struct hash_ops trivial_hash_ops_free_free = { - .hash = trivial_hash_func, - .compare = trivial_compare_func, - .free_key = free, - .free_value = free, -}; +DEFINE_HASH_OPS(trivial_hash_ops, + void, trivial_hash_func, trivial_compare_func); +DEFINE_HASH_OPS_WITH_KEY_DESTRUCTOR( + trivial_hash_ops_free, + void, trivial_hash_func, trivial_compare_func, free); +DEFINE_HASH_OPS_WITH_VALUE_DESTRUCTOR( + trivial_hash_ops_value_free, + void, trivial_hash_func, trivial_compare_func, + void, free); +DEFINE_HASH_OPS_FULL( + trivial_hash_ops_free_free, + void, trivial_hash_func, trivial_compare_func, free, + void, free); void uint64_hash_func(const uint64_t *p, struct siphash *state) { siphash24_compress_typesafe(*p, state); @@ -100,7 +107,12 @@ int uint64_compare_func(const uint64_t *a, const uint64_t *b) { return CMP(*a, *b); } -DEFINE_HASH_OPS(uint64_hash_ops, uint64_t, uint64_hash_func, uint64_compare_func); +DEFINE_HASH_OPS(uint64_hash_ops, + uint64_t, uint64_hash_func, uint64_compare_func); +DEFINE_HASH_OPS_WITH_VALUE_DESTRUCTOR( + uint64_hash_ops_value_free, + uint64_t, uint64_hash_func, uint64_compare_func, + void, free); #if SIZEOF_DEV_T != 8 void devt_hash_func(const dev_t *p, struct siphash *state) { diff --git a/src/basic/hash-funcs.h b/src/basic/hash-funcs.h index 3804e94d98..d0736807ba 100644 --- a/src/basic/hash-funcs.h +++ b/src/basic/hash-funcs.h @@ -77,6 +77,7 @@ void string_hash_func(const char *p, struct siphash *state); #define string_compare_func strcmp extern const struct hash_ops string_hash_ops; extern const struct hash_ops string_hash_ops_free; +extern const struct hash_ops string_hash_ops_value_free; extern const struct hash_ops string_hash_ops_free_free; extern const struct hash_ops string_hash_ops_free_strv_free; @@ -91,6 +92,7 @@ void trivial_hash_func(const void *p, struct siphash *state); int trivial_compare_func(const void *a, const void *b) _const_; extern const struct hash_ops trivial_hash_ops; extern const struct hash_ops trivial_hash_ops_free; +extern const struct hash_ops trivial_hash_ops_value_free; extern const struct hash_ops trivial_hash_ops_free_free; /* 32-bit values we can always just embed in the pointer itself, but in order to support 32-bit archs we need store 64-bit @@ -98,6 +100,7 @@ extern const struct hash_ops trivial_hash_ops_free_free; void uint64_hash_func(const uint64_t *p, struct siphash *state); int uint64_compare_func(const uint64_t *a, const uint64_t *b) _pure_; extern const struct hash_ops uint64_hash_ops; +extern const struct hash_ops uint64_hash_ops_value_free; /* On some archs dev_t is 32-bit, and on others 64-bit. And sometimes it's 64-bit on 32-bit archs, and sometimes 32-bit on * 64-bit archs. Yuck! */ From 57f15d05bf51ef19721d38540762b44d4cad8ac7 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 22 Jan 2025 04:15:49 +0900 Subject: [PATCH 02/15] udevadm-monitor: use hash ops with destructor This also make it use STATIC_DESTRUCTOR_REGISTER() macro, and logs OOM error. --- src/udev/udevadm-monitor.c | 51 ++++++++++++++++---------------------- 1 file changed, 21 insertions(+), 30 deletions(-) diff --git a/src/udev/udevadm-monitor.c b/src/udev/udevadm-monitor.c index 9585ac892f..76b953c917 100644 --- a/src/udev/udevadm-monitor.c +++ b/src/udev/udevadm-monitor.c @@ -15,10 +15,11 @@ #include "hashmap.h" #include "set.h" #include "signal-util.h" +#include "static-destruct.h" #include "string-util.h" +#include "time-util.h" #include "udevadm.h" #include "virt.h" -#include "time-util.h" static bool arg_show_property = false; static bool arg_print_kernel = false; @@ -26,6 +27,9 @@ static bool arg_print_udev = false; static Set *arg_tag_filter = NULL; static Hashmap *arg_subsystem_filter = NULL; +STATIC_DESTRUCTOR_REGISTER(arg_tag_filter, set_freep); +STATIC_DESTRUCTOR_REGISTER(arg_subsystem_filter, hashmap_freep); + static int device_monitor_handler(sd_device_monitor *monitor, sd_device *device, void *userdata) { sd_device_action_t action = _SD_DEVICE_ACTION_INVALID; const char *devpath = NULL, *subsystem = NULL; @@ -143,28 +147,27 @@ static int parse_argv(int argc, char *argv[]) { if (slash) { devtype = strdup(slash + 1); if (!devtype) - return -ENOMEM; + return log_oom(); subsystem = strndup(optarg, slash - optarg); } else subsystem = strdup(optarg); if (!subsystem) - return -ENOMEM; + return log_oom(); - r = hashmap_ensure_put(&arg_subsystem_filter, NULL, subsystem, devtype); + r = hashmap_ensure_put(&arg_subsystem_filter, &trivial_hash_ops_free_free, subsystem, devtype); if (r < 0) - return r; + return log_oom(); TAKE_PTR(subsystem); TAKE_PTR(devtype); break; } case 't': - /* optarg is stored in argv[], so we don't need to copy it */ - r = set_ensure_put(&arg_tag_filter, &string_hash_ops, optarg); + r = set_put_strdup(&arg_tag_filter, optarg); if (r < 0) - return r; + return log_oom(); break; case 'V': @@ -192,7 +195,7 @@ int monitor_main(int argc, char *argv[], void *userdata) { r = parse_argv(argc, argv); if (r <= 0) - goto finalize; + return r; if (running_in_chroot() > 0) { log_info("Running in chroot, ignoring request."); @@ -203,22 +206,18 @@ int monitor_main(int argc, char *argv[], void *userdata) { setlinebuf(stdout); r = sd_event_default(&event); - if (r < 0) { - log_error_errno(r, "Failed to initialize event: %m"); - goto finalize; - } + if (r < 0) + return log_error_errno(r, "Failed to initialize event: %m"); r = sd_event_set_signal_exit(event, true); - if (r < 0) { - log_error_errno(r, "Failed to install SIGINT/SIGTERM handling: %m"); - goto finalize; - } + if (r < 0) + return log_error_errno(r, "Failed to install SIGINT/SIGTERM handling: %m"); printf("monitor will print the received events for:\n"); if (arg_print_udev) { r = setup_monitor(MONITOR_GROUP_UDEV, event, &udev_monitor); if (r < 0) - goto finalize; + return r; printf("UDEV - the event which udev sends out after rule processing\n"); } @@ -226,23 +225,15 @@ int monitor_main(int argc, char *argv[], void *userdata) { if (arg_print_kernel) { r = setup_monitor(MONITOR_GROUP_KERNEL, event, &kernel_monitor); if (r < 0) - goto finalize; + return r; printf("KERNEL - the kernel uevent\n"); } printf("\n"); r = sd_event_loop(event); - if (r < 0) { - log_error_errno(r, "Failed to run event loop: %m"); - goto finalize; - } + if (r < 0) + return log_error_errno(r, "Failed to run event loop: %m"); - r = 0; - -finalize: - hashmap_free_free_free(arg_subsystem_filter); - set_free(arg_tag_filter); - - return r; + return 0; } From a22620e39f9edc4ebab07dc78b891a454aafda62 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 22 Jan 2025 11:08:07 +0900 Subject: [PATCH 03/15] udev: use hash ops with destructor --- src/udev/udev-config.c | 2 +- src/udev/udev-event.c | 4 ++-- src/udev/udev-manager.c | 2 +- src/udev/udev-rules.c | 16 ++++++++-------- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/udev/udev-config.c b/src/udev/udev-config.c index 6f02d25f45..6562b34d2f 100644 --- a/src/udev/udev-config.c +++ b/src/udev/udev-config.c @@ -407,7 +407,7 @@ static int manager_set_environment_one(Manager *manager, const char *s) { _cleanup_free_ char *old_key = NULL, *old_value = NULL; old_value = hashmap_get2(manager->properties, key, (void**) &old_key); - r = hashmap_ensure_replace(&manager->properties, &string_hash_ops, key, value); + r = hashmap_ensure_replace(&manager->properties, &string_hash_ops_free_free, key, value); if (r < 0) { assert(!old_key); assert(!old_value); diff --git a/src/udev/udev-event.c b/src/udev/udev-event.c index fc4c9f9fd1..7d9153061f 100644 --- a/src/udev/udev-event.c +++ b/src/udev/udev-event.c @@ -52,8 +52,8 @@ static UdevEvent* udev_event_free(UdevEvent *event) { sd_device_unref(event->dev); sd_device_unref(event->dev_db_clone); sd_netlink_unref(event->rtnl); - ordered_hashmap_free_free_key(event->run_list); - ordered_hashmap_free_free_free(event->seclabel_list); + ordered_hashmap_free(event->run_list); + ordered_hashmap_free(event->seclabel_list); hashmap_free(event->written_sysattrs); hashmap_free(event->written_sysctls); free(event->program_result); diff --git a/src/udev/udev-manager.c b/src/udev/udev-manager.c index 873a076a2d..97465b796d 100644 --- a/src/udev/udev-manager.c +++ b/src/udev/udev-manager.c @@ -139,7 +139,7 @@ Manager* manager_free(Manager *manager) { udev_builtin_exit(); - hashmap_free_free_free(manager->properties); + hashmap_free(manager->properties); udev_rules_free(manager->rules); hashmap_free(manager->workers); diff --git a/src/udev/udev-rules.c b/src/udev/udev-rules.c index ec4660968c..6c5c0f4533 100644 --- a/src/udev/udev-rules.c +++ b/src/udev/udev-rules.c @@ -467,8 +467,8 @@ UdevRules* udev_rules_free(UdevRules *rules) { LIST_FOREACH(rule_files, i, rules->rule_files) udev_rule_file_free(i); - hashmap_free_free_key(rules->known_users); - hashmap_free_free_key(rules->known_groups); + hashmap_free(rules->known_users); + hashmap_free(rules->known_groups); hashmap_free(rules->stats_by_path); return mfree(rules); } @@ -504,7 +504,7 @@ static int rule_resolve_user(UdevRuleLine *rule_line, const char *name, uid_t *r if (!n) return -ENOMEM; - r = hashmap_ensure_put(known_users, &string_hash_ops, n, UID_TO_PTR(uid)); + r = hashmap_ensure_put(known_users, &string_hash_ops_free, n, UID_TO_PTR(uid)); if (r < 0) return r; @@ -544,7 +544,7 @@ static int rule_resolve_group(UdevRuleLine *rule_line, const char *name, gid_t * if (!n) return -ENOMEM; - r = hashmap_ensure_put(known_groups, &string_hash_ops, n, GID_TO_PTR(gid)); + r = hashmap_ensure_put(known_groups, &string_hash_ops_free, n, GID_TO_PTR(gid)); if (r < 0) return r; @@ -2756,9 +2756,9 @@ static int udev_rule_apply_token_to_event( return log_oom(); if (token->op == OP_ASSIGN) - ordered_hashmap_clear_free_free(event->seclabel_list); + ordered_hashmap_clear(event->seclabel_list); - r = ordered_hashmap_ensure_put(&event->seclabel_list, NULL, name, label); + r = ordered_hashmap_ensure_put(&event->seclabel_list, &trivial_hash_ops_free_free, name, label); if (r == -ENOMEM) return log_oom(); if (r < 0) @@ -3021,7 +3021,7 @@ static int udev_rule_apply_token_to_event( event->run_final = true; if (IN_SET(token->op, OP_ASSIGN, OP_ASSIGN_FINAL)) - ordered_hashmap_clear_free_key(event->run_list); + ordered_hashmap_clear(event->run_list); if (!apply_format_value(event, token, buf, sizeof(buf), "command")) return true; @@ -3030,7 +3030,7 @@ static int udev_rule_apply_token_to_event( if (!cmd) return log_oom(); - r = ordered_hashmap_ensure_put(&event->run_list, NULL, cmd, token->data); + r = ordered_hashmap_ensure_put(&event->run_list, &trivial_hash_ops_free, cmd, token->data); if (r < 0) return log_event_error_errno(event, token, r, "Failed to store command \"%s\": %m", cmd); From 12006a723337e22d3e905c94bf9254b6cdf39048 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 22 Jan 2025 11:08:25 +0900 Subject: [PATCH 04/15] wait-online: use hash ops with destructor --- src/network/wait-online/wait-online.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/network/wait-online/wait-online.c b/src/network/wait-online/wait-online.c index 6f5aef903a..48c4d1c0ee 100644 --- a/src/network/wait-online/wait-online.c +++ b/src/network/wait-online/wait-online.c @@ -23,7 +23,7 @@ static LinkOperationalStateRange arg_required_operstate = LINK_OPERSTATE_RANGE_I static AddressFamily arg_required_family = ADDRESS_FAMILY_NO; static bool arg_any = false; -STATIC_DESTRUCTOR_REGISTER(arg_interfaces, hashmap_free_free_freep); +STATIC_DESTRUCTOR_REGISTER(arg_interfaces, hashmap_freep); STATIC_DESTRUCTOR_REGISTER(arg_ignore, strv_freep); static int help(void) { @@ -85,7 +85,7 @@ static int parse_interface_with_operstate_range(const char *str) { return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Invalid interface name: %s", ifname); - r = hashmap_ensure_put(&arg_interfaces, &string_hash_ops, ifname, range); + r = hashmap_ensure_put(&arg_interfaces, &string_hash_ops_free_free, ifname, range); if (r == -ENOMEM) return log_oom(); if (r < 0) From 852c05c94fbe050f4db16b1919b717f882c7079e Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 22 Jan 2025 05:24:35 +0900 Subject: [PATCH 05/15] catalog: modernize code - set destructors to catalog_hash_ops, - acquire OrderedHashmap when necessary, - gracefully handle NULL catalog directories and output stream, - rename function output arguments, - add many many assertions, - use RET_GATHER(). --- src/fuzz/fuzz-catalog.c | 6 +- src/journal/journalctl-catalog.c | 6 +- src/libsystemd/sd-journal/catalog.c | 227 +++++++++++++---------- src/libsystemd/sd-journal/catalog.h | 15 +- src/libsystemd/sd-journal/test-catalog.c | 20 +- 5 files changed, 144 insertions(+), 130 deletions(-) diff --git a/src/fuzz/fuzz-catalog.c b/src/fuzz/fuzz-catalog.c index f9561f24c4..cbe8463a89 100644 --- a/src/fuzz/fuzz-catalog.c +++ b/src/fuzz/fuzz-catalog.c @@ -9,17 +9,15 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) { _cleanup_(unlink_tempfilep) char name[] = "/tmp/fuzz-catalog.XXXXXX"; _cleanup_close_ int fd = -EBADF; - _cleanup_ordered_hashmap_free_free_free_ OrderedHashmap *h = NULL; + _cleanup_ordered_hashmap_free_ OrderedHashmap *h = NULL; fuzz_setup_logging(); - assert_se(h = ordered_hashmap_new(&catalog_hash_ops)); - fd = mkostemp_safe(name); assert_se(fd >= 0); assert_se(write(fd, data, size) == (ssize_t) size); - (void) catalog_import_file(h, name); + (void) catalog_import_file(&h, name); return 0; } diff --git a/src/journal/journalctl-catalog.c b/src/journal/journalctl-catalog.c index 116e152be0..41ea3816e7 100644 --- a/src/journal/journalctl-catalog.c +++ b/src/journal/journalctl-catalog.c @@ -19,7 +19,7 @@ int action_update_catalog(void) { e = secure_getenv("SYSTEMD_CATALOG_SOURCES"); r = catalog_update(database, arg_root, - e ? STRV_MAKE_CONST(e) : catalog_file_dirs); + e ? STRV_MAKE_CONST(e) : NULL); if (r < 0) return log_error_errno(r, "Failed to update catalog: %m"); @@ -41,9 +41,9 @@ int action_list_catalog(char **items) { pager_open(arg_pager_flags); if (items) - r = catalog_list_items(stdout, database, oneline, items); + r = catalog_list_items(/* f = */ NULL, database, oneline, items); else - r = catalog_list(stdout, database, oneline); + r = catalog_list(/* f = */ NULL, database, oneline); if (r < 0) return log_error_errno(r, "Failed to list catalog: %m"); diff --git a/src/libsystemd/sd-journal/catalog.c b/src/libsystemd/sd-journal/catalog.c index 7dcc35d8d5..4e3733ce2e 100644 --- a/src/libsystemd/sd-journal/catalog.c +++ b/src/libsystemd/sd-journal/catalog.c @@ -55,15 +55,20 @@ typedef struct CatalogItem { } CatalogItem; static void catalog_hash_func(const CatalogItem *i, struct siphash *state) { + assert(i); + assert(state); + siphash24_compress_typesafe(i->id, state); siphash24_compress_string(i->language, state); } static int catalog_compare_func(const CatalogItem *a, const CatalogItem *b) { - unsigned k; int r; - for (k = 0; k < ELEMENTSOF(b->id.bytes); k++) { + assert(a); + assert(b); + + for (size_t k = 0; k < ELEMENTSOF(b->id.bytes); k++) { r = CMP(a->id.bytes[k], b->id.bytes[k]); if (r != 0) return r; @@ -72,11 +77,17 @@ static int catalog_compare_func(const CatalogItem *a, const CatalogItem *b) { return strcmp(a->language, b->language); } -DEFINE_HASH_OPS(catalog_hash_ops, CatalogItem, catalog_hash_func, catalog_compare_func); +DEFINE_PRIVATE_HASH_OPS_FULL( + catalog_hash_ops, + CatalogItem, catalog_hash_func, catalog_compare_func, free, + void, free); static bool next_header(const char **s) { const char *e; + assert(s); + assert(*s); + e = strchr(*s, '\n'); /* Unexpected end */ @@ -91,17 +102,22 @@ static bool next_header(const char **s) { return true; } -static const char *skip_header(const char *s) { +static const char* skip_header(const char *s) { + assert(s); + while (next_header(&s)) ; return s; } -static char *combine_entries(const char *one, const char *two) { +static char* combine_entries(const char *one, const char *two) { const char *b1, *b2; size_t l1, l2, n; char *dest, *p; + assert(one); + assert(two); + /* Find split point of headers to body */ b1 = skip_header(one); b2 = skip_header(two); @@ -140,10 +156,11 @@ static char *combine_entries(const char *one, const char *two) { } static int finish_item( - OrderedHashmap *h, + OrderedHashmap **h, sd_id128_t id, const char *language, - char *payload, size_t payload_size) { + const char *payload, + size_t payload_size) { _cleanup_free_ CatalogItem *i = NULL; _cleanup_free_ char *combined = NULL; @@ -164,14 +181,14 @@ static int finish_item( strcpy(i->language, language); } - prev = ordered_hashmap_get(h, i); + prev = ordered_hashmap_get(*h, i); if (prev) { /* Already have such an item, combine them */ combined = combine_entries(payload, prev); if (!combined) return log_oom(); - r = ordered_hashmap_update(h, i, combined); + r = ordered_hashmap_update(*h, i, combined); if (r < 0) return log_error_errno(r, "Failed to update catalog item: %m"); @@ -183,7 +200,7 @@ static int finish_item( if (!combined) return log_oom(); - r = ordered_hashmap_put(h, i, combined); + r = ordered_hashmap_ensure_put(h, &catalog_hash_ops, i, combined); if (r < 0) return log_error_errno(r, "Failed to insert catalog item: %m"); @@ -194,8 +211,11 @@ static int finish_item( return 0; } -int catalog_file_lang(const char* filename, char **lang) { - char *beg, *end, *_lang; +int catalog_file_lang(const char *filename, char **ret) { + char *beg, *end, *lang; + + assert(filename); + assert(ret); end = endswith(filename, ".catalog"); if (!end) @@ -208,21 +228,23 @@ int catalog_file_lang(const char* filename, char **lang) { if (*beg != '.' || end <= beg + 1) return 0; - _lang = strndup(beg + 1, end - beg - 1); - if (!_lang) + lang = strndup(beg + 1, end - beg - 1); + if (!lang) return -ENOMEM; - *lang = _lang; + *ret = lang; return 1; } static int catalog_entry_lang( - const char* filename, + const char *filename, unsigned line, - const char* t, - const char* deflang, + const char *t, + const char *deflang, char **ret) { + assert(t); + size_t c = strlen(t); if (c < 2) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), @@ -243,7 +265,7 @@ static int catalog_entry_lang( return strdup_to(ret, t); } -int catalog_import_file(OrderedHashmap *h, const char *path) { +int catalog_import_file(OrderedHashmap **h, const char *path) { _cleanup_fclose_ FILE *f = NULL; _cleanup_free_ char *payload = NULL; size_t payload_size = 0; @@ -371,14 +393,19 @@ int catalog_import_file(OrderedHashmap *h, const char *path) { static int64_t write_catalog( const char *database, - struct strbuf *sb, - CatalogItem *items, - size_t n) { + const struct strbuf *sb, + const CatalogItem *items, + size_t n_items) { _cleanup_(unlink_and_freep) char *p = NULL; _cleanup_fclose_ FILE *w = NULL; int r; + assert(database); + assert(sb); + assert(items); + assert(n_items > 0); + r = mkdir_parents(database, 0755); if (r < 0) return log_error_errno(r, "Failed to create parent directories of %s: %m", database); @@ -391,13 +418,13 @@ static int64_t write_catalog( .signature = CATALOG_SIGNATURE, .header_size = htole64(CONST_ALIGN_TO(sizeof(CatalogHeader), 8)), .catalog_item_size = htole64(sizeof(CatalogItem)), - .n_items = htole64(n), + .n_items = htole64(n_items), }; if (fwrite(&header, sizeof(header), 1, w) != 1) return log_error_errno(SYNTHETIC_ERRNO(EIO), "%s: failed to write header.", p); - if (fwrite(items, sizeof(CatalogItem), n, w) != n) + if (fwrite(items, sizeof(CatalogItem), n_items, w) != n_items) return log_error_errno(SYNTHETIC_ERRNO(EIO), "%s: failed to write database.", p); if (fwrite(sb->buf, sb->len, 1, w) != 1) @@ -416,30 +443,23 @@ static int64_t write_catalog( return ftello(w); } -int catalog_update(const char* database, const char* root, const char* const* dirs) { - _cleanup_strv_free_ char **files = NULL; - _cleanup_(strbuf_freep) struct strbuf *sb = NULL; - _cleanup_ordered_hashmap_free_free_free_ OrderedHashmap *h = NULL; - _cleanup_free_ CatalogItem *items = NULL; - ssize_t offset; - char *payload; - CatalogItem *i; - unsigned n; +int catalog_update(const char *database, const char *root, const char* const *dirs) { int r; - int64_t sz; - h = ordered_hashmap_new(&catalog_hash_ops); - sb = strbuf_new(); - if (!h || !sb) - return log_oom(); + assert(database); + if (!dirs) + dirs = catalog_file_dirs; + + _cleanup_strv_free_ char **files = NULL; r = conf_files_list_strv(&files, ".catalog", root, 0, dirs); if (r < 0) return log_error_errno(r, "Failed to get catalog files: %m"); + _cleanup_ordered_hashmap_free_ OrderedHashmap *h = NULL; STRV_FOREACH(f, files) { log_debug("Reading file '%s'", *f); - r = catalog_import_file(h, *f); + r = catalog_import_file(&h, *f); if (r < 0) return log_error_errno(r, "Failed to import file '%s': %m", *f); } @@ -451,17 +471,23 @@ int catalog_update(const char* database, const char* root, const char* const* di log_debug("Found %u items in catalog.", ordered_hashmap_size(h)); - items = new(CatalogItem, ordered_hashmap_size(h)); + _cleanup_free_ CatalogItem *items = new(CatalogItem, ordered_hashmap_size(h)); if (!items) return log_oom(); - n = 0; + _cleanup_(strbuf_freep) struct strbuf *sb = strbuf_new(); + if (!sb) + return log_oom(); + + unsigned n = 0; + char *payload; + CatalogItem *i; ORDERED_HASHMAP_FOREACH_KEY(payload, i, h) { log_trace("Found " SD_ID128_FORMAT_STR ", language %s", SD_ID128_FORMAT_VAL(i->id), isempty(i->language) ? "C" : i->language); - offset = strbuf_add_string(sb, payload); + ssize_t offset = strbuf_add_string(sb, payload); if (offset < 0) return log_oom(); @@ -474,7 +500,7 @@ int catalog_update(const char* database, const char* root, const char* const* di strbuf_complete(sb); - sz = write_catalog(database, sb, items, n); + int64_t sz = write_catalog(database, sb, items, n); if (sz < 0) return log_error_errno(sz, "Failed to write %s: %m", database); @@ -483,31 +509,28 @@ int catalog_update(const char* database, const char* root, const char* const* di return 0; } -static int open_mmap(const char *database, int *_fd, struct stat *_st, void **_p) { - _cleanup_close_ int fd = -EBADF; - const CatalogHeader *h; - struct stat st; - void *p; +static int open_mmap(const char *database, int *ret_fd, struct stat *ret_st, void **ret_map) { + assert(database); + assert(ret_fd); + assert(ret_st); + assert(ret_map); - assert(_fd); - assert(_st); - assert(_p); - - fd = open(database, O_RDONLY|O_CLOEXEC); + _cleanup_close_ int fd = open(database, O_RDONLY|O_CLOEXEC); if (fd < 0) return -errno; + struct stat st; if (fstat(fd, &st) < 0) return -errno; if (st.st_size < (off_t) sizeof(CatalogHeader) || file_offset_beyond_memory_size(st.st_size)) return -EINVAL; - p = mmap(NULL, st.st_size, PROT_READ, MAP_SHARED, fd, 0); + void *p = mmap(NULL, st.st_size, PROT_READ, MAP_SHARED, fd, 0); if (p == MAP_FAILED) return -errno; - h = p; + const CatalogHeader *h = p; if (memcmp(h->signature, (const uint8_t[]) CATALOG_SIGNATURE, sizeof(h->signature)) != 0 || le64toh(h->header_size) < sizeof(CatalogHeader) || le64toh(h->catalog_item_size) < sizeof(CatalogItem) || @@ -518,16 +541,15 @@ static int open_mmap(const char *database, int *_fd, struct stat *_st, void **_p return -EBADMSG; } - *_fd = TAKE_FD(fd); - *_st = st; - *_p = p; - + *ret_fd = TAKE_FD(fd); + *ret_st = st; + *ret_map = p; return 0; } -static const char *find_id(void *p, sd_id128_t id) { +static const char* find_id(const void *p, sd_id128_t id) { CatalogItem *f = NULL, key = { .id = id }; - const CatalogHeader *h = p; + const CatalogHeader *h = ASSERT_PTR(p); const char *loc; loc = setlocale(LC_MESSAGES, NULL); @@ -580,33 +602,33 @@ static const char *find_id(void *p, sd_id128_t id) { le64toh(f->offset); } -int catalog_get(const char* database, sd_id128_t id, char **ret_text) { - _cleanup_close_ int fd = -EBADF; - void *p = NULL; - struct stat st; +int catalog_get(const char *database, sd_id128_t id, char **ret_text) { int r; - const char *s; + assert(database); assert(ret_text); + _cleanup_close_ int fd = -EBADF; + struct stat st; + void *p; r = open_mmap(database, &fd, &st, &p); if (r < 0) return r; - s = find_id(p, id); - if (!s) { + const char *s = find_id(p, id); + if (!s) r = -ENOENT; - goto finish; - } + else + r = strdup_to(ret_text, s); - r = strdup_to(ret_text, s); -finish: (void) munmap(p, st.st_size); - return r; } -static char *find_header(const char *s, const char *header) { +static char* find_header(const char *s, const char *header) { + + assert(s); + assert(header); for (;;) { const char *v; @@ -623,6 +645,11 @@ static char *find_header(const char *s, const char *header) { } static void dump_catalog_entry(FILE *f, sd_id128_t id, const char *s, bool oneline) { + assert(s); + + if (!f) + f = stdout; + if (oneline) { _cleanup_free_ char *subject = NULL, *defined_by = NULL; @@ -638,24 +665,23 @@ static void dump_catalog_entry(FILE *f, sd_id128_t id, const char *s, bool oneli } int catalog_list(FILE *f, const char *database, bool oneline) { - _cleanup_close_ int fd = -EBADF; - void *p = NULL; - struct stat st; - const CatalogHeader *h; - const CatalogItem *items; int r; - unsigned n; - sd_id128_t last_id; - bool last_id_set = false; + assert(database); + + _cleanup_close_ int fd = -EBADF; + struct stat st; + void *p; r = open_mmap(database, &fd, &st, &p); if (r < 0) return r; - h = p; - items = (const CatalogItem*) ((const uint8_t*) p + le64toh(h->header_size)); + const CatalogHeader *h = p; + const CatalogItem *items = (const CatalogItem*) ((const uint8_t*) p + le64toh(h->header_size)); - for (n = 0; n < le64toh(h->n_items); n++) { + sd_id128_t last_id; + bool last_id_set = false; + for (size_t n = 0; n < le64toh(h->n_items); n++) { const char *s; if (last_id_set && sd_id128_equal(last_id, items[n].id)) @@ -669,38 +695,33 @@ int catalog_list(FILE *f, const char *database, bool oneline) { last_id = items[n].id; } - munmap(p, st.st_size); - + (void) munmap(p, st.st_size); return 0; } int catalog_list_items(FILE *f, const char *database, bool oneline, char **items) { - int r = 0; + int r, ret = 0; + + assert(database); STRV_FOREACH(item, items) { sd_id128_t id; - int k; - _cleanup_free_ char *msg = NULL; - - k = sd_id128_from_string(*item, &id); - if (k < 0) { - log_error_errno(k, "Failed to parse id128 '%s': %m", *item); - if (r == 0) - r = k; + r = sd_id128_from_string(*item, &id); + if (r < 0) { + RET_GATHER(ret, log_error_errno(r, "Failed to parse id128 '%s': %m", *item)); continue; } - k = catalog_get(database, id, &msg); - if (k < 0) { - log_full_errno(k == -ENOENT ? LOG_NOTICE : LOG_ERR, k, - "Failed to retrieve catalog entry for '%s': %m", *item); - if (r == 0) - r = k; + _cleanup_free_ char *msg = NULL; + r = catalog_get(database, id, &msg); + if (r < 0) { + RET_GATHER(ret, log_full_errno(r == -ENOENT ? LOG_NOTICE : LOG_ERR, r, + "Failed to retrieve catalog entry for '%s': %m", *item)); continue; } dump_catalog_entry(f, id, msg, oneline); } - return r; + return ret; } diff --git a/src/libsystemd/sd-journal/catalog.h b/src/libsystemd/sd-journal/catalog.h index b5a97fa6fb..e6a8c9b8db 100644 --- a/src/libsystemd/sd-journal/catalog.h +++ b/src/libsystemd/sd-journal/catalog.h @@ -7,13 +7,10 @@ #include "sd-id128.h" #include "hashmap.h" -#include "strbuf.h" -int catalog_import_file(OrderedHashmap *h, const char *path); -int catalog_update(const char* database, const char* root, const char* const* dirs); -int catalog_get(const char* database, sd_id128_t id, char **ret_text); -int catalog_list(FILE *f, const char* database, bool oneline); -int catalog_list_items(FILE *f, const char* database, bool oneline, char **items); -int catalog_file_lang(const char *filename, char **lang); -extern const char * const catalog_file_dirs[]; -extern const struct hash_ops catalog_hash_ops; +int catalog_import_file(OrderedHashmap **h, const char *path); +int catalog_update(const char *database, const char *root, const char* const *dirs); +int catalog_get(const char *database, sd_id128_t id, char **ret_text); +int catalog_list(FILE *f, const char *database, bool oneline); +int catalog_list_items(FILE *f, const char *database, bool oneline, char **items); +int catalog_file_lang(const char *filename, char **ret); diff --git a/src/libsystemd/sd-journal/test-catalog.c b/src/libsystemd/sd-journal/test-catalog.c index 603952e904..ee8bd4e0ec 100644 --- a/src/libsystemd/sd-journal/test-catalog.c +++ b/src/libsystemd/sd-journal/test-catalog.c @@ -28,31 +28,29 @@ static const char *no_catalog_dirs[] = { static OrderedHashmap* test_import(const char* contents, ssize_t size, int code) { _cleanup_(unlink_tempfilep) char name[] = "/tmp/test-catalog.XXXXXX"; _cleanup_close_ int fd = -EBADF; - OrderedHashmap *h; + OrderedHashmap *h = NULL; if (size < 0) size = strlen(contents); - assert_se(h = ordered_hashmap_new(&catalog_hash_ops)); - fd = mkostemp_safe(name); assert_se(fd >= 0); assert_se(write(fd, contents, size) == size); - assert_se(catalog_import_file(h, name) == code); + assert_se(catalog_import_file(&h, name) == code); return h; } static void test_catalog_import_invalid(void) { - _cleanup_ordered_hashmap_free_free_free_ OrderedHashmap *h = NULL; + _cleanup_ordered_hashmap_free_ OrderedHashmap *h = NULL; h = test_import("xxx", -1, -EINVAL); assert_se(ordered_hashmap_isempty(h)); } static void test_catalog_import_badid(void) { - _unused_ _cleanup_ordered_hashmap_free_free_free_ OrderedHashmap *h = NULL; + _unused_ _cleanup_ordered_hashmap_free_ OrderedHashmap *h = NULL; const char *input = "-- 0027229ca0644181a76c4e92458afaff dededededededededededededededede\n" \ "Subject: message\n" \ @@ -62,7 +60,7 @@ static void test_catalog_import_badid(void) { } static void test_catalog_import_one(void) { - _cleanup_ordered_hashmap_free_free_free_ OrderedHashmap *h = NULL; + _cleanup_ordered_hashmap_free_ OrderedHashmap *h = NULL; char *payload; const char *input = @@ -86,7 +84,7 @@ static void test_catalog_import_one(void) { } static void test_catalog_import_merge(void) { - _cleanup_ordered_hashmap_free_free_free_ OrderedHashmap *h = NULL; + _cleanup_ordered_hashmap_free_ OrderedHashmap *h = NULL; char *payload; const char *input = @@ -118,7 +116,7 @@ static void test_catalog_import_merge(void) { } static void test_catalog_import_merge_no_body(void) { - _cleanup_ordered_hashmap_free_free_free_ OrderedHashmap *h = NULL; + _cleanup_ordered_hashmap_free_ OrderedHashmap *h = NULL; char *payload; const char *input = @@ -222,10 +220,10 @@ int main(int argc, char *argv[]) { test_catalog_update(database); - r = catalog_list(stdout, database, true); + r = catalog_list(NULL, database, true); assert_se(r >= 0); - r = catalog_list(stdout, database, false); + r = catalog_list(NULL, database, false); assert_se(r >= 0); assert_se(catalog_get(database, SD_MESSAGE_COREDUMP, &text) >= 0); From c1bfee0bdbae82519be83137354fa91b0d8fe5a1 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 22 Jan 2025 05:41:37 +0900 Subject: [PATCH 06/15] bootctl: use hash ops with destructor This also makes the hashmap allocated when necessary. --- src/bootctl/bootctl-status.c | 47 ++++++++++++++++-------------------- 1 file changed, 21 insertions(+), 26 deletions(-) diff --git a/src/bootctl/bootctl-status.c b/src/bootctl/bootctl-status.c index 2ca49ec485..40642d3c1c 100644 --- a/src/bootctl/bootctl-status.c +++ b/src/bootctl/bootctl-status.c @@ -568,24 +568,24 @@ int verb_status(int argc, char *argv[], void *userdata) { return r; } -static int ref_file(Hashmap *known_files, const char *fn, int increment) { +static int ref_file(Hashmap **known_files, const char *fn, int increment) { char *k = NULL; int n, r; assert(known_files); - /* just gracefully ignore this. This way the caller doesn't - have to verify whether the bootloader entry is relevant */ + /* just gracefully ignore this. This way the caller doesn't have to verify whether the bootloader + * entry is relevant. */ if (!fn) return 0; - n = PTR_TO_INT(hashmap_get2(known_files, fn, (void**)&k)); + n = PTR_TO_INT(hashmap_get2(*known_files, fn, (void**)&k)); n += increment; assert(n >= 0); if (n == 0) { - (void) hashmap_remove(known_files, fn); + (void) hashmap_remove(*known_files, fn); free(k); } else if (!k) { _cleanup_free_ char *t = NULL; @@ -593,12 +593,12 @@ static int ref_file(Hashmap *known_files, const char *fn, int increment) { t = strdup(fn); if (!t) return -ENOMEM; - r = hashmap_put(known_files, t, INT_TO_PTR(n)); + r = hashmap_ensure_put(known_files, &path_hash_ops_free, t, INT_TO_PTR(n)); if (r < 0) return r; TAKE_PTR(t); } else { - r = hashmap_update(known_files, fn, INT_TO_PTR(n)); + r = hashmap_update(*known_files, fn, INT_TO_PTR(n)); if (r < 0) return r; } @@ -606,7 +606,7 @@ static int ref_file(Hashmap *known_files, const char *fn, int increment) { return n; } -static void deref_unlink_file(Hashmap *known_files, const char *fn, const char *root) { +static void deref_unlink_file(Hashmap **known_files, const char *fn, const char *root) { _cleanup_free_ char *path = NULL; int r; @@ -647,38 +647,34 @@ static void deref_unlink_file(Hashmap *known_files, const char *fn, const char * } static int count_known_files(const BootConfig *config, const char* root, Hashmap **ret_known_files) { - _cleanup_(hashmap_free_free_keyp) Hashmap *known_files = NULL; + _cleanup_hashmap_free_ Hashmap *known_files = NULL; int r; assert(config); assert(ret_known_files); - known_files = hashmap_new(&path_hash_ops); - if (!known_files) - return -ENOMEM; - for (size_t i = 0; i < config->n_entries; i++) { const BootEntry *e = config->entries + i; if (!path_equal(e->root, root)) continue; - r = ref_file(known_files, e->kernel, +1); + r = ref_file(&known_files, e->kernel, +1); if (r < 0) return r; - r = ref_file(known_files, e->efi, +1); + r = ref_file(&known_files, e->efi, +1); if (r < 0) return r; STRV_FOREACH(s, e->initrd) { - r = ref_file(known_files, *s, +1); + r = ref_file(&known_files, *s, +1); if (r < 0) return r; } - r = ref_file(known_files, e->device_tree, +1); + r = ref_file(&known_files, e->device_tree, +1); if (r < 0) return r; STRV_FOREACH(s, e->device_tree_overlay) { - r = ref_file(known_files, *s, +1); + r = ref_file(&known_files, *s, +1); if (r < 0) return r; } @@ -704,7 +700,7 @@ static int boot_config_find_in(const BootConfig *config, const char *root, const } static int unlink_entry(const BootConfig *config, const char *root, const char *id) { - _cleanup_(hashmap_free_free_keyp) Hashmap *known_files = NULL; + _cleanup_hashmap_free_ Hashmap *known_files = NULL; const BootEntry *e = NULL; int r; @@ -725,13 +721,13 @@ static int unlink_entry(const BootConfig *config, const char *root, const char * e = &config->entries[r]; - deref_unlink_file(known_files, e->kernel, e->root); - deref_unlink_file(known_files, e->efi, e->root); + deref_unlink_file(&known_files, e->kernel, e->root); + deref_unlink_file(&known_files, e->efi, e->root); STRV_FOREACH(s, e->initrd) - deref_unlink_file(known_files, *s, e->root); - deref_unlink_file(known_files, e->device_tree, e->root); + deref_unlink_file(&known_files, *s, e->root); + deref_unlink_file(&known_files, e->device_tree, e->root); STRV_FOREACH(s, e->device_tree_overlay) - deref_unlink_file(known_files, *s, e->root); + deref_unlink_file(&known_files, *s, e->root); if (arg_dry_run) log_info("Would remove \"%s\"", e->path); @@ -758,7 +754,6 @@ static int list_remove_orphaned_file( Hashmap *known_files = userdata; assert(path); - assert(known_files); if (event != RECURSE_DIR_ENTRY) return RECURSE_DIR_CONTINUE; @@ -780,7 +775,7 @@ static int cleanup_orphaned_files( const BootConfig *config, const char *root) { - _cleanup_(hashmap_free_free_keyp) Hashmap *known_files = NULL; + _cleanup_hashmap_free_ Hashmap *known_files = NULL; _cleanup_free_ char *full = NULL, *p = NULL; _cleanup_close_ int dir_fd = -EBADF; int r; From 451602283319a55e9492d562d3f58c88473cd0c2 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 22 Jan 2025 06:06:38 +0900 Subject: [PATCH 07/15] delta: use hash ops with destructor This also makes it use RET_GATHER(). --- src/delta/delta.c | 86 +++++++++++++++++++---------------------------- 1 file changed, 34 insertions(+), 52 deletions(-) diff --git a/src/delta/delta.c b/src/delta/delta.c index 3433250549..c7cf6c9b0b 100644 --- a/src/delta/delta.c +++ b/src/delta/delta.c @@ -184,10 +184,15 @@ static int found_override(const char *top, const char *bottom) { return r; } +DEFINE_PRIVATE_HASH_OPS_FULL( + drop_hash_ops, + char, string_hash_func, string_compare_func, free, + OrderedHashmap, ordered_hashmap_free); + static int enumerate_dir_d( - OrderedHashmap *top, - OrderedHashmap *bottom, - OrderedHashmap *drops, + OrderedHashmap **top, + OrderedHashmap **bottom, + OrderedHashmap **drops, const char *toppath, const char *drop) { _cleanup_free_ char *unit = NULL; @@ -234,7 +239,7 @@ static int enumerate_dir_d( d = p + strlen(toppath) + 1; log_debug("Adding at top: %s %s %s", d, special_glyph(SPECIAL_GLYPH_ARROW_RIGHT), p); - k = ordered_hashmap_put(top, d, p); + k = ordered_hashmap_ensure_put(top, &string_hash_ops_value_free, d, p); if (k >= 0) { p = strdup(p); if (!p) @@ -246,19 +251,19 @@ static int enumerate_dir_d( } log_debug("Adding at bottom: %s %s %s", d, special_glyph(SPECIAL_GLYPH_ARROW_RIGHT), p); - free(ordered_hashmap_remove(bottom, d)); - k = ordered_hashmap_put(bottom, d, p); + free(ordered_hashmap_remove(*bottom, d)); + k = ordered_hashmap_ensure_put(bottom, &string_hash_ops_value_free, d, p); if (k < 0) { free(p); return k; } - h = ordered_hashmap_get(drops, unit); + h = ordered_hashmap_get(*drops, unit); if (!h) { - h = ordered_hashmap_new(&string_hash_ops); + h = ordered_hashmap_new(&string_hash_ops_value_free); if (!h) return -ENOMEM; - ordered_hashmap_put(drops, unit, h); + ordered_hashmap_ensure_put(drops, &drop_hash_ops, unit, h); unit = strdup(unit); if (!unit) return -ENOMEM; @@ -281,9 +286,9 @@ static int enumerate_dir_d( } static int enumerate_dir( - OrderedHashmap *top, - OrderedHashmap *bottom, - OrderedHashmap *drops, + OrderedHashmap **top, + OrderedHashmap **bottom, + OrderedHashmap **drops, const char *path, bool dropins) { _cleanup_closedir_ DIR *d = NULL; @@ -346,7 +351,7 @@ static int enumerate_dir( return -ENOMEM; log_debug("Adding at top: %s %s %s", basename(p), special_glyph(SPECIAL_GLYPH_ARROW_RIGHT), p); - r = ordered_hashmap_put(top, basename(p), p); + r = ordered_hashmap_ensure_put(top, &string_hash_ops_value_free, basename(p), p); if (r >= 0) { p = strdup(p); if (!p) @@ -355,8 +360,8 @@ static int enumerate_dir( return r; log_debug("Adding at bottom: %s %s %s", basename(p), special_glyph(SPECIAL_GLYPH_ARROW_RIGHT), p); - free(ordered_hashmap_remove(bottom, basename(p))); - r = ordered_hashmap_put(bottom, basename(p), p); + free(ordered_hashmap_remove(*bottom, basename(p))); + r = ordered_hashmap_ensure_put(bottom, &string_hash_ops_value_free, basename(p), p); if (r < 0) return r; p = NULL; @@ -366,39 +371,27 @@ static int enumerate_dir( } static int process_suffix(const char *suffix, const char *onlyprefix) { - char *f, *key; - OrderedHashmap *top, *bottom, *drops, *h; - int r = 0, k, n_found = 0; - bool dropins; + int r, ret = 0; assert(suffix); assert(!startswith(suffix, "/")); assert(!strstr(suffix, "//")); - dropins = nulstr_contains(have_dropins, suffix); - - top = ordered_hashmap_new(&string_hash_ops); - bottom = ordered_hashmap_new(&string_hash_ops); - drops = ordered_hashmap_new(&string_hash_ops); - if (!top || !bottom || !drops) { - r = -ENOMEM; - goto finish; - } + bool dropins = nulstr_contains(have_dropins, suffix); + _cleanup_ordered_hashmap_free_ OrderedHashmap *top = NULL, *bottom = NULL, *drops = NULL; NULSTR_FOREACH(p, prefixes) { _cleanup_free_ char *t = NULL; t = path_join(p, suffix); - if (!t) { - r = -ENOMEM; - goto finish; - } + if (!t) + return -ENOMEM; - k = enumerate_dir(top, bottom, drops, t, dropins); - if (r == 0) - r = k; + RET_GATHER(ret, enumerate_dir(&top, &bottom, &drops, t, dropins)); } + int n_found = 0; + char *f, *key; ORDERED_HASHMAP_FOREACH_KEY(f, key, top) { char *o; @@ -409,33 +402,22 @@ static int process_suffix(const char *suffix, const char *onlyprefix) { if (path_equal(o, f)) { notify_override_unchanged(f); } else { - k = found_override(f, o); - if (k < 0) - r = k; + r = found_override(f, o); + if (r < 0) + RET_GATHER(ret, r); else - n_found += k; + n_found += r; } } - h = ordered_hashmap_get(drops, key); + OrderedHashmap *h = ordered_hashmap_get(drops, key); if (h) ORDERED_HASHMAP_FOREACH(o, h) if (!onlyprefix || startswith(o, onlyprefix)) n_found += notify_override_extended(f, o); } -finish: - ordered_hashmap_free_free(top); - ordered_hashmap_free_free(bottom); - - ORDERED_HASHMAP_FOREACH_KEY(h, key, drops) { - ordered_hashmap_free_free(ordered_hashmap_remove(drops, key)); - ordered_hashmap_remove(drops, key); - free(key); - } - ordered_hashmap_free(drops); - - return r < 0 ? r : n_found; + return ret < 0 ? ret : n_found; } static int process_suffixes(const char *onlyprefix) { From b87501ea3c185b161be1daa9c1989384169d20c4 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 22 Jan 2025 06:29:23 +0900 Subject: [PATCH 08/15] sd-bus: use hash ops with destructor This also makes vtable_methods and vtable_properties managed by Set, as the key and value of each entry are equivalent. --- src/libsystemd/sd-bus/bus-internal.h | 4 ++-- src/libsystemd/sd-bus/bus-objects.c | 36 +++++++++++++--------------- src/libsystemd/sd-bus/bus-slot.c | 4 ++-- src/libsystemd/sd-bus/sd-bus.c | 12 ++++------ 4 files changed, 25 insertions(+), 31 deletions(-) diff --git a/src/libsystemd/sd-bus/bus-internal.h b/src/libsystemd/sd-bus/bus-internal.h index 5b3cae759b..25fe4bd956 100644 --- a/src/libsystemd/sd-bus/bus-internal.h +++ b/src/libsystemd/sd-bus/bus-internal.h @@ -240,8 +240,8 @@ struct sd_bus { LIST_HEAD(struct filter_callback, filter_callbacks); Hashmap *nodes; - Hashmap *vtable_methods; - Hashmap *vtable_properties; + Set *vtable_methods; + Set *vtable_properties; union sockaddr_union sockaddr; socklen_t sockaddr_size; diff --git a/src/libsystemd/sd-bus/bus-objects.c b/src/libsystemd/sd-bus/bus-objects.c index e528987a6d..af79bbc5f0 100644 --- a/src/libsystemd/sd-bus/bus-objects.c +++ b/src/libsystemd/sd-bus/bus-objects.c @@ -220,7 +220,7 @@ static int get_child_nodes( OrderedSet **ret, sd_bus_error *error) { - _cleanup_ordered_set_free_free_ OrderedSet *s = NULL; + _cleanup_ordered_set_free_ OrderedSet *s = NULL; int r; assert(bus); @@ -228,7 +228,7 @@ static int get_child_nodes( assert(n); assert(ret); - s = ordered_set_new(&string_hash_ops); + s = ordered_set_new(&string_hash_ops_free); if (!s) return -ENOMEM; @@ -926,7 +926,7 @@ int introspect_path( char **ret, sd_bus_error *error) { - _cleanup_ordered_set_free_free_ OrderedSet *s = NULL; + _cleanup_ordered_set_free_ OrderedSet *s = NULL; _cleanup_(introspect_done) struct introspect intro = {}; bool empty; int r; @@ -1229,7 +1229,7 @@ static int process_get_managed_objects( _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; - _cleanup_ordered_set_free_free_ OrderedSet *s = NULL; + _cleanup_ordered_set_free_ OrderedSet *s = NULL; char *path; int r; @@ -1314,7 +1314,7 @@ static int object_find_and_run( vtable_key.interface = m->interface; vtable_key.member = m->member; - v = hashmap_get(bus->vtable_methods, &vtable_key); + v = set_get(bus->vtable_methods, &vtable_key); if (v) { r = method_callbacks_run(bus, m, v, require_fallback, found_object); if (r != 0) @@ -1341,7 +1341,7 @@ static int object_find_and_run( if (r < 0) return sd_bus_reply_method_errorf(m, SD_BUS_ERROR_INVALID_ARGS, "Expected interface and member parameters"); - v = hashmap_get(bus->vtable_properties, &vtable_key); + v = set_get(bus->vtable_properties, &vtable_key); if (v) { r = property_get_set_callbacks_run(bus, m, v, require_fallback, get, found_object); if (r != 0) @@ -1686,7 +1686,9 @@ static int vtable_member_compare_func(const struct vtable_member *x, const struc return strcmp(x->member, y->member); } -DEFINE_PRIVATE_HASH_OPS(vtable_member_hash_ops, struct vtable_member, vtable_member_hash_func, vtable_member_compare_func); +DEFINE_PRIVATE_HASH_OPS_WITH_KEY_DESTRUCTOR( + vtable_member_hash_ops, + struct vtable_member, vtable_member_hash_func, vtable_member_compare_func, free); typedef enum { NAMES_FIRST_PART = 1 << 0, /* first part of argument name list (input names). It is reset by names_are_valid() */ @@ -1812,14 +1814,6 @@ static int add_object_vtable_internal( !streq(interface, "org.freedesktop.DBus.Peer") && !streq(interface, "org.freedesktop.DBus.ObjectManager"), -EINVAL); - r = hashmap_ensure_allocated(&bus->vtable_methods, &vtable_member_hash_ops); - if (r < 0) - return r; - - r = hashmap_ensure_allocated(&bus->vtable_properties, &vtable_member_hash_ops); - if (r < 0) - return r; - n = bus_node_allocate(bus, path); if (!n) return -ENOMEM; @@ -1892,7 +1886,9 @@ static int add_object_vtable_internal( m->member = v->x.method.member; m->vtable = v; - r = hashmap_put(bus->vtable_methods, m, m); + r = set_ensure_put(&bus->vtable_methods, &vtable_member_hash_ops, m); + if (r == 0) + r = -EEXIST; if (r < 0) { free(m); goto fail; @@ -1940,7 +1936,9 @@ static int add_object_vtable_internal( m->member = v->x.property.member; m->vtable = v; - r = hashmap_put(bus->vtable_properties, m, m); + r = set_ensure_put(&bus->vtable_properties, &vtable_member_hash_ops, m); + if (r == 0) + r = -EEXIST; if (r < 0) { free(m); goto fail; @@ -2128,7 +2126,7 @@ static int emit_properties_changed_on_interface( assert_return(member_name_is_valid(*property), -EINVAL); key.member = *property; - v = hashmap_get(bus->vtable_properties, &key); + v = set_get(bus->vtable_properties, &key); if (!v) return -ENOENT; @@ -2222,7 +2220,7 @@ static int emit_properties_changed_on_interface( struct vtable_member *v; key.member = *property; - assert_se(v = hashmap_get(bus->vtable_properties, &key)); + assert_se(v = set_get(bus->vtable_properties, &key)); assert(c == v->parent); if (!(v->vtable->flags & SD_BUS_VTABLE_PROPERTY_EMITS_INVALIDATION)) diff --git a/src/libsystemd/sd-bus/bus-slot.c b/src/libsystemd/sd-bus/bus-slot.c index 9f289575ef..8238e31c90 100644 --- a/src/libsystemd/sd-bus/bus-slot.c +++ b/src/libsystemd/sd-bus/bus-slot.c @@ -129,7 +129,7 @@ void bus_slot_disconnect(sd_bus_slot *slot, bool unref) { key.interface = slot->node_vtable.interface; key.member = v->x.method.member; - x = hashmap_remove(slot->bus->vtable_methods, &key); + x = set_remove(slot->bus->vtable_methods, &key); break; } @@ -141,7 +141,7 @@ void bus_slot_disconnect(sd_bus_slot *slot, bool unref) { key.interface = slot->node_vtable.interface; key.member = v->x.method.member; - x = hashmap_remove(slot->bus->vtable_properties, &key); + x = set_remove(slot->bus->vtable_properties, &key); break; }} diff --git a/src/libsystemd/sd-bus/sd-bus.c b/src/libsystemd/sd-bus/sd-bus.c index 699761fc31..c14d144753 100644 --- a/src/libsystemd/sd-bus/sd-bus.c +++ b/src/libsystemd/sd-bus/sd-bus.c @@ -220,14 +220,14 @@ static sd_bus* bus_free(sd_bus *b) { bus_reset_queues(b); - ordered_hashmap_free_free(b->reply_callbacks); + ordered_hashmap_free(b->reply_callbacks); prioq_free(b->reply_callbacks_prioq); assert(b->match_callbacks.type == BUS_MATCH_ROOT); bus_match_free(&b->match_callbacks); - hashmap_free_free(b->vtable_methods); - hashmap_free_free(b->vtable_properties); + set_free(b->vtable_methods); + set_free(b->vtable_properties); assert(hashmap_isempty(b->nodes)); hashmap_free(b->nodes); @@ -2332,10 +2332,6 @@ _public_ int sd_bus_call_async( if (!callback && !slot && !m->sealed) m->header->flags |= BUS_MESSAGE_NO_REPLY_EXPECTED; - r = ordered_hashmap_ensure_allocated(&bus->reply_callbacks, &uint64_hash_ops); - if (r < 0) - return r; - r = prioq_ensure_allocated(&bus->reply_callbacks_prioq, timeout_compare); if (r < 0) return r; @@ -2356,7 +2352,7 @@ _public_ int sd_bus_call_async( s->reply_callback.callback = callback; s->reply_callback.cookie = BUS_MESSAGE_COOKIE(m); - r = ordered_hashmap_put(bus->reply_callbacks, &s->reply_callback.cookie, &s->reply_callback); + r = ordered_hashmap_ensure_put(&bus->reply_callbacks, &uint64_hash_ops_value_free, &s->reply_callback.cookie, &s->reply_callback); if (r < 0) { s->reply_callback.cookie = 0; return r; From 2d23cadd1912d8d62457c4b4e319bc276f40a43b Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 22 Jan 2025 07:24:03 +0900 Subject: [PATCH 09/15] journal-file: use hash ops with destructor This also makes JournalFile.chain_cache allocated when necessary. --- src/libsystemd/sd-journal/journal-file.c | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/src/libsystemd/sd-journal/journal-file.c b/src/libsystemd/sd-journal/journal-file.c index 7e941edb19..fe45e3e7ce 100644 --- a/src/libsystemd/sd-journal/journal-file.c +++ b/src/libsystemd/sd-journal/journal-file.c @@ -291,7 +291,7 @@ JournalFile* journal_file_close(JournalFile *f) { safe_close(f->fd); free(f->path); - ordered_hashmap_free_free(f->chain_cache); + ordered_hashmap_free(f->chain_cache); #if HAVE_COMPRESSION free(f->compress_buffer); @@ -2646,7 +2646,7 @@ typedef struct ChainCacheItem { } ChainCacheItem; static void chain_cache_put( - OrderedHashmap *h, + JournalFile *f, ChainCacheItem *ci, uint64_t first, uint64_t array, @@ -2654,7 +2654,7 @@ static void chain_cache_put( uint64_t total, uint64_t last_index) { - assert(h); + assert(f); if (!ci) { /* If the chain item to cache for this chain is the @@ -2662,8 +2662,8 @@ static void chain_cache_put( if (array == first) return; - if (ordered_hashmap_size(h) >= CHAIN_CACHE_MAX) { - ci = ordered_hashmap_steal_first(h); + if (ordered_hashmap_size(f->chain_cache) >= CHAIN_CACHE_MAX) { + ci = ordered_hashmap_steal_first(f->chain_cache); assert(ci); } else { ci = new(ChainCacheItem, 1); @@ -2673,7 +2673,7 @@ static void chain_cache_put( ci->first = first; - if (ordered_hashmap_put(h, &ci->first, ci) < 0) { + if (ordered_hashmap_ensure_put(&f->chain_cache, &uint64_hash_ops_value_free, &ci->first, ci) < 0) { free(ci); return; } @@ -2847,7 +2847,7 @@ static int generic_array_get( r = journal_file_move_to_object(f, OBJECT_ENTRY, p, ret_object); if (r >= 0) { /* Let's cache this item for the next invocation */ - chain_cache_put(f->chain_cache, ci, first, a, journal_file_entry_array_item(f, o, 0), t, i); + chain_cache_put(f, ci, first, a, journal_file_entry_array_item(f, o, 0), t, i); if (ret_offset) *ret_offset = p; @@ -3187,7 +3187,7 @@ found: return -EBADMSG; /* Let's cache this item for the next invocation */ - chain_cache_put(f->chain_cache, ci, first, a, p, t, i); + chain_cache_put(f, ci, first, a, p, t, i); p = journal_file_entry_array_item(f, array, i); if (p == 0) @@ -4115,12 +4115,6 @@ int journal_file_open( } } - f->chain_cache = ordered_hashmap_new(&uint64_hash_ops); - if (!f->chain_cache) { - r = -ENOMEM; - goto fail; - } - if (f->fd < 0) { /* We pass O_NONBLOCK here, so that in case somebody pointed us to some character device node or FIFO * or so, we likely fail quickly than block for long. For regular files O_NONBLOCK has no effect, hence From 938a6b49bde97951e9c15f57039461a0c355f1e6 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 22 Jan 2025 10:39:35 +0900 Subject: [PATCH 10/15] sd-journal: use hash ops with destructor --- src/libsystemd/sd-journal/sd-journal.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libsystemd/sd-journal/sd-journal.c b/src/libsystemd/sd-journal/sd-journal.c index 858aeca45e..83f77206c3 100644 --- a/src/libsystemd/sd-journal/sd-journal.c +++ b/src/libsystemd/sd-journal/sd-journal.c @@ -83,7 +83,7 @@ static int journal_put_error(sd_journal *j, int r, const char *path) { return -ENOMEM; } - r = hashmap_ensure_put(&j->errors, NULL, INT_TO_PTR(r), copy); + r = hashmap_ensure_put(&j->errors, &trivial_hash_ops_value_free, INT_TO_PTR(r), copy); if (r == -EEXIST) return 0; if (r < 0) @@ -2560,7 +2560,7 @@ _public_ void sd_journal_close(sd_journal *j) { if (j->mmap) mmap_cache_unref(j->mmap); - hashmap_free_free(j->errors); + hashmap_free(j->errors); set_free(j->exclude_syslog_identifiers); From 04b7949ecf25b16f81ffb2798886c3d67115c238 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 22 Jan 2025 10:46:09 +0900 Subject: [PATCH 11/15] network: use hash ops with destructor --- src/network/networkd-address-pool.c | 2 +- src/network/networkd-manager.c | 2 +- src/network/networkd-network.c | 6 +++--- src/network/networkd-nexthop.c | 16 +++++++--------- 4 files changed, 12 insertions(+), 14 deletions(-) diff --git a/src/network/networkd-address-pool.c b/src/network/networkd-address-pool.c index d9ac78a1a9..8e4d6826a8 100644 --- a/src/network/networkd-address-pool.c +++ b/src/network/networkd-address-pool.c @@ -32,7 +32,7 @@ static int address_pool_new( .in_addr = *u, }; - r = ordered_set_ensure_put(&m->address_pools, NULL, p); + r = ordered_set_ensure_put(&m->address_pools, &trivial_hash_ops_free, p); if (r < 0) return r; diff --git a/src/network/networkd-manager.c b/src/network/networkd-manager.c index 867feeff1b..1592e258ec 100644 --- a/src/network/networkd-manager.c +++ b/src/network/networkd-manager.c @@ -692,7 +692,7 @@ Manager* manager_free(Manager *m) { m->wiphy_by_name = hashmap_free(m->wiphy_by_name); m->wiphy_by_index = hashmap_free_with_destructor(m->wiphy_by_index, wiphy_free); - ordered_set_free_free(m->address_pools); + ordered_set_free(m->address_pools); hashmap_free(m->route_table_names_by_number); hashmap_free(m->route_table_numbers_by_name); diff --git a/src/network/networkd-network.c b/src/network/networkd-network.c index 3ecddff129..7de0027aae 100644 --- a/src/network/networkd-network.c +++ b/src/network/networkd-network.c @@ -170,7 +170,7 @@ int network_verify(Network *network) { network->bond_name = mfree(network->bond_name); network->bridge_name = mfree(network->bridge_name); network->vrf_name = mfree(network->vrf_name); - network->stacked_netdev_names = hashmap_free_free_key(network->stacked_netdev_names); + network->stacked_netdev_names = hashmap_free(network->stacked_netdev_names); if (network->bond) { /* Bonding slave does not support addressing. */ @@ -818,7 +818,7 @@ static Network *network_free(Network *network) { free(network->bridge_name); free(network->bond_name); free(network->vrf_name); - hashmap_free_free_key(network->stacked_netdev_names); + hashmap_free(network->stacked_netdev_names); netdev_unref(network->bridge); netdev_unref(network->bond); netdev_unref(network->vrf); @@ -949,7 +949,7 @@ int config_parse_stacked_netdev( if (!name) return log_oom(); - r = hashmap_ensure_put(h, &string_hash_ops, name, INT_TO_PTR(kind)); + r = hashmap_ensure_put(h, &string_hash_ops_free, name, INT_TO_PTR(kind)); if (r == -ENOMEM) return log_oom(); if (r < 0) diff --git a/src/network/networkd-nexthop.c b/src/network/networkd-nexthop.c index 18b130b959..a3bf4aa1c0 100644 --- a/src/network/networkd-nexthop.c +++ b/src/network/networkd-nexthop.c @@ -97,7 +97,7 @@ static NextHop* nexthop_free(NextHop *nexthop) { nexthop_detach_impl(nexthop); config_section_free(nexthop->section); - hashmap_free_free(nexthop->group); + hashmap_free(nexthop->group); set_free(nexthop->nexthops); set_free(nexthop->routes); @@ -271,7 +271,7 @@ static int nexthop_dup(const NextHop *src, NextHop **ret) { if (!g) return -ENOMEM; - r = hashmap_ensure_put(&dest->group, NULL, UINT32_TO_PTR(g->id), g); + r = hashmap_ensure_put(&dest->group, &trivial_hash_ops_value_free, UINT32_TO_PTR(g->id), g); if (r < 0) return r; if (r > 0) @@ -1018,7 +1018,7 @@ void link_forget_nexthops(Link *link) { } static int nexthop_update_group(NextHop *nexthop, sd_netlink_message *message) { - _cleanup_hashmap_free_free_ Hashmap *h = NULL; + _cleanup_hashmap_free_ Hashmap *h = NULL; _cleanup_free_ struct nexthop_grp *group = NULL; size_t size = 0, n_group; int r; @@ -1058,7 +1058,7 @@ static int nexthop_update_group(NextHop *nexthop, sd_netlink_message *message) { if (!nhg) return log_oom(); - r = hashmap_ensure_put(&h, NULL, UINT32_TO_PTR(nhg->id), nhg); + r = hashmap_ensure_put(&h, &trivial_hash_ops_value_free, UINT32_TO_PTR(nhg->id), nhg); if (r == -ENOMEM) return log_oom(); if (r < 0) { @@ -1069,9 +1069,7 @@ static int nexthop_update_group(NextHop *nexthop, sd_netlink_message *message) { TAKE_PTR(nhg); } - hashmap_free_free(nexthop->group); - nexthop->group = TAKE_PTR(h); - + hashmap_free_and_replace(nexthop->group, h); nexthop_attach_to_group_members(nexthop); return 0; } @@ -1377,7 +1375,7 @@ static int config_parse_nexthop_group( int r; if (isempty(rvalue)) { - *group = hashmap_free_free(*group); + *group = hashmap_free(*group); return 1; } @@ -1431,7 +1429,7 @@ static int config_parse_nexthop_group( continue; } - r = hashmap_ensure_put(group, NULL, UINT32_TO_PTR(nhg->id), nhg); + r = hashmap_ensure_put(group, &trivial_hash_ops_value_free, UINT32_TO_PTR(nhg->id), nhg); if (r == -ENOMEM) return log_oom(); if (r == -EEXIST) { From 60cc858e9d8238e8bb83a6c2dfcfbb4b94f4ad84 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 22 Jan 2025 10:49:07 +0900 Subject: [PATCH 12/15] exec-util: use hash ops with destructor --- src/shared/exec-util.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/shared/exec-util.c b/src/shared/exec-util.c index c673e344ee..bc997a9383 100644 --- a/src/shared/exec-util.c +++ b/src/shared/exec-util.c @@ -99,7 +99,7 @@ static int do_execute( char *envp[], ExecDirFlags flags) { - _cleanup_hashmap_free_free_ Hashmap *pids = NULL; + _cleanup_hashmap_free_ Hashmap *pids = NULL; bool parallel_execution; int r; @@ -114,12 +114,6 @@ static int do_execute( parallel_execution = FLAGS_SET(flags, EXEC_DIR_PARALLEL) && !callbacks; - if (parallel_execution) { - pids = hashmap_new(NULL); - if (!pids) - return log_oom(); - } - /* Abort execution of this process after the timeout. We simply rely on SIGALRM as * default action terminating the process, and turn on alarm(). */ @@ -176,7 +170,7 @@ static int do_execute( continue; if (parallel_execution) { - r = hashmap_put(pids, PID_TO_PTR(pid), t); + r = hashmap_ensure_put(&pids, &trivial_hash_ops_value_free, PID_TO_PTR(pid), t); if (r < 0) return log_oom(); t = NULL; From 06835cb397069e77043d34002cc3324c1f1184c8 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 22 Jan 2025 10:51:13 +0900 Subject: [PATCH 13/15] remount-fs: use hash ops with destructor --- src/remount-fs/remount-fs.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/remount-fs/remount-fs.c b/src/remount-fs/remount-fs.c index 37c7b389b4..21167a7b4a 100644 --- a/src/remount-fs/remount-fs.c +++ b/src/remount-fs/remount-fs.c @@ -34,7 +34,7 @@ static int track_pid(Hashmap **h, const char *path, pid_t pid) { if (!c) return log_oom(); - r = hashmap_ensure_put(h, NULL, PID_TO_PTR(pid), c); + r = hashmap_ensure_put(h, &trivial_hash_ops_value_free, PID_TO_PTR(pid), c); if (r == -ENOMEM) return log_oom(); if (r < 0) @@ -70,7 +70,7 @@ static int do_remount(const char *path, bool force_rw, Hashmap **pids) { } static int remount_by_fstab(Hashmap **ret_pids) { - _cleanup_hashmap_free_free_ Hashmap *pids = NULL; + _cleanup_hashmap_free_ Hashmap *pids = NULL; _cleanup_endmntent_ FILE *f = NULL; bool has_root = false; struct mntent* me; @@ -108,7 +108,7 @@ static int remount_by_fstab(Hashmap **ret_pids) { } static int run(int argc, char *argv[]) { - _cleanup_hashmap_free_free_ Hashmap *pids = NULL; + _cleanup_hashmap_free_ Hashmap *pids = NULL; int r; log_setup(); From 58f0cd14a0b9f3f489c7dc40f8a73fa58a886c54 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 22 Jan 2025 11:10:10 +0900 Subject: [PATCH 14/15] test: use hash ops with destructor --- src/test/test-hashmap-plain.c | 105 +++++++++++++------------------- src/test/test-mountpoint-util.c | 4 +- 2 files changed, 45 insertions(+), 64 deletions(-) diff --git a/src/test/test-hashmap-plain.c b/src/test/test-hashmap-plain.c index 81effec21a..6b6058b08a 100644 --- a/src/test/test-hashmap-plain.c +++ b/src/test/test-hashmap-plain.c @@ -74,36 +74,21 @@ TEST(hashmap_ensure_replace) { } TEST(hashmap_copy) { - _cleanup_hashmap_free_ Hashmap *m = NULL; - _cleanup_hashmap_free_free_ Hashmap *copy = NULL; - char *val1, *val2, *val3, *val4, *r; + _cleanup_hashmap_free_ Hashmap *m = NULL, *copy = NULL; - val1 = strdup("val1"); - assert_se(val1); - val2 = strdup("val2"); - assert_se(val2); - val3 = strdup("val3"); - assert_se(val3); - val4 = strdup("val4"); - assert_se(val4); + ASSERT_NOT_NULL(m = hashmap_new(&string_hash_ops)); - m = hashmap_new(&string_hash_ops); + ASSERT_OK_POSITIVE(hashmap_put(m, "key 1", (void*) "val1")); + ASSERT_OK_POSITIVE(hashmap_put(m, "key 2", (void*) "val2")); + ASSERT_OK_POSITIVE(hashmap_put(m, "key 3", (void*) "val3")); + ASSERT_OK_POSITIVE(hashmap_put(m, "key 4", (void*) "val4")); - hashmap_put(m, "key 1", val1); - hashmap_put(m, "key 2", val2); - hashmap_put(m, "key 3", val3); - hashmap_put(m, "key 4", val4); + ASSERT_NOT_NULL(copy = hashmap_copy(m)); - copy = hashmap_copy(m); - - r = hashmap_get(copy, "key 1"); - ASSERT_STREQ(r, "val1"); - r = hashmap_get(copy, "key 2"); - ASSERT_STREQ(r, "val2"); - r = hashmap_get(copy, "key 3"); - ASSERT_STREQ(r, "val3"); - r = hashmap_get(copy, "key 4"); - ASSERT_STREQ(r, "val4"); + ASSERT_STREQ(hashmap_get(copy, "key 1"), "val1"); + ASSERT_STREQ(hashmap_get(copy, "key 2"), "val2"); + ASSERT_STREQ(hashmap_get(copy, "key 3"), "val3"); + ASSERT_STREQ(hashmap_get(copy, "key 4"), "val4"); } TEST(hashmap_get_strv) { @@ -140,7 +125,7 @@ TEST(hashmap_get_strv) { } TEST(hashmap_move_one) { - _cleanup_hashmap_free_free_ Hashmap *m = NULL, *n = NULL; + _cleanup_hashmap_free_ Hashmap *m = NULL, *n = NULL; char *val1, *val2, *val3, *val4, *r; val1 = strdup("val1"); @@ -152,8 +137,8 @@ TEST(hashmap_move_one) { val4 = strdup("val4"); assert_se(val4); - m = hashmap_new(&string_hash_ops); - n = hashmap_new(&string_hash_ops); + m = hashmap_new(&string_hash_ops_value_free); + n = hashmap_new(&string_hash_ops_value_free); hashmap_put(m, "key 1", val1); hashmap_put(m, "key 2", val2); @@ -176,7 +161,7 @@ TEST(hashmap_move_one) { } TEST(hashmap_move) { - _cleanup_hashmap_free_free_ Hashmap *m = NULL, *n = NULL; + _cleanup_hashmap_free_ Hashmap *m = NULL, *n = NULL; char *val1, *val2, *val3, *val4, *r; val1 = strdup("val1"); @@ -188,8 +173,8 @@ TEST(hashmap_move) { val4 = strdup("val4"); assert_se(val4); - m = hashmap_new(&string_hash_ops); - n = hashmap_new(&string_hash_ops); + m = hashmap_new(&string_hash_ops_value_free); + n = hashmap_new(&string_hash_ops_value_free); hashmap_put(n, "key 1", strdup(val1)); hashmap_put(m, "key 1", val1); @@ -282,7 +267,7 @@ TEST(hashmap_remove1) { } TEST(hashmap_remove2) { - _cleanup_hashmap_free_free_free_ Hashmap *m = NULL; + _cleanup_hashmap_free_ Hashmap *m = NULL; char key1[] = "key 1"; char key2[] = "key 2"; char val1[] = "val 1"; @@ -292,7 +277,7 @@ TEST(hashmap_remove2) { r = hashmap_remove2(NULL, "key 1", &r2); ASSERT_NULL(r); - m = hashmap_new(&string_hash_ops); + m = hashmap_new(&string_hash_ops_free_free); assert_se(m); r = hashmap_remove2(m, "no such key", &r2); @@ -480,7 +465,7 @@ TEST(hashmap_foreach_key) { } TEST(hashmap_foreach) { - _cleanup_hashmap_free_free_ Hashmap *m = NULL; + _cleanup_hashmap_free_ Hashmap *m = NULL; bool value_found[] = { false, false, false, false }; char *val1, *val2, *val3, *val4, *s; unsigned count; @@ -499,7 +484,7 @@ TEST(hashmap_foreach) { count++; assert_se(count == 0); - m = hashmap_new(&string_hash_ops); + m = hashmap_new(&string_hash_ops_value_free); count = 0; HASHMAP_FOREACH(s, m) @@ -527,8 +512,7 @@ TEST(hashmap_foreach) { } TEST(hashmap_merge) { - _cleanup_hashmap_free_free_ Hashmap *m = NULL; - _cleanup_hashmap_free_ Hashmap *n = NULL; + _cleanup_hashmap_free_ Hashmap *m = NULL, *n = NULL; char *val1, *val2, *val3, *val4, *r; val1 = strdup("my val1"); @@ -540,7 +524,7 @@ TEST(hashmap_merge) { val4 = strdup("my val4"); assert_se(val4); - m = hashmap_new(&string_hash_ops); + m = hashmap_new(&string_hash_ops_value_free); n = hashmap_new(&string_hash_ops); hashmap_put(m, "Key 1", val1); @@ -559,13 +543,13 @@ TEST(hashmap_merge) { } TEST(hashmap_contains) { - _cleanup_hashmap_free_free_ Hashmap *m = NULL; + _cleanup_hashmap_free_ Hashmap *m = NULL; char *val1; val1 = strdup("my val"); assert_se(val1); - m = hashmap_new(&string_hash_ops); + m = hashmap_new(&string_hash_ops_value_free); assert_se(!hashmap_contains(m, "Key 1")); hashmap_put(m, "Key 1", val1); @@ -578,13 +562,13 @@ TEST(hashmap_contains) { } TEST(hashmap_isempty) { - _cleanup_hashmap_free_free_ Hashmap *m = NULL; + _cleanup_hashmap_free_ Hashmap *m = NULL; char *val1; val1 = strdup("my val"); assert_se(val1); - m = hashmap_new(&string_hash_ops); + m = hashmap_new(&string_hash_ops_value_free); assert_se(hashmap_isempty(m)); hashmap_put(m, "Key 1", val1); @@ -594,7 +578,7 @@ TEST(hashmap_isempty) { } TEST(hashmap_size) { - _cleanup_hashmap_free_free_ Hashmap *m = NULL; + _cleanup_hashmap_free_ Hashmap *m = NULL; char *val1, *val2, *val3, *val4; val1 = strdup("my val"); @@ -609,7 +593,7 @@ TEST(hashmap_size) { assert_se(hashmap_size(NULL) == 0); assert_se(hashmap_buckets(NULL) == 0); - m = hashmap_new(&string_hash_ops); + m = hashmap_new(&string_hash_ops_value_free); hashmap_put(m, "Key 1", val1); hashmap_put(m, "Key 2", val2); @@ -622,7 +606,7 @@ TEST(hashmap_size) { } TEST(hashmap_get) { - _cleanup_hashmap_free_free_ Hashmap *m = NULL; + _cleanup_hashmap_free_ Hashmap *m = NULL; char *r; char *val; @@ -632,7 +616,7 @@ TEST(hashmap_get) { r = hashmap_get(NULL, "Key 1"); ASSERT_NULL(r); - m = hashmap_new(&string_hash_ops); + m = hashmap_new(&string_hash_ops_value_free); hashmap_put(m, "Key 1", val); @@ -646,7 +630,7 @@ TEST(hashmap_get) { } TEST(hashmap_get2) { - _cleanup_hashmap_free_free_free_ Hashmap *m = NULL; + _cleanup_hashmap_free_ Hashmap *m = NULL; char *r; char *val; char key_orig[] = "Key 1"; @@ -661,7 +645,7 @@ TEST(hashmap_get2) { r = hashmap_get2(NULL, key_orig, &key_copy); ASSERT_NULL(r); - m = hashmap_new(&string_hash_ops); + m = hashmap_new(&string_hash_ops_free_free); hashmap_put(m, key_copy, val); key_copy = NULL; @@ -870,32 +854,29 @@ TEST(hashmap_steal_first) { assert_se(hashmap_isempty(m)); } -TEST(hashmap_clear_free_free) { +DEFINE_PRIVATE_HASH_OPS_WITH_KEY_DESTRUCTOR(test_hash_ops_key, char, string_hash_func, string_compare_func, free); +DEFINE_PRIVATE_HASH_OPS_FULL(test_hash_ops_full, char, string_hash_func, string_compare_func, free, char, free); + +TEST(hashmap_clear) { _cleanup_hashmap_free_ Hashmap *m = NULL; - m = hashmap_new(&string_hash_ops); + m = hashmap_new(&string_hash_ops_free_free); assert_se(m); assert_se(hashmap_put(m, strdup("key 1"), NULL) == 1); assert_se(hashmap_put(m, strdup("key 2"), NULL) == 1); assert_se(hashmap_put(m, strdup("key 3"), NULL) == 1); - hashmap_clear_free_free(m); + hashmap_clear(m); assert_se(hashmap_isempty(m)); assert_se(hashmap_put(m, strdup("key 1"), strdup("value 1")) == 1); assert_se(hashmap_put(m, strdup("key 2"), strdup("value 2")) == 1); assert_se(hashmap_put(m, strdup("key 3"), strdup("value 3")) == 1); - hashmap_clear_free_free(m); + hashmap_clear(m); assert_se(hashmap_isempty(m)); -} - -DEFINE_PRIVATE_HASH_OPS_WITH_KEY_DESTRUCTOR(test_hash_ops_key, char, string_hash_func, string_compare_func, free); -DEFINE_PRIVATE_HASH_OPS_FULL(test_hash_ops_full, char, string_hash_func, string_compare_func, free, char, free); - -TEST(hashmap_clear_free_with_destructor) { - _cleanup_hashmap_free_ Hashmap *m = NULL; + m = hashmap_free(m); m = hashmap_new(&test_hash_ops_key); assert_se(m); @@ -904,7 +885,7 @@ TEST(hashmap_clear_free_with_destructor) { assert_se(hashmap_put(m, strdup("key 2"), NULL) == 1); assert_se(hashmap_put(m, strdup("key 3"), NULL) == 1); - hashmap_clear_free(m); + hashmap_clear(m); assert_se(hashmap_isempty(m)); m = hashmap_free(m); @@ -915,7 +896,7 @@ TEST(hashmap_clear_free_with_destructor) { assert_se(hashmap_put(m, strdup("key 2"), strdup("value 2")) == 1); assert_se(hashmap_put(m, strdup("key 3"), strdup("value 3")) == 1); - hashmap_clear_free(m); + hashmap_clear(m); assert_se(hashmap_isempty(m)); } diff --git a/src/test/test-mountpoint-util.c b/src/test/test-mountpoint-util.c index c712cd8f2b..32a3b927e5 100644 --- a/src/test/test-mountpoint-util.c +++ b/src/test/test-mountpoint-util.c @@ -50,13 +50,13 @@ TEST(mount_propagation_flag) { TEST(mnt_id) { _cleanup_fclose_ FILE *f = NULL; - _cleanup_hashmap_free_free_ Hashmap *h = NULL; + _cleanup_hashmap_free_ Hashmap *h = NULL; char *p; void *k; int r; assert_se(f = fopen("/proc/self/mountinfo", "re")); - assert_se(h = hashmap_new(&trivial_hash_ops)); + assert_se(h = hashmap_new(&trivial_hash_ops_value_free)); for (;;) { _cleanup_free_ char *line = NULL, *path = NULL; From 38f7edd9d36992d334659628b636332acfd97ca5 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 22 Jan 2025 11:37:06 +0900 Subject: [PATCH 15/15] hashmap: drop hashmap_free_free() and friends --- src/basic/hashmap.h | 52 ----------------------------------------- src/basic/ordered-set.h | 10 -------- 2 files changed, 62 deletions(-) diff --git a/src/basic/hashmap.h b/src/basic/hashmap.h index 01a4fb3204..091062b5e9 100644 --- a/src/basic/hashmap.h +++ b/src/basic/hashmap.h @@ -99,27 +99,6 @@ static inline OrderedHashmap* ordered_hashmap_free(OrderedHashmap *h) { return (void*) _hashmap_free(HASHMAP_BASE(h), NULL, NULL); } -static inline Hashmap* hashmap_free_free(Hashmap *h) { - return (void*) _hashmap_free(HASHMAP_BASE(h), NULL, free); -} -static inline OrderedHashmap* ordered_hashmap_free_free(OrderedHashmap *h) { - return (void*) _hashmap_free(HASHMAP_BASE(h), NULL, free); -} - -static inline Hashmap* hashmap_free_free_key(Hashmap *h) { - return (void*) _hashmap_free(HASHMAP_BASE(h), free, NULL); -} -static inline OrderedHashmap* ordered_hashmap_free_free_key(OrderedHashmap *h) { - return (void*) _hashmap_free(HASHMAP_BASE(h), free, NULL); -} - -static inline Hashmap* hashmap_free_free_free(Hashmap *h) { - return (void*) _hashmap_free(HASHMAP_BASE(h), free, free); -} -static inline OrderedHashmap* ordered_hashmap_free_free_free(OrderedHashmap *h) { - return (void*) _hashmap_free(HASHMAP_BASE(h), free, free); -} - IteratedCache* iterated_cache_free(IteratedCache *cache); int iterated_cache_get(IteratedCache *cache, const void ***res_keys, const void ***res_values, unsigned *res_n_entries); @@ -293,27 +272,6 @@ static inline void ordered_hashmap_clear(OrderedHashmap *h) { _hashmap_clear(HASHMAP_BASE(h), NULL, NULL); } -static inline void hashmap_clear_free(Hashmap *h) { - _hashmap_clear(HASHMAP_BASE(h), NULL, free); -} -static inline void ordered_hashmap_clear_free(OrderedHashmap *h) { - _hashmap_clear(HASHMAP_BASE(h), NULL, free); -} - -static inline void hashmap_clear_free_key(Hashmap *h) { - _hashmap_clear(HASHMAP_BASE(h), free, NULL); -} -static inline void ordered_hashmap_clear_free_key(OrderedHashmap *h) { - _hashmap_clear(HASHMAP_BASE(h), free, NULL); -} - -static inline void hashmap_clear_free_free(Hashmap *h) { - _hashmap_clear(HASHMAP_BASE(h), free, free); -} -static inline void ordered_hashmap_clear_free_free(OrderedHashmap *h) { - _hashmap_clear(HASHMAP_BASE(h), free, free); -} - /* * Note about all *_first*() functions * @@ -459,20 +417,10 @@ static inline int ordered_hashmap_dump_keys_sorted(OrderedHashmap *h, void ***re _ORDERED_HASHMAP_FOREACH_KEY(e, k, h, UNIQ_T(i, UNIQ)) DEFINE_TRIVIAL_CLEANUP_FUNC(Hashmap*, hashmap_free); -DEFINE_TRIVIAL_CLEANUP_FUNC(Hashmap*, hashmap_free_free); -DEFINE_TRIVIAL_CLEANUP_FUNC(Hashmap*, hashmap_free_free_key); -DEFINE_TRIVIAL_CLEANUP_FUNC(Hashmap*, hashmap_free_free_free); DEFINE_TRIVIAL_CLEANUP_FUNC(OrderedHashmap*, ordered_hashmap_free); -DEFINE_TRIVIAL_CLEANUP_FUNC(OrderedHashmap*, ordered_hashmap_free_free); -DEFINE_TRIVIAL_CLEANUP_FUNC(OrderedHashmap*, ordered_hashmap_free_free_key); -DEFINE_TRIVIAL_CLEANUP_FUNC(OrderedHashmap*, ordered_hashmap_free_free_free); #define _cleanup_hashmap_free_ _cleanup_(hashmap_freep) -#define _cleanup_hashmap_free_free_ _cleanup_(hashmap_free_freep) -#define _cleanup_hashmap_free_free_free_ _cleanup_(hashmap_free_free_freep) #define _cleanup_ordered_hashmap_free_ _cleanup_(ordered_hashmap_freep) -#define _cleanup_ordered_hashmap_free_free_ _cleanup_(ordered_hashmap_free_freep) -#define _cleanup_ordered_hashmap_free_free_free_ _cleanup_(ordered_hashmap_free_free_freep) DEFINE_TRIVIAL_CLEANUP_FUNC(IteratedCache*, iterated_cache_free); diff --git a/src/basic/ordered-set.h b/src/basic/ordered-set.h index e73da20573..5f2b45309f 100644 --- a/src/basic/ordered-set.h +++ b/src/basic/ordered-set.h @@ -22,18 +22,10 @@ static inline void ordered_set_clear(OrderedSet *s) { return ordered_hashmap_clear((OrderedHashmap*) s); } -static inline void ordered_set_clear_free(OrderedSet *s) { - return ordered_hashmap_clear_free((OrderedHashmap*) s); -} - static inline OrderedSet* ordered_set_free(OrderedSet *s) { return (OrderedSet*) ordered_hashmap_free((OrderedHashmap*) s); } -static inline OrderedSet* ordered_set_free_free(OrderedSet *s) { - return (OrderedSet*) ordered_hashmap_free_free((OrderedHashmap*) s); -} - static inline int ordered_set_contains(OrderedSet *s, const void *p) { return ordered_hashmap_contains((OrderedHashmap*) s, p); } @@ -103,7 +95,5 @@ void ordered_set_print(FILE *f, const char *field, OrderedSet *s); ordered_set_free(ordered_set_clear_with_destructor(s, f)) DEFINE_TRIVIAL_CLEANUP_FUNC(OrderedSet*, ordered_set_free); -DEFINE_TRIVIAL_CLEANUP_FUNC(OrderedSet*, ordered_set_free_free); #define _cleanup_ordered_set_free_ _cleanup_(ordered_set_freep) -#define _cleanup_ordered_set_free_free_ _cleanup_(ordered_set_free_freep)