From be32732168e07b7d52ec77fa67cf93a80a9a8293 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 29 Apr 2020 08:47:51 +0200 Subject: [PATCH 1/9] basic/set: let set_put_strdup() create the set with string hash ops If we're using a set with _put_strdup(), most of the time we want to use string hash ops on the set, and free the strings when done. This defines the appropriate a new string_hash_ops_free structure to automatically free the keys when removing the set, and makes set_put_strdup() and set_put_strdupv() instantiate the set with those hash ops. hashmap_put_strdup() was already doing something similar. (It is OK to instantiate the set earlier, possibly with a different hash ops structure. set_put_strdup() will then use the existing set. It is also OK to call set_free_free() instead of set_free() on a set with string_hash_ops_free, the effect is the same, we're just overriding the override of the cleanup function.) No functional change intended. --- src/analyze/analyze-security.c | 8 ++----- src/analyze/analyze.c | 10 +++----- src/basic/conf-files.c | 4 +++- src/basic/hash-funcs.c | 2 ++ src/basic/hash-funcs.h | 1 + src/basic/hashmap.c | 13 +++++++---- src/basic/set.h | 4 ++-- src/busctl/busctl.c | 2 +- src/core/mount.c | 4 ++-- src/core/unit.c | 2 +- src/libsystemd/sd-device/device-enumerator.c | 8 +++---- src/libsystemd/sd-device/sd-device.c | 24 +++++--------------- src/login/logind-acl.c | 2 +- src/network/networkd-link-bus.c | 2 +- src/nspawn/nspawn-cgroup.c | 10 +++----- src/portable/portable.c | 18 ++++----------- src/resolve/resolved-dns-trust-anchor.c | 3 +-- src/resolve/resolved-link-bus.c | 2 +- src/resolve/resolved-link.c | 2 +- src/shared/bus-util.c | 8 ++----- src/shared/bus-wait-for-jobs.c | 10 ++------ src/shared/logs-show.c | 23 ++++++++----------- src/shared/mount-util.c | 4 ++-- src/shared/unit-file.c | 4 ++-- src/udev/udevadm-trigger.c | 9 ++++---- 25 files changed, 71 insertions(+), 108 deletions(-) diff --git a/src/analyze/analyze-security.c b/src/analyze/analyze-security.c index d681251c04..0137883976 100644 --- a/src/analyze/analyze-security.c +++ b/src/analyze/analyze-security.c @@ -141,7 +141,7 @@ static void security_info_free(struct security_info *i) { strv_free(i->supplementary_groups); strv_free(i->system_call_architectures); - set_free_free(i->system_call_filter); + set_free(i->system_call_filter); } static bool security_info_runs_privileged(const struct security_info *i) { @@ -1728,11 +1728,7 @@ static int property_read_system_call_filter( if (r == 0) break; - r = set_ensure_allocated(&info->system_call_filter, &string_hash_ops); - if (r < 0) - return r; - - r = set_put_strdup(info->system_call_filter, name); + r = set_put_strdup(&info->system_call_filter, name); if (r < 0) return r; } diff --git a/src/analyze/analyze.c b/src/analyze/analyze.c index 3ea9041c18..3e7740593d 100644 --- a/src/analyze/analyze.c +++ b/src/analyze/analyze.c @@ -1655,7 +1655,7 @@ static int dump_exit_status(int argc, char *argv[], void *userdata) { #if HAVE_SECCOMP static int load_kernel_syscalls(Set **ret) { - _cleanup_(set_free_freep) Set *syscalls = NULL; + _cleanup_set_free_ Set *syscalls = NULL; _cleanup_fclose_ FILE *f = NULL; int r; @@ -1691,11 +1691,7 @@ static int load_kernel_syscalls(Set **ret) { if (STR_IN_SET(e, "newuname", "newfstat", "newstat", "newlstat", "sysctl")) continue; - r = set_ensure_allocated(&syscalls, &string_hash_ops); - if (r < 0) - return log_oom(); - - r = set_put_strdup(syscalls, e); + r = set_put_strdup(&syscalls, e); if (r < 0) return log_error_errno(r, "Failed to add system call to list: %m"); } @@ -1735,7 +1731,7 @@ static int dump_syscall_filters(int argc, char *argv[], void *userdata) { (void) pager_open(arg_pager_flags); if (strv_isempty(strv_skip(argv, 1))) { - _cleanup_(set_free_freep) Set *kernel = NULL; + _cleanup_set_free_ Set *kernel = NULL; int i, k; k = load_kernel_syscalls(&kernel); diff --git a/src/basic/conf-files.c b/src/basic/conf-files.c index 58eb62fb7a..eb19516c2a 100644 --- a/src/basic/conf-files.c +++ b/src/basic/conf-files.c @@ -77,8 +77,10 @@ static int files_add( /* Is this a masking entry? */ if ((flags & CONF_FILES_FILTER_MASKED)) if (null_or_empty(&st)) { + assert(masked); + /* Mark this one as masked */ - r = set_put_strdup(masked, de->d_name); + r = set_put_strdup(&masked, de->d_name); if (r < 0) return r; diff --git a/src/basic/hash-funcs.c b/src/basic/hash-funcs.c index fce339512c..fee0ba98eb 100644 --- a/src/basic/hash-funcs.c +++ b/src/basic/hash-funcs.c @@ -10,6 +10,8 @@ void string_hash_func(const char *p, struct siphash *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, char, free); diff --git a/src/basic/hash-funcs.h b/src/basic/hash-funcs.h index 7bb5d1cd02..264316c3dc 100644 --- a/src/basic/hash-funcs.h +++ b/src/basic/hash-funcs.h @@ -76,6 +76,7 @@ struct hash_ops { 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_free_free; void path_hash_func(const char *p, struct siphash *state); diff --git a/src/basic/hashmap.c b/src/basic/hashmap.c index 4853514c96..7abe62fa93 100644 --- a/src/basic/hashmap.c +++ b/src/basic/hashmap.c @@ -1793,23 +1793,28 @@ int hashmap_put_strdup(Hashmap **h, const char *k, const char *v) { return 0; } -int set_put_strdup(Set *s, const char *p) { +int set_put_strdup(Set **s, const char *p) { char *c; + int r; assert(s); assert(p); - if (set_contains(s, (char*) p)) + r = set_ensure_allocated(s, &string_hash_ops_free); + if (r < 0) + return r; + + if (set_contains(*s, (char*) p)) return 0; c = strdup(p); if (!c) return -ENOMEM; - return set_consume(s, c); + return set_consume(*s, c); } -int set_put_strdupv(Set *s, char **l) { +int set_put_strdupv(Set **s, char **l) { int n = 0, r; char **i; diff --git a/src/basic/set.h b/src/basic/set.h index 5f1956177e..f3501d17ae 100644 --- a/src/basic/set.h +++ b/src/basic/set.h @@ -113,8 +113,8 @@ static inline char **set_get_strv(Set *s) { } int set_consume(Set *s, void *value); -int set_put_strdup(Set *s, const char *p); -int set_put_strdupv(Set *s, char **l); +int set_put_strdup(Set **s, const char *p); +int set_put_strdupv(Set **s, char **l); int set_put_strsplit(Set *s, const char *v, const char *separators, ExtractFlags flags); #define SET_FOREACH(e, s, i) \ diff --git a/src/busctl/busctl.c b/src/busctl/busctl.c index 4fb5a8cedc..9ab9a51198 100644 --- a/src/busctl/busctl.c +++ b/src/busctl/busctl.c @@ -458,7 +458,7 @@ static int on_path(const char *path, void *userdata) { assert(paths); - r = set_put_strdup(paths, path); + r = set_put_strdup(&paths, path); if (r < 0) return log_oom(); diff --git a/src/core/mount.c b/src/core/mount.c index 97caf3e734..016c63bb2e 100644 --- a/src/core/mount.c +++ b/src/core/mount.c @@ -1878,7 +1878,7 @@ static int mount_process_proc_self_mountinfo(Manager *m) { /* Remember that this device might just have disappeared */ if (set_ensure_allocated(&gone, &path_hash_ops) < 0 || - set_put_strdup(gone, mount->parameters_proc_self_mountinfo.what) < 0) + set_put_strdup(&gone, mount->parameters_proc_self_mountinfo.what) < 0) log_oom(); /* we don't care too much about OOM here... */ } @@ -1933,7 +1933,7 @@ static int mount_process_proc_self_mountinfo(Manager *m) { /* Track devices currently used */ if (set_ensure_allocated(&around, &path_hash_ops) < 0 || - set_put_strdup(around, mount->parameters_proc_self_mountinfo.what) < 0) + set_put_strdup(&around, mount->parameters_proc_self_mountinfo.what) < 0) log_oom(); } diff --git a/src/core/unit.c b/src/core/unit.c index 19da4a2af9..24d7e901ba 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -4709,7 +4709,7 @@ int unit_write_setting(Unit *u, UnitWriteFlags flags, const char *name, const ch /* Make sure the drop-in dir is registered in our path cache. This way we don't need to stupidly * recreate the cache after every drop-in we write. */ if (u->manager->unit_path_cache) { - r = set_put_strdup(u->manager->unit_path_cache, p); + r = set_put_strdup(&u->manager->unit_path_cache, p); if (r < 0) return r; } diff --git a/src/libsystemd/sd-device/device-enumerator.c b/src/libsystemd/sd-device/device-enumerator.c index a1932f41f9..cd3d75c517 100644 --- a/src/libsystemd/sd-device/device-enumerator.c +++ b/src/libsystemd/sd-device/device-enumerator.c @@ -101,7 +101,7 @@ _public_ int sd_device_enumerator_add_match_subsystem(sd_device_enumerator *enum if (r < 0) return r; - r = set_put_strdup(*set, subsystem); + r = set_put_strdup(set, subsystem); if (r < 0) return r; @@ -192,7 +192,7 @@ _public_ int sd_device_enumerator_add_match_sysname(sd_device_enumerator *enumer if (r < 0) return r; - r = set_put_strdup(enumerator->match_sysname, sysname); + r = set_put_strdup(&enumerator->match_sysname, sysname); if (r < 0) return r; @@ -211,7 +211,7 @@ _public_ int sd_device_enumerator_add_match_tag(sd_device_enumerator *enumerator if (r < 0) return r; - r = set_put_strdup(enumerator->match_tag, tag); + r = set_put_strdup(&enumerator->match_tag, tag); if (r < 0) return r; @@ -242,7 +242,7 @@ int device_enumerator_add_match_parent_incremental(sd_device_enumerator *enumera if (r < 0) return r; - r = set_put_strdup(enumerator->match_parent, path); + r = set_put_strdup(&enumerator->match_parent, path); if (r < 0) return r; diff --git a/src/libsystemd/sd-device/sd-device.c b/src/libsystemd/sd-device/sd-device.c index 1f2451f8e1..24f34dc182 100644 --- a/src/libsystemd/sd-device/sd-device.c +++ b/src/libsystemd/sd-device/sd-device.c @@ -68,9 +68,9 @@ static sd_device *device_free(sd_device *device) { ordered_hashmap_free_free_free(device->properties); ordered_hashmap_free_free_free(device->properties_db); hashmap_free_free_free(device->sysattr_values); - set_free_free(device->sysattrs); - set_free_free(device->tags); - set_free_free(device->devlinks); + set_free(device->sysattrs); + set_free(device->tags); + set_free(device->devlinks); return mfree(device); } @@ -1078,11 +1078,7 @@ int device_add_tag(sd_device *device, const char *tag) { if (!is_valid_tag(tag)) return -EINVAL; - r = set_ensure_allocated(&device->tags, &string_hash_ops); - if (r < 0) - return r; - - r = set_put_strdup(device->tags, tag); + r = set_put_strdup(&device->tags, tag); if (r < 0) return r; @@ -1098,11 +1094,7 @@ int device_add_devlink(sd_device *device, const char *devlink) { assert(device); assert(devlink); - r = set_ensure_allocated(&device->devlinks, &string_hash_ops); - if (r < 0) - return r; - - r = set_put_strdup(device->devlinks, devlink); + r = set_put_strdup(&device->devlinks, devlink); if (r < 0) return r; @@ -1591,10 +1583,6 @@ static int device_sysattrs_read_all(sd_device *device) { if (!dir) return -errno; - r = set_ensure_allocated(&device->sysattrs, &string_hash_ops); - if (r < 0) - return r; - FOREACH_DIRENT_ALL(dent, dir, return -errno) { _cleanup_free_ char *path = NULL; struct stat statbuf; @@ -1613,7 +1601,7 @@ static int device_sysattrs_read_all(sd_device *device) { if (!(statbuf.st_mode & S_IRUSR)) continue; - r = set_put_strdup(device->sysattrs, dent->d_name); + r = set_put_strdup(&device->sysattrs, dent->d_name); if (r < 0) return r; } diff --git a/src/login/logind-acl.c b/src/login/logind-acl.c index ff192c53eb..76af208af1 100644 --- a/src/login/logind-acl.c +++ b/src/login/logind-acl.c @@ -206,7 +206,7 @@ int devnode_acl_all(const char *seat, continue; log_device_debug(d, "Found udev node %s for seat %s", node, seat); - r = set_put_strdup(nodes, node); + r = set_put_strdup(&nodes, node); if (r < 0) return r; } diff --git a/src/network/networkd-link-bus.c b/src/network/networkd-link-bus.c index 62f84faee3..54d6bb2330 100644 --- a/src/network/networkd-link-bus.c +++ b/src/network/networkd-link-bus.c @@ -498,7 +498,7 @@ int bus_link_method_set_dnssec_negative_trust_anchors(sd_bus_message *message, v return -ENOMEM; STRV_FOREACH(i, ntas) { - r = set_put_strdup(ns, *i); + r = set_put_strdup(&ns, *i); if (r < 0) return r; } diff --git a/src/nspawn/nspawn-cgroup.c b/src/nspawn/nspawn-cgroup.c index f5048d9473..dcee851e93 100644 --- a/src/nspawn/nspawn-cgroup.c +++ b/src/nspawn/nspawn-cgroup.c @@ -199,16 +199,12 @@ int create_subcgroup(pid_t pid, bool keep_unit, CGroupUnified unified_requested) * namespace. */ static int get_process_controllers(Set **ret) { - _cleanup_set_free_free_ Set *controllers = NULL; + _cleanup_set_free_ Set *controllers = NULL; _cleanup_fclose_ FILE *f = NULL; int r; assert(ret); - controllers = set_new(&string_hash_ops); - if (!controllers) - return -ENOMEM; - f = fopen("/proc/self/cgroup", "re"); if (!f) return errno == ENOENT ? -ESRCH : -errno; @@ -237,7 +233,7 @@ static int get_process_controllers(Set **ret) { if (STR_IN_SET(l, "", "name=systemd", "name=unified")) continue; - r = set_put_strdup(controllers, l); + r = set_put_strdup(&controllers, l); if (r < 0) return r; } @@ -303,7 +299,7 @@ static int mount_legacy_cgns_supported( uid_t uid_range, const char *selinux_apifs_context) { - _cleanup_set_free_free_ Set *controllers = NULL; + _cleanup_set_free_ Set *controllers = NULL; const char *cgroup_root = "/sys/fs/cgroup", *c; int r; diff --git a/src/portable/portable.c b/src/portable/portable.c index 49087179c8..60bdc24af5 100644 --- a/src/portable/portable.c +++ b/src/portable/portable.c @@ -1156,10 +1156,6 @@ int portable_detach( return log_debug_errno(errno, "Failed to open '%s' directory: %m", where); } - unit_files = set_new(&string_hash_ops); - if (!unit_files) - return -ENOMEM; - markers = set_new(&path_hash_ops); if (!markers) return -ENOMEM; @@ -1172,7 +1168,7 @@ int portable_detach( continue; /* Filter out duplicates */ - if (set_get(unit_files, de->d_name)) + if (set_contains(unit_files, de->d_name)) continue; dirent_ensure_type(d, de); @@ -1197,7 +1193,7 @@ int portable_detach( if (r > 0) return sd_bus_error_setf(error, BUS_ERROR_UNIT_EXISTS, "Unit file '%s' is active, can't detach.", de->d_name); - r = set_put_strdup(unit_files, de->d_name); + r = set_put_strdup(&unit_files, de->d_name); if (r < 0) return log_debug_errno(r, "Failed to add unit name '%s' to set: %m", de->d_name); @@ -1310,7 +1306,7 @@ static int portable_get_state_internal( _cleanup_(lookup_paths_free) LookupPaths paths = {}; bool found_enabled = false, found_running = false; - _cleanup_set_free_free_ Set *unit_files = NULL; + _cleanup_set_free_ Set *unit_files = NULL; _cleanup_closedir_ DIR *d = NULL; const char *where; struct dirent *de; @@ -1336,10 +1332,6 @@ static int portable_get_state_internal( return log_debug_errno(errno, "Failed to open '%s' directory: %m", where); } - unit_files = set_new(&string_hash_ops); - if (!unit_files) - return -ENOMEM; - FOREACH_DIRENT(de, d, return log_debug_errno(errno, "Failed to enumerate '%s' directory: %m", where)) { UnitFileState state; @@ -1347,7 +1339,7 @@ static int portable_get_state_internal( continue; /* Filter out duplicates */ - if (set_get(unit_files, de->d_name)) + if (set_contains(unit_files, de->d_name)) continue; dirent_ensure_type(d, de); @@ -1372,7 +1364,7 @@ static int portable_get_state_internal( if (r > 0) found_running = true; - r = set_put_strdup(unit_files, de->d_name); + r = set_put_strdup(&unit_files, de->d_name); if (r < 0) return log_debug_errno(r, "Failed to add unit name '%s' to set: %m", de->d_name); } diff --git a/src/resolve/resolved-dns-trust-anchor.c b/src/resolve/resolved-dns-trust-anchor.c index 92842bcf89..843f4c0f45 100644 --- a/src/resolve/resolved-dns-trust-anchor.c +++ b/src/resolve/resolved-dns-trust-anchor.c @@ -184,11 +184,10 @@ static int dns_trust_anchor_add_builtin_negative(DnsTrustAnchor *d) { * unsigned. */ NULSTR_FOREACH(name, private_domains) { - if (dns_trust_anchor_knows_domain_positive(d, name)) continue; - r = set_put_strdup(d->negative_by_name, name); + r = set_put_strdup(&d->negative_by_name, name); if (r < 0) return r; } diff --git a/src/resolve/resolved-link-bus.c b/src/resolve/resolved-link-bus.c index 4ba179ce06..a2ca5cb8f6 100644 --- a/src/resolve/resolved-link-bus.c +++ b/src/resolve/resolved-link-bus.c @@ -629,7 +629,7 @@ int bus_link_method_set_dnssec_negative_trust_anchors(sd_bus_message *message, v return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid negative trust anchor domain: %s", *i); - r = set_put_strdup(ns, *i); + r = set_put_strdup(&ns, *i); if (r < 0) return r; } diff --git a/src/resolve/resolved-link.c b/src/resolve/resolved-link.c index f19fc2f3aa..5eb184a10f 100644 --- a/src/resolve/resolved-link.c +++ b/src/resolve/resolved-link.c @@ -493,7 +493,7 @@ static int link_update_dnssec_negative_trust_anchors(Link *l) { if (!ns) return -ENOMEM; - r = set_put_strdupv(ns, ntas); + r = set_put_strdupv(&ns, ntas); if (r < 0) return r; diff --git a/src/shared/bus-util.c b/src/shared/bus-util.c index df701a3bfe..c7611a6e85 100644 --- a/src/shared/bus-util.c +++ b/src/shared/bus-util.c @@ -1695,14 +1695,10 @@ int bus_introspect_implementations( if (impl != main_impl) bus_introspect_implementation(&intro, impl); - _cleanup_set_free_free_ Set *nodes = NULL; + _cleanup_set_free_ Set *nodes = NULL; for (size_t i = 0; impl->children && impl->children[i]; i++) { - r = set_ensure_allocated(&nodes, &string_hash_ops); - if (r < 0) - return log_oom(); - - r = set_put_strdup(nodes, impl->children[i]->path); + r = set_put_strdup(&nodes, impl->children[i]->path); if (r < 0) return log_oom(); } diff --git a/src/shared/bus-wait-for-jobs.c b/src/shared/bus-wait-for-jobs.c index 4e6b862d5e..eb33ba2340 100644 --- a/src/shared/bus-wait-for-jobs.c +++ b/src/shared/bus-wait-for-jobs.c @@ -65,7 +65,7 @@ void bus_wait_for_jobs_free(BusWaitForJobs *d) { if (!d) return; - set_free_free(d->jobs); + set_free(d->jobs); sd_bus_slot_unref(d->slot_disconnected); sd_bus_slot_unref(d->slot_job_removed); @@ -315,15 +315,9 @@ int bus_wait_for_jobs(BusWaitForJobs *d, bool quiet, const char* const* extra_ar } int bus_wait_for_jobs_add(BusWaitForJobs *d, const char *path) { - int r; - assert(d); - r = set_ensure_allocated(&d->jobs, &string_hash_ops); - if (r < 0) - return r; - - return set_put_strdup(d->jobs, path); + return set_put_strdup(&d->jobs, path); } int bus_wait_for_jobs_one(BusWaitForJobs *d, const char *path, bool quiet) { diff --git a/src/shared/logs-show.c b/src/shared/logs-show.c index 6737cda1d2..eade4dfbfd 100644 --- a/src/shared/logs-show.c +++ b/src/shared/logs-show.c @@ -1133,30 +1133,25 @@ int show_journal_entry( const size_t highlight[2], bool *ellipsized) { - int ret; - _cleanup_set_free_free_ Set *fields = NULL; + _cleanup_set_free_ Set *fields = NULL; + int r; + assert(mode >= 0); assert(mode < _OUTPUT_MODE_MAX); if (n_columns <= 0) n_columns = columns(); - if (output_fields) { - fields = set_new(&string_hash_ops); - if (!fields) - return log_oom(); + r = set_put_strdupv(&fields, output_fields); + if (r < 0) + return r; - ret = set_put_strdupv(fields, output_fields); - if (ret < 0) - return ret; - } + r = output_funcs[mode](f, j, mode, n_columns, flags, fields, highlight); - ret = output_funcs[mode](f, j, mode, n_columns, flags, fields, highlight); - - if (ellipsized && ret > 0) + if (ellipsized && r > 0) *ellipsized = true; - return ret; + return r; } static int maybe_print_begin_newline(FILE *f, OutputFlags *flags) { diff --git a/src/shared/mount-util.c b/src/shared/mount-util.c index ae6ff9108a..f3ee656c0f 100644 --- a/src/shared/mount-util.c +++ b/src/shared/mount-util.c @@ -239,7 +239,7 @@ int bind_remount_recursive_with_mountinfo( } if (!set_contains(done, path)) { - r = set_put_strdup(todo, path); + r = set_put_strdup(&todo, path); if (r < 0) return r; } @@ -266,7 +266,7 @@ int bind_remount_recursive_with_mountinfo( log_debug("Made top-level directory %s a mount point.", prefix); - r = set_put_strdup(done, simplified); + r = set_put_strdup(&done, simplified); if (r < 0) return r; } diff --git a/src/shared/unit-file.c b/src/shared/unit-file.c index 7b64bbf7f1..4fe2489c55 100644 --- a/src/shared/unit-file.c +++ b/src/shared/unit-file.c @@ -463,7 +463,7 @@ int unit_file_find_fragment( /* The unit always has its own name if it's not a template. */ if (IN_SET(name_type, UNIT_NAME_PLAIN, UNIT_NAME_INSTANCE)) { - r = set_put_strdup(names, unit_name); + r = set_put_strdup(&names, unit_name); if (r < 0) return r; } @@ -493,7 +493,7 @@ int unit_file_find_fragment( if (!streq(unit_name, *t)) log_debug("%s: %s has alias %s", __func__, unit_name, *t); - r = set_put_strdup(names, *t); + r = set_put_strdup(&names, *t); } if (r < 0) return r; diff --git a/src/udev/udevadm-trigger.c b/src/udev/udevadm-trigger.c index 60c68b5029..39113d2fa2 100644 --- a/src/udev/udevadm-trigger.c +++ b/src/udev/udevadm-trigger.c @@ -23,7 +23,7 @@ static bool arg_verbose = false; static bool arg_dry_run = false; -static int exec_list(sd_device_enumerator *e, const char *action, Set *settle_set) { +static int exec_list(sd_device_enumerator *e, const char *action, Set **settle_set) { sd_device *d; int r, ret = 0; @@ -172,7 +172,7 @@ int trigger_main(int argc, char *argv[], void *userdata) { _cleanup_(sd_device_enumerator_unrefp) sd_device_enumerator *e = NULL; _cleanup_(sd_device_monitor_unrefp) sd_device_monitor *m = NULL; _cleanup_(sd_event_unrefp) sd_event *event = NULL; - _cleanup_set_free_free_ Set *settle_set = NULL; + _cleanup_set_free_ Set *settle_set = NULL; usec_t ping_timeout_usec = 5 * USEC_PER_SEC; bool settle = false, ping = false; int c, r; @@ -342,7 +342,7 @@ int trigger_main(int argc, char *argv[], void *userdata) { } if (settle) { - settle_set = set_new(&string_hash_ops); + settle_set = set_new(&string_hash_ops_free); if (!settle_set) return log_oom(); @@ -377,7 +377,8 @@ int trigger_main(int argc, char *argv[], void *userdata) { default: assert_not_reached("Unknown device type"); } - r = exec_list(e, action, settle_set); + + r = exec_list(e, action, settle ? &settle_set : NULL); if (r < 0) return r; From de747a0008876ea06c9bc73fff0f3d6593a9c399 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 29 Apr 2020 08:50:37 +0200 Subject: [PATCH 2/9] test-set: make test-set not link to libshared and test test_set_put_strdup*() The sets are such basic functionality that it is convenient to be able to build test-set without all the machinery in shared, and to test it without the mempool to validate memory accesses easier. --- src/test/meson.build | 2 +- src/test/test-set.c | 23 +++++++++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/src/test/meson.build b/src/test/meson.build index b133980e45..318dc25906 100644 --- a/src/test/meson.build +++ b/src/test/meson.build @@ -470,7 +470,7 @@ tests += [ '', 'timeout=90'], [['src/test/test-set.c'], - [], + [libbasic], []], [['src/test/test-ordered-set.c'], diff --git a/src/test/test-set.c b/src/test/test-set.c index b4e7a52fd9..9c93685dbc 100644 --- a/src/test/test-set.c +++ b/src/test/test-set.c @@ -3,6 +3,8 @@ #include "set.h" #include "strv.h" +const bool mempool_use_allowed = VALGRIND; + static void test_set_steal_first(void) { _cleanup_set_free_ Set *m = NULL; int seen[3] = {}; @@ -86,11 +88,32 @@ static void test_set_put(void) { assert_se(strv_length(t) == 3); } +static void test_set_put_strdup(void) { + _cleanup_set_free_ Set *m = NULL; + + assert_se(set_put_strdup(&m, "aaa") == 1); + assert_se(set_put_strdup(&m, "aaa") == 0); + assert_se(set_put_strdup(&m, "bbb") == 1); + assert_se(set_put_strdup(&m, "bbb") == 0); + assert_se(set_put_strdup(&m, "aaa") == 0); + assert_se(set_size(m) == 2); +} + +static void test_set_put_strdupv(void) { + _cleanup_set_free_ Set *m = NULL; + + assert_se(set_put_strdupv(&m, STRV_MAKE("aaa", "aaa", "bbb", "bbb", "aaa")) == 2); + assert_se(set_put_strdupv(&m, STRV_MAKE("aaa", "aaa", "bbb", "bbb", "ccc")) == 1); + assert_se(set_size(m) == 3); +} + int main(int argc, const char *argv[]) { test_set_steal_first(); test_set_free_with_destructor(); test_set_free_with_hash_ops(); test_set_put(); + test_set_put_strdup(); + test_set_put_strdupv(); return 0; } From c73bb51364457f049082eb84447755e53892f542 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 29 Apr 2020 09:01:18 +0200 Subject: [PATCH 3/9] sd-device: use string hash ops in device enumerator There should be no functional change, except that when the same string is added more than once, we skip the duplicate entries. --- src/libsystemd/sd-device/device-enumerator.c | 28 +++++--------------- 1 file changed, 6 insertions(+), 22 deletions(-) diff --git a/src/libsystemd/sd-device/device-enumerator.c b/src/libsystemd/sd-device/device-enumerator.c index cd3d75c517..d6ceb2aff3 100644 --- a/src/libsystemd/sd-device/device-enumerator.c +++ b/src/libsystemd/sd-device/device-enumerator.c @@ -71,14 +71,14 @@ static sd_device_enumerator *device_enumerator_free(sd_device_enumerator *enumer sd_device_unref(enumerator->devices[i]); free(enumerator->devices); - set_free_free(enumerator->match_subsystem); - set_free_free(enumerator->nomatch_subsystem); + set_free(enumerator->match_subsystem); + set_free(enumerator->nomatch_subsystem); hashmap_free_free_free(enumerator->match_sysattr); hashmap_free_free_free(enumerator->nomatch_sysattr); hashmap_free_free_free(enumerator->match_property); - set_free_free(enumerator->match_sysname); - set_free_free(enumerator->match_tag); - set_free_free(enumerator->match_parent); + set_free(enumerator->match_sysname); + set_free(enumerator->match_tag); + set_free(enumerator->match_parent); return mfree(enumerator); } @@ -97,10 +97,6 @@ _public_ int sd_device_enumerator_add_match_subsystem(sd_device_enumerator *enum else set = &enumerator->nomatch_subsystem; - r = set_ensure_allocated(set, NULL); - if (r < 0) - return r; - r = set_put_strdup(set, subsystem); if (r < 0) return r; @@ -188,10 +184,6 @@ _public_ int sd_device_enumerator_add_match_sysname(sd_device_enumerator *enumer assert_return(enumerator, -EINVAL); assert_return(sysname, -EINVAL); - r = set_ensure_allocated(&enumerator->match_sysname, NULL); - if (r < 0) - return r; - r = set_put_strdup(&enumerator->match_sysname, sysname); if (r < 0) return r; @@ -207,10 +199,6 @@ _public_ int sd_device_enumerator_add_match_tag(sd_device_enumerator *enumerator assert_return(enumerator, -EINVAL); assert_return(tag, -EINVAL); - r = set_ensure_allocated(&enumerator->match_tag, NULL); - if (r < 0) - return r; - r = set_put_strdup(&enumerator->match_tag, tag); if (r < 0) return r; @@ -224,7 +212,7 @@ static void device_enumerator_clear_match_parent(sd_device_enumerator *enumerato if (!enumerator) return; - set_clear_free(enumerator->match_parent); + set_clear(enumerator->match_parent); } int device_enumerator_add_match_parent_incremental(sd_device_enumerator *enumerator, sd_device *parent) { @@ -238,10 +226,6 @@ int device_enumerator_add_match_parent_incremental(sd_device_enumerator *enumera if (r < 0) return r; - r = set_ensure_allocated(&enumerator->match_parent, NULL); - if (r < 0) - return r; - r = set_put_strdup(&enumerator->match_parent, path); if (r < 0) return r; From 25b3e2a8355e2f92767d042e85d7dbb1d54ad6d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 29 Apr 2020 09:55:28 +0200 Subject: [PATCH 4/9] basic/hashmap: allow NULL values in strdup hashmaps and add test --- src/basic/hashmap.c | 20 ++++++++++----- src/test/test-hashmap.c | 55 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+), 6 deletions(-) diff --git a/src/basic/hashmap.c b/src/basic/hashmap.c index 7abe62fa93..efbe95bb9e 100644 --- a/src/basic/hashmap.c +++ b/src/basic/hashmap.c @@ -1775,22 +1775,30 @@ int hashmap_put_strdup(Hashmap **h, const char *k, const char *v) { return r; _cleanup_free_ char *kdup = NULL, *vdup = NULL; + kdup = strdup(k); - vdup = strdup(v); - if (!kdup || !vdup) + if (!kdup) return -ENOMEM; + if (v) { + vdup = strdup(v); + if (!vdup) + return -ENOMEM; + } + r = hashmap_put(*h, kdup, vdup); if (r < 0) { - if (r == -EEXIST && streq(v, hashmap_get(*h, kdup))) + if (r == -EEXIST && streq_ptr(v, hashmap_get(*h, kdup))) return 0; return r; } - assert(r > 0); /* 0 would mean vdup is already in the hashmap, which cannot be */ - kdup = vdup = NULL; + /* 0 with non-null vdup would mean vdup is already in the hashmap, which cannot be */ + assert(vdup == NULL || r > 0); + if (r > 0) + kdup = vdup = NULL; - return 0; + return r; } int set_put_strdup(Set **s, const char *p) { diff --git a/src/test/test-hashmap.c b/src/test/test-hashmap.c index 1a6e8ffa58..94dbbf1576 100644 --- a/src/test/test-hashmap.c +++ b/src/test/test-hashmap.c @@ -1,6 +1,7 @@ /* SPDX-License-Identifier: LGPL-2.1+ */ #include "hashmap.h" +#include "string-util.h" #include "util.h" unsigned custom_counter = 0; @@ -109,6 +110,58 @@ static void test_iterated_cache(void) { assert_se(iterated_cache_free(c) == NULL); } +static void test_hashmap_put_strdup(void) { + _cleanup_hashmap_free_ Hashmap *m = NULL; + char *s; + + /* We don't have ordered_hashmap_put_strdup() yet. If it is added, + * these tests should be moved to test-hashmap-plain.c. */ + + log_info("/* %s */", __func__); + + assert_se(hashmap_put_strdup(&m, "foo", "bar") == 1); + assert_se(hashmap_put_strdup(&m, "foo", "bar") == 0); + assert_se(hashmap_put_strdup(&m, "foo", "BAR") == -EEXIST); + assert_se(hashmap_put_strdup(&m, "foo", "bar") == 0); + assert_se(hashmap_contains(m, "foo")); + + s = hashmap_get(m, "foo"); + assert_se(streq(s, "bar")); + + assert_se(hashmap_put_strdup(&m, "xxx", "bar") == 1); + assert_se(hashmap_put_strdup(&m, "xxx", "bar") == 0); + assert_se(hashmap_put_strdup(&m, "xxx", "BAR") == -EEXIST); + assert_se(hashmap_put_strdup(&m, "xxx", "bar") == 0); + assert_se(hashmap_contains(m, "xxx")); + + s = hashmap_get(m, "xxx"); + assert_se(streq(s, "bar")); +} + +static void test_hashmap_put_strdup_null(void) { + _cleanup_hashmap_free_ Hashmap *m = NULL; + char *s; + + log_info("/* %s */", __func__); + + assert_se(hashmap_put_strdup(&m, "foo", "bar") == 1); + assert_se(hashmap_put_strdup(&m, "foo", "bar") == 0); + assert_se(hashmap_put_strdup(&m, "foo", NULL) == -EEXIST); + assert_se(hashmap_put_strdup(&m, "foo", "bar") == 0); + assert_se(hashmap_contains(m, "foo")); + + s = hashmap_get(m, "foo"); + assert_se(streq(s, "bar")); + + assert_se(hashmap_put_strdup(&m, "xxx", NULL) == 1); + assert_se(hashmap_put_strdup(&m, "xxx", "bar") == -EEXIST); + assert_se(hashmap_put_strdup(&m, "xxx", NULL) == 0); + assert_se(hashmap_contains(m, "xxx")); + + s = hashmap_get(m, "xxx"); + assert_se(s == NULL); +} + int main(int argc, const char *argv[]) { /* This file tests in test-hashmap-plain.c, and tests in test-hashmap-ordered.c, which is generated * from test-hashmap-plain.c. Hashmap tests should be added to test-hashmap-plain.c, and here only if @@ -127,6 +180,8 @@ int main(int argc, const char *argv[]) { test_trivial_compare_func(); test_string_compare_func(); test_iterated_cache(); + test_hashmap_put_strdup(); + test_hashmap_put_strdup_null(); return 0; } From eb1c1dc029c91750e6255c3fd844b4f4bf238fab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 29 Apr 2020 14:28:56 +0200 Subject: [PATCH 5/9] sd-device: use hashmap_put_strdup() --- src/libsystemd/sd-device/device-enumerator.c | 54 ++++---------------- 1 file changed, 9 insertions(+), 45 deletions(-) diff --git a/src/libsystemd/sd-device/device-enumerator.c b/src/libsystemd/sd-device/device-enumerator.c index d6ceb2aff3..f9b14f2ffb 100644 --- a/src/libsystemd/sd-device/device-enumerator.c +++ b/src/libsystemd/sd-device/device-enumerator.c @@ -73,9 +73,9 @@ static sd_device_enumerator *device_enumerator_free(sd_device_enumerator *enumer free(enumerator->devices); set_free(enumerator->match_subsystem); set_free(enumerator->nomatch_subsystem); - hashmap_free_free_free(enumerator->match_sysattr); - hashmap_free_free_free(enumerator->nomatch_sysattr); - hashmap_free_free_free(enumerator->match_property); + hashmap_free(enumerator->match_sysattr); + hashmap_free(enumerator->nomatch_sysattr); + hashmap_free(enumerator->match_property); set_free(enumerator->match_sysname); set_free(enumerator->match_tag); set_free(enumerator->match_parent); @@ -106,73 +106,37 @@ _public_ int sd_device_enumerator_add_match_subsystem(sd_device_enumerator *enum return 0; } -_public_ int sd_device_enumerator_add_match_sysattr(sd_device_enumerator *enumerator, const char *_sysattr, const char *_value, int match) { - _cleanup_free_ char *sysattr = NULL, *value = NULL; +_public_ int sd_device_enumerator_add_match_sysattr(sd_device_enumerator *enumerator, const char *sysattr, const char *value, int match) { Hashmap **hashmap; int r; assert_return(enumerator, -EINVAL); - assert_return(_sysattr, -EINVAL); + assert_return(sysattr, -EINVAL); if (match) hashmap = &enumerator->match_sysattr; else hashmap = &enumerator->nomatch_sysattr; - r = hashmap_ensure_allocated(hashmap, NULL); + r = hashmap_put_strdup(hashmap, sysattr, value); if (r < 0) return r; - sysattr = strdup(_sysattr); - if (!sysattr) - return -ENOMEM; - - if (_value) { - value = strdup(_value); - if (!value) - return -ENOMEM; - } - - r = hashmap_put(*hashmap, sysattr, value); - if (r < 0) - return r; - - sysattr = NULL; - value = NULL; - enumerator->scan_uptodate = false; return 0; } -_public_ int sd_device_enumerator_add_match_property(sd_device_enumerator *enumerator, const char *_property, const char *_value) { - _cleanup_free_ char *property = NULL, *value = NULL; +_public_ int sd_device_enumerator_add_match_property(sd_device_enumerator *enumerator, const char *property, const char *value) { int r; assert_return(enumerator, -EINVAL); - assert_return(_property, -EINVAL); + assert_return(property, -EINVAL); - r = hashmap_ensure_allocated(&enumerator->match_property, NULL); + r = hashmap_put_strdup(&enumerator->match_property, property, value); if (r < 0) return r; - property = strdup(_property); - if (!property) - return -ENOMEM; - - if (_value) { - value = strdup(_value); - if (!value) - return -ENOMEM; - } - - r = hashmap_put(enumerator->match_property, property, value); - if (r < 0) - return r; - - property = NULL; - value = NULL; - enumerator->scan_uptodate = false; return 0; From 2204f018cd788e7653741f52e601a007f4edb8cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 29 Apr 2020 14:33:51 +0200 Subject: [PATCH 6/9] sd-device: optimize addition of already present matches Our hashmap and set helpers return a different code whenever an entry already exists, so let's use this to avoid unsetting scan_uptodate when not necessary. Thus, the return convention for sd_device_enumerator_add_match_subsystem, sd_device_enumerator_add_match_sysattr, sd_device_enumerator_add_match_property, sd_device_enumerator_add_match_sysname, sd_device_enumerator_add_match_tag, device_enumerator_add_match_parent_incremental, sd_device_enumerator_add_match_parent, sd_device_enumerator_allow_uninitialized, device_enumerator_add_match_is_initialized is that "1" is returned if action was taken, and "0" on noop. --- src/libsystemd/sd-device/device-enumerator.c | 28 ++++++++++---------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/src/libsystemd/sd-device/device-enumerator.c b/src/libsystemd/sd-device/device-enumerator.c index f9b14f2ffb..94352e129c 100644 --- a/src/libsystemd/sd-device/device-enumerator.c +++ b/src/libsystemd/sd-device/device-enumerator.c @@ -98,12 +98,12 @@ _public_ int sd_device_enumerator_add_match_subsystem(sd_device_enumerator *enum set = &enumerator->nomatch_subsystem; r = set_put_strdup(set, subsystem); - if (r < 0) + if (r <= 0) return r; enumerator->scan_uptodate = false; - return 0; + return 1; } _public_ int sd_device_enumerator_add_match_sysattr(sd_device_enumerator *enumerator, const char *sysattr, const char *value, int match) { @@ -119,12 +119,12 @@ _public_ int sd_device_enumerator_add_match_sysattr(sd_device_enumerator *enumer hashmap = &enumerator->nomatch_sysattr; r = hashmap_put_strdup(hashmap, sysattr, value); - if (r < 0) + if (r <= 0) return r; enumerator->scan_uptodate = false; - return 0; + return 1; } _public_ int sd_device_enumerator_add_match_property(sd_device_enumerator *enumerator, const char *property, const char *value) { @@ -134,12 +134,12 @@ _public_ int sd_device_enumerator_add_match_property(sd_device_enumerator *enume assert_return(property, -EINVAL); r = hashmap_put_strdup(&enumerator->match_property, property, value); - if (r < 0) + if (r <= 0) return r; enumerator->scan_uptodate = false; - return 0; + return 1; } _public_ int sd_device_enumerator_add_match_sysname(sd_device_enumerator *enumerator, const char *sysname) { @@ -149,12 +149,12 @@ _public_ int sd_device_enumerator_add_match_sysname(sd_device_enumerator *enumer assert_return(sysname, -EINVAL); r = set_put_strdup(&enumerator->match_sysname, sysname); - if (r < 0) + if (r <= 0) return r; enumerator->scan_uptodate = false; - return 0; + return 1; } _public_ int sd_device_enumerator_add_match_tag(sd_device_enumerator *enumerator, const char *tag) { @@ -164,12 +164,12 @@ _public_ int sd_device_enumerator_add_match_tag(sd_device_enumerator *enumerator assert_return(tag, -EINVAL); r = set_put_strdup(&enumerator->match_tag, tag); - if (r < 0) + if (r <= 0) return r; enumerator->scan_uptodate = false; - return 0; + return 1; } static void device_enumerator_clear_match_parent(sd_device_enumerator *enumerator) { @@ -191,12 +191,12 @@ int device_enumerator_add_match_parent_incremental(sd_device_enumerator *enumera return r; r = set_put_strdup(&enumerator->match_parent, path); - if (r < 0) + if (r <= 0) return r; enumerator->scan_uptodate = false; - return 0; + return 1; } _public_ int sd_device_enumerator_add_match_parent(sd_device_enumerator *enumerator, sd_device *parent) { @@ -211,7 +211,7 @@ _public_ int sd_device_enumerator_allow_uninitialized(sd_device_enumerator *enum enumerator->scan_uptodate = false; - return 0; + return 1; } int device_enumerator_add_match_is_initialized(sd_device_enumerator *enumerator) { @@ -221,7 +221,7 @@ int device_enumerator_add_match_is_initialized(sd_device_enumerator *enumerator) enumerator->scan_uptodate = false; - return 0; + return 1; } static int device_compare(sd_device * const *_a, sd_device * const *_b) { From 476a63e9c0b18c481697060a6be60f1f9655698c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 29 Apr 2020 14:44:25 +0200 Subject: [PATCH 7/9] sd-device: get rid of device_enumerator_clear_match_parent This helper wasn't helping all that much. It seems better to verify args first, and only then start modifying the state. --- src/libsystemd/sd-device/device-enumerator.c | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/src/libsystemd/sd-device/device-enumerator.c b/src/libsystemd/sd-device/device-enumerator.c index 94352e129c..95dfc2f077 100644 --- a/src/libsystemd/sd-device/device-enumerator.c +++ b/src/libsystemd/sd-device/device-enumerator.c @@ -172,19 +172,12 @@ _public_ int sd_device_enumerator_add_match_tag(sd_device_enumerator *enumerator return 1; } -static void device_enumerator_clear_match_parent(sd_device_enumerator *enumerator) { - if (!enumerator) - return; - - set_clear(enumerator->match_parent); -} - int device_enumerator_add_match_parent_incremental(sd_device_enumerator *enumerator, sd_device *parent) { const char *path; int r; - assert_return(enumerator, -EINVAL); - assert_return(parent, -EINVAL); + assert(enumerator); + assert(parent); r = sd_device_get_syspath(parent, &path); if (r < 0) @@ -200,7 +193,11 @@ int device_enumerator_add_match_parent_incremental(sd_device_enumerator *enumera } _public_ int sd_device_enumerator_add_match_parent(sd_device_enumerator *enumerator, sd_device *parent) { - device_enumerator_clear_match_parent(enumerator); + assert_return(enumerator, -EINVAL); + assert_return(parent, -EINVAL); + + set_clear(enumerator->match_parent); + return device_enumerator_add_match_parent_incremental(enumerator, parent); } From 2f063186d5bb2095ae9d6e05ac727c4d83670b81 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 29 Apr 2020 14:47:48 +0200 Subject: [PATCH 8/9] shared/logs-show: constify Set *fields --- src/shared/logs-show.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/shared/logs-show.c b/src/shared/logs-show.c index eade4dfbfd..f7fe3b3220 100644 --- a/src/shared/logs-show.c +++ b/src/shared/logs-show.c @@ -121,8 +121,8 @@ static int parse_fieldv(const void *data, size_t length, const ParseFieldVec *fi return 0; } -static int field_set_test(Set *fields, const char *name, size_t n) { - char *s = NULL; +static int field_set_test(const Set *fields, const char *name, size_t n) { + char *s; if (!fields) return 1; @@ -369,7 +369,7 @@ static int output_short( OutputMode mode, unsigned n_columns, OutputFlags flags, - Set *output_fields, + const Set *output_fields, const size_t highlight[2]) { int r; @@ -533,7 +533,7 @@ static int output_verbose( OutputMode mode, unsigned n_columns, OutputFlags flags, - Set *output_fields, + const Set *output_fields, const size_t highlight[2]) { const void *data; @@ -652,7 +652,7 @@ static int output_export( OutputMode mode, unsigned n_columns, OutputFlags flags, - Set *output_fields, + const Set *output_fields, const size_t highlight[2]) { sd_id128_t boot_id; @@ -849,7 +849,7 @@ static int update_json_data( static int update_json_data_split( Hashmap *h, OutputFlags flags, - Set *output_fields, + const Set *output_fields, const void *data, size_t size) { @@ -870,7 +870,7 @@ static int update_json_data_split( return 0; name = strndupa(data, eq - (const char*) data); - if (output_fields && !set_get(output_fields, name)) + if (output_fields && !set_contains(output_fields, name)) return 0; return update_json_data(h, flags, name, eq + 1, size - (eq - (const char*) data) - 1); @@ -882,7 +882,7 @@ static int output_json( OutputMode mode, unsigned n_columns, OutputFlags flags, - Set *output_fields, + const Set *output_fields, const size_t highlight[2]) { char sid[SD_ID128_STRING_MAX], usecbuf[DECIMAL_STR_MAX(usec_t)]; @@ -1073,7 +1073,7 @@ static int output_cat( OutputMode mode, unsigned n_columns, OutputFlags flags, - Set *output_fields, + const Set *output_fields, const size_t highlight[2]) { const char *field; @@ -1103,7 +1103,7 @@ static int (*output_funcs[_OUTPUT_MODE_MAX])( OutputMode mode, unsigned n_columns, OutputFlags flags, - Set *output_fields, + const Set *output_fields, const size_t highlight[2]) = { [OUTPUT_SHORT] = output_short, From e57ac1b017e8f52be6d6066e6dc61e80404aa9df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 29 Apr 2020 14:53:26 +0200 Subject: [PATCH 9/9] tree-wide: use _cleanup_set_free_ where appropriate If we already have the helper defined, let's use it instead of open-coding. --- src/core/dbus-unit.c | 2 +- src/test/test-prioq.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core/dbus-unit.c b/src/core/dbus-unit.c index 75e9060649..dedc395666 100644 --- a/src/core/dbus-unit.c +++ b/src/core/dbus-unit.c @@ -1724,7 +1724,7 @@ int bus_unit_queue_job( _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; _cleanup_free_ char *job_path = NULL, *unit_path = NULL; - _cleanup_(set_freep) Set *affected = NULL; + _cleanup_set_free_ Set *affected = NULL; Iterator i; Job *j, *a; int r; diff --git a/src/test/test-prioq.c b/src/test/test-prioq.c index 21d5b44fbb..50f66cb970 100644 --- a/src/test/test-prioq.c +++ b/src/test/test-prioq.c @@ -60,7 +60,7 @@ DEFINE_PRIVATE_HASH_OPS(test_hash_ops, struct test, test_hash, test_compare); static void test_struct(void) { _cleanup_(prioq_freep) Prioq *q = NULL; - _cleanup_(set_freep) Set *s = NULL; + _cleanup_set_free_ Set *s = NULL; unsigned previous = 0, i; struct test *t;