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.
This commit is contained in:
Zbigniew Jędrzejewski-Szmek 2020-04-29 08:47:51 +02:00
parent 0894f08bf1
commit be32732168
25 changed files with 71 additions and 108 deletions

View File

@ -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;
}

View File

@ -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);

View File

@ -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;

View File

@ -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);

View File

@ -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);

View File

@ -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;

View File

@ -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) \

View File

@ -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();

View File

@ -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();
}

View File

@ -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;
}

View File

@ -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;

View File

@ -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;
}

View File

@ -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;
}

View File

@ -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;
}

View File

@ -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;

View File

@ -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);
}

View File

@ -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;
}

View File

@ -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;
}

View File

@ -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;

View File

@ -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();
}

View File

@ -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) {

View File

@ -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) {

View File

@ -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;
}

View File

@ -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;

View File

@ -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;