diff options
author | Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl> | 2020-06-24 17:42:13 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2020-06-24 17:42:13 +0200 |
commit | f83803a6499cd7ad314ba35be2f5f69ae12ab0e9 (patch) | |
tree | ba34799ac9e404113b92cf36023e607be53db3a2 | |
parent | log: introduce log_parse_environment_cli() and log_setup_cli() (diff) | |
parent | sysv-generator: reduce scope of variables (diff) | |
download | systemd-f83803a6499cd7ad314ba35be2f5f69ae12ab0e9.tar.xz systemd-f83803a6499cd7ad314ba35be2f5f69ae12ab0e9.zip |
Merge pull request #16238 from keszybz/set-handling-more
Fix handling of cases where a duplicate item is added to a set and related cleanups
46 files changed, 276 insertions, 504 deletions
diff --git a/coccinelle/set_ensure_put.cocci b/coccinelle/set_ensure_put.cocci new file mode 100644 index 0000000000..92d7970ade --- /dev/null +++ b/coccinelle/set_ensure_put.cocci @@ -0,0 +1,18 @@ +@@ +local idexpression r; +expression p, k, x; +@@ +- r = set_ensure_allocated(&p, k); +- if (r < 0) +- return ...; +- r = set_put(p, x); ++ r = set_ensure_put(&p, k, x); +@@ +local idexpression r; +expression p, k, x; +@@ +- r = set_ensure_allocated(p, k); +- if (r < 0) +- return ...; +- r = set_put(*p, x); ++ r = set_ensure_put(p, k, x); diff --git a/src/basic/hashmap.c b/src/basic/hashmap.c index 15c8c4723c..67c4391230 100644 --- a/src/basic/hashmap.c +++ b/src/basic/hashmap.c @@ -768,7 +768,7 @@ static void reset_direct_storage(HashmapBase *h) { memset(p, DIB_RAW_INIT, sizeof(dib_raw_t) * hi->n_direct_buckets); } -static struct HashmapBase *hashmap_base_new(const struct hash_ops *hash_ops, enum HashmapType type HASHMAP_DEBUG_PARAMS) { +static struct HashmapBase *hashmap_base_new(const struct hash_ops *hash_ops, enum HashmapType type HASHMAP_DEBUG_PARAMS) { HashmapBase *h; const struct hashmap_type_info *hi = &hashmap_type_info[type]; bool up; @@ -808,19 +808,19 @@ static struct HashmapBase *hashmap_base_new(const struct hash_ops *hash_ops, enu } Hashmap *_hashmap_new(const struct hash_ops *hash_ops HASHMAP_DEBUG_PARAMS) { - return (Hashmap*) hashmap_base_new(hash_ops, HASHMAP_TYPE_PLAIN HASHMAP_DEBUG_PASS_ARGS); + return (Hashmap*) hashmap_base_new(hash_ops, HASHMAP_TYPE_PLAIN HASHMAP_DEBUG_PASS_ARGS); } OrderedHashmap *_ordered_hashmap_new(const struct hash_ops *hash_ops HASHMAP_DEBUG_PARAMS) { - return (OrderedHashmap*) hashmap_base_new(hash_ops, HASHMAP_TYPE_ORDERED HASHMAP_DEBUG_PASS_ARGS); + return (OrderedHashmap*) hashmap_base_new(hash_ops, HASHMAP_TYPE_ORDERED HASHMAP_DEBUG_PASS_ARGS); } Set *_set_new(const struct hash_ops *hash_ops HASHMAP_DEBUG_PARAMS) { - return (Set*) hashmap_base_new(hash_ops, HASHMAP_TYPE_SET HASHMAP_DEBUG_PASS_ARGS); + return (Set*) hashmap_base_new(hash_ops, HASHMAP_TYPE_SET HASHMAP_DEBUG_PASS_ARGS); } static int hashmap_base_ensure_allocated(HashmapBase **h, const struct hash_ops *hash_ops, - enum HashmapType type HASHMAP_DEBUG_PARAMS) { + enum HashmapType type HASHMAP_DEBUG_PARAMS) { HashmapBase *q; assert(h); @@ -828,7 +828,7 @@ static int hashmap_base_ensure_allocated(HashmapBase **h, const struct hash_ops if (*h) return 0; - q = hashmap_base_new(hash_ops, type HASHMAP_DEBUG_PASS_ARGS); + q = hashmap_base_new(hash_ops, type HASHMAP_DEBUG_PASS_ARGS); if (!q) return -ENOMEM; @@ -837,15 +837,15 @@ static int hashmap_base_ensure_allocated(HashmapBase **h, const struct hash_ops } int _hashmap_ensure_allocated(Hashmap **h, const struct hash_ops *hash_ops HASHMAP_DEBUG_PARAMS) { - return hashmap_base_ensure_allocated((HashmapBase**)h, hash_ops, HASHMAP_TYPE_PLAIN HASHMAP_DEBUG_PASS_ARGS); + return hashmap_base_ensure_allocated((HashmapBase**)h, hash_ops, HASHMAP_TYPE_PLAIN HASHMAP_DEBUG_PASS_ARGS); } int _ordered_hashmap_ensure_allocated(OrderedHashmap **h, const struct hash_ops *hash_ops HASHMAP_DEBUG_PARAMS) { - return hashmap_base_ensure_allocated((HashmapBase**)h, hash_ops, HASHMAP_TYPE_ORDERED HASHMAP_DEBUG_PASS_ARGS); + return hashmap_base_ensure_allocated((HashmapBase**)h, hash_ops, HASHMAP_TYPE_ORDERED HASHMAP_DEBUG_PASS_ARGS); } int _set_ensure_allocated(Set **s, const struct hash_ops *hash_ops HASHMAP_DEBUG_PARAMS) { - return hashmap_base_ensure_allocated((HashmapBase**)s, hash_ops, HASHMAP_TYPE_SET HASHMAP_DEBUG_PASS_ARGS); + return hashmap_base_ensure_allocated((HashmapBase**)s, hash_ops, HASHMAP_TYPE_SET HASHMAP_DEBUG_PASS_ARGS); } static void hashmap_free_no_clear(HashmapBase *h) { @@ -1247,6 +1247,30 @@ int set_put(Set *s, const void *key) { return hashmap_put_boldly(s, hash, &swap, true); } +int _set_ensure_put(Set **s, const struct hash_ops *hash_ops, const void *key HASHMAP_DEBUG_PARAMS) { + int r; + + r = _set_ensure_allocated(s, hash_ops HASHMAP_DEBUG_PASS_ARGS); + if (r < 0) + return r; + + return set_put(*s, key); +} + +int _set_ensure_consume(Set **s, const struct hash_ops *hash_ops, void *key HASHMAP_DEBUG_PARAMS) { + int r; + + r = _set_ensure_put(s, hash_ops, key HASHMAP_DEBUG_PASS_ARGS); + if (r <= 0) { + if (hash_ops && hash_ops->free_key) + hash_ops->free_key(key); + else + free(key); + } + + return r; +} + int hashmap_replace(Hashmap *h, const void *key, void *value) { struct swap_entries swap; struct plain_hashmap_entry *e; @@ -1687,13 +1711,13 @@ int _hashmap_move_one(HashmapBase *h, HashmapBase *other, const void *key) { return 0; } -HashmapBase *_hashmap_copy(HashmapBase *h) { +HashmapBase *_hashmap_copy(HashmapBase *h HASHMAP_DEBUG_PARAMS) { HashmapBase *copy; int r; assert(h); - copy = hashmap_base_new(h->hash_ops, h->type HASHMAP_DEBUG_SRC_ARGS); + copy = hashmap_base_new(h->hash_ops, h->type HASHMAP_DEBUG_PASS_ARGS); if (!copy) return NULL; @@ -1709,10 +1733,8 @@ HashmapBase *_hashmap_copy(HashmapBase *h) { assert_not_reached("Unknown hashmap type"); } - if (r < 0) { - _hashmap_free(copy, false, false); - return NULL; - } + if (r < 0) + return _hashmap_free(copy, false, false); return copy; } @@ -1765,10 +1787,10 @@ int set_consume(Set *s, void *value) { return r; } -int hashmap_put_strdup(Hashmap **h, const char *k, const char *v) { +int _hashmap_put_strdup(Hashmap **h, const char *k, const char *v HASHMAP_DEBUG_PARAMS) { int r; - r = hashmap_ensure_allocated(h, &string_hash_ops_free_free); + r = _hashmap_ensure_allocated(h, &string_hash_ops_free_free HASHMAP_DEBUG_PASS_ARGS); if (r < 0) return r; @@ -1799,14 +1821,14 @@ int hashmap_put_strdup(Hashmap **h, const char *k, const char *v) { return r; } -int set_put_strdup(Set **s, const char *p) { +int _set_put_strdup(Set **s, const char *p HASHMAP_DEBUG_PARAMS) { char *c; int r; assert(s); assert(p); - r = set_ensure_allocated(s, &string_hash_ops_free); + r = _set_ensure_allocated(s, &string_hash_ops_free HASHMAP_DEBUG_PASS_ARGS); if (r < 0) return r; @@ -1820,14 +1842,14 @@ int set_put_strdup(Set **s, const char *p) { return set_consume(*s, c); } -int set_put_strdupv(Set **s, char **l) { +int _set_put_strdupv(Set **s, char **l HASHMAP_DEBUG_PARAMS) { int n = 0, r; char **i; assert(s); STRV_FOREACH(i, l) { - r = set_put_strdup(s, *i); + r = _set_put_strdup(s, *i HASHMAP_DEBUG_PASS_ARGS); if (r < 0) return r; diff --git a/src/basic/hashmap.h b/src/basic/hashmap.h index 230d322213..6009441621 100644 --- a/src/basic/hashmap.h +++ b/src/basic/hashmap.h @@ -128,13 +128,9 @@ static inline OrderedHashmap *ordered_hashmap_free_free_free(OrderedHashmap *h) IteratedCache *iterated_cache_free(IteratedCache *cache); int iterated_cache_get(IteratedCache *cache, const void ***res_keys, const void ***res_values, unsigned *res_n_entries); -HashmapBase *_hashmap_copy(HashmapBase *h); -static inline Hashmap *hashmap_copy(Hashmap *h) { - return (Hashmap*) _hashmap_copy(HASHMAP_BASE(h)); -} -static inline OrderedHashmap *ordered_hashmap_copy(OrderedHashmap *h) { - return (OrderedHashmap*) _hashmap_copy(HASHMAP_BASE(h)); -} +HashmapBase *_hashmap_copy(HashmapBase *h HASHMAP_DEBUG_PARAMS); +#define hashmap_copy(h) ((Hashmap*) _hashmap_copy(HASHMAP_BASE(h) HASHMAP_DEBUG_SRC_ARGS)) +#define ordered_hashmap_copy(h) ((OrderedHashmap*) _hashmap_copy(HASHMAP_BASE(h) HASHMAP_DEBUG_SRC_ARGS)) int _hashmap_ensure_allocated(Hashmap **h, const struct hash_ops *hash_ops HASHMAP_DEBUG_PARAMS); int _ordered_hashmap_ensure_allocated(OrderedHashmap **h, const struct hash_ops *hash_ops HASHMAP_DEBUG_PARAMS); @@ -154,7 +150,8 @@ static inline int ordered_hashmap_put(OrderedHashmap *h, const void *key, void * return hashmap_put(PLAIN_HASHMAP(h), key, value); } -int hashmap_put_strdup(Hashmap **h, const char *k, const char *v); +int _hashmap_put_strdup(Hashmap **h, const char *k, const char *v HASHMAP_DEBUG_PARAMS); +#define hashmap_put_strdup(h, k, v) _hashmap_put_strdup(h, k, v HASHMAP_DEBUG_SRC_ARGS) int hashmap_update(Hashmap *h, const void *key, void *value); static inline int ordered_hashmap_update(OrderedHashmap *h, const void *key, void *value) { diff --git a/src/basic/set.h b/src/basic/set.h index 621e83bf27..e4fc1e3c4a 100644 --- a/src/basic/set.h +++ b/src/basic/set.h @@ -26,9 +26,7 @@ static inline Set *set_free_free(Set *s) { /* no set_free_free_free */ -static inline Set *set_copy(Set *s) { - return (Set*) _hashmap_copy(HASHMAP_BASE(s)); -} +#define set_copy(s) ((Set*) _hashmap_copy(HASHMAP_BASE(h) HASHMAP_DEBUG_SRC_ARGS)) int _set_ensure_allocated(Set **s, const struct hash_ops *hash_ops HASHMAP_DEBUG_PARAMS); #define set_ensure_allocated(h, ops) _set_ensure_allocated(h, ops HASHMAP_DEBUG_SRC_ARGS) @@ -120,9 +118,19 @@ static inline char **set_get_strv(Set *s) { return _hashmap_get_strv(HASHMAP_BASE(s)); } +int _set_ensure_put(Set **s, const struct hash_ops *hash_ops, const void *key HASHMAP_DEBUG_PARAMS); +#define set_ensure_put(s, hash_ops, key) _set_ensure_put(s, hash_ops, key HASHMAP_DEBUG_SRC_ARGS) + +int _set_ensure_consume(Set **s, const struct hash_ops *hash_ops, void *key HASHMAP_DEBUG_PARAMS); +#define set_ensure_consume(s, hash_ops, key) _set_ensure_consume(s, hash_ops, key HASHMAP_DEBUG_SRC_ARGS) + 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 HASHMAP_DEBUG_PARAMS); +#define set_put_strdup(s, p) _set_put_strdup(s, p HASHMAP_DEBUG_SRC_ARGS) +int _set_put_strdupv(Set **s, char **l HASHMAP_DEBUG_PARAMS); +#define set_put_strdupv(s, l) _set_put_strdupv(s, l HASHMAP_DEBUG_SRC_ARGS) + int set_put_strsplit(Set *s, const char *v, const char *separators, ExtractFlags flags); #define SET_FOREACH(e, s, i) \ diff --git a/src/core/automount.c b/src/core/automount.c index 566b56eb34..77420b929f 100644 --- a/src/core/automount.c +++ b/src/core/automount.c @@ -912,13 +912,7 @@ static int automount_deserialize_item(Unit *u, const char *key, const char *valu if (safe_atou(value, &token) < 0) log_unit_debug(u, "Failed to parse token value: %s", value); else { - r = set_ensure_allocated(&a->tokens, NULL); - if (r < 0) { - log_oom(); - return 0; - } - - r = set_put(a->tokens, UINT_TO_PTR(token)); + r = set_ensure_put(&a->tokens, NULL, UINT_TO_PTR(token)); if (r < 0) log_unit_error_errno(u, r, "Failed to add token to set: %m"); } @@ -928,13 +922,7 @@ static int automount_deserialize_item(Unit *u, const char *key, const char *valu if (safe_atou(value, &token) < 0) log_unit_debug(u, "Failed to parse token value: %s", value); else { - r = set_ensure_allocated(&a->expire_tokens, NULL); - if (r < 0) { - log_oom(); - return 0; - } - - r = set_put(a->expire_tokens, UINT_TO_PTR(token)); + r = set_ensure_put(&a->expire_tokens, NULL, UINT_TO_PTR(token)); if (r < 0) log_unit_error_errno(u, r, "Failed to add expire token to set: %m"); } @@ -1010,13 +998,7 @@ static int automount_dispatch_io(sd_event_source *s, int fd, uint32_t events, vo } else log_unit_debug(UNIT(a), "Got direct mount request on %s", a->where); - r = set_ensure_allocated(&a->tokens, NULL); - if (r < 0) { - log_unit_error(UNIT(a), "Failed to allocate token set."); - goto fail; - } - - r = set_put(a->tokens, UINT_TO_PTR(packet.v5_packet.wait_queue_token)); + r = set_ensure_put(&a->tokens, NULL, UINT_TO_PTR(packet.v5_packet.wait_queue_token)); if (r < 0) { log_unit_error_errno(UNIT(a), r, "Failed to remember token: %m"); goto fail; @@ -1030,13 +1012,7 @@ static int automount_dispatch_io(sd_event_source *s, int fd, uint32_t events, vo automount_stop_expire(a); - r = set_ensure_allocated(&a->expire_tokens, NULL); - if (r < 0) { - log_unit_error(UNIT(a), "Failed to allocate token set."); - goto fail; - } - - r = set_put(a->expire_tokens, UINT_TO_PTR(packet.v5_packet.wait_queue_token)); + r = set_ensure_put(&a->expire_tokens, NULL, UINT_TO_PTR(packet.v5_packet.wait_queue_token)); if (r < 0) { log_unit_error_errno(UNIT(a), r, "Failed to remember token: %m"); goto fail; diff --git a/src/core/bpf-firewall.c b/src/core/bpf-firewall.c index 96c1a28b4f..2ec274df01 100644 --- a/src/core/bpf-firewall.c +++ b/src/core/bpf-firewall.c @@ -595,7 +595,7 @@ static int load_bpf_progs_from_fs_to_set(Unit *u, char **filter_paths, Set **set set_clear(*set); STRV_FOREACH(bpf_fs_path, filter_paths) { - _cleanup_free_ BPFProgram *prog = NULL; + _cleanup_(bpf_program_unrefp) BPFProgram *prog = NULL; int r; r = bpf_program_new(BPF_PROG_TYPE_CGROUP_SKB, &prog); @@ -606,14 +606,9 @@ static int load_bpf_progs_from_fs_to_set(Unit *u, char **filter_paths, Set **set if (r < 0) return log_unit_error_errno(u, r, "Loading of ingress BPF program %s failed: %m", *bpf_fs_path); - r = set_ensure_allocated(set, &filter_prog_hash_ops); - if (r < 0) - return log_unit_error_errno(u, r, "Can't allocate BPF program set: %m"); - - r = set_put(*set, prog); + r = set_ensure_consume(set, &filter_prog_hash_ops, TAKE_PTR(prog)); if (r < 0) return log_unit_error_errno(u, r, "Can't add program to BPF program set: %m"); - TAKE_PTR(prog); } return 0; @@ -662,12 +657,9 @@ static int attach_custom_bpf_progs(Unit *u, const char *path, int attach_type, S r = bpf_program_cgroup_attach(prog, attach_type, path, BPF_F_ALLOW_MULTI); if (r < 0) return log_unit_error_errno(u, r, "Attaching custom egress BPF program to cgroup %s failed: %m", path); - /* Remember that these BPF programs are installed now. */ - r = set_ensure_allocated(set_installed, &filter_prog_hash_ops); - if (r < 0) - return log_unit_error_errno(u, r, "Can't allocate BPF program set: %m"); - r = set_put(*set_installed, prog); + /* Remember that these BPF programs are installed now. */ + r = set_ensure_put(set_installed, &filter_prog_hash_ops, prog); if (r < 0) return log_unit_error_errno(u, r, "Can't add program to BPF program set: %m"); bpf_program_ref(prog); diff --git a/src/core/dbus-execute.c b/src/core/dbus-execute.c index a584895ea9..68ce395c56 100644 --- a/src/core/dbus-execute.c +++ b/src/core/dbus-execute.c @@ -1768,10 +1768,6 @@ int bus_exec_context_set_transient_property( else { char **s; - r = set_ensure_allocated(&c->syscall_archs, NULL); - if (r < 0) - return r; - STRV_FOREACH(s, l) { uint32_t a; @@ -1779,7 +1775,7 @@ int bus_exec_context_set_transient_property( if (r < 0) return r; - r = set_put(c->syscall_archs, UINT32_TO_PTR(a + 1)); + r = set_ensure_put(&c->syscall_archs, NULL, UINT32_TO_PTR(a + 1)); if (r < 0) return r; } diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c index 75759e887b..c3186f3824 100644 --- a/src/core/load-fragment.c +++ b/src/core/load-fragment.c @@ -3102,10 +3102,6 @@ int config_parse_syscall_archs( return 0; } - r = set_ensure_allocated(archs, NULL); - if (r < 0) - return log_oom(); - for (;;) { _cleanup_free_ char *word = NULL; uint32_t a; @@ -3128,7 +3124,7 @@ int config_parse_syscall_archs( continue; } - r = set_put(*archs, UINT32_TO_PTR(a + 1)); + r = set_ensure_put(archs, NULL, UINT32_TO_PTR(a + 1)); if (r < 0) return log_oom(); } diff --git a/src/core/manager.c b/src/core/manager.c index 959181d204..ab3d0b1192 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -4474,12 +4474,9 @@ int manager_update_failed_units(Manager *m, Unit *u, bool failed) { size = set_size(m->failed_units); if (failed) { - r = set_ensure_allocated(&m->failed_units, NULL); + r = set_ensure_put(&m->failed_units, NULL, u); if (r < 0) return log_oom(); - - if (set_put(m->failed_units, u) < 0) - return log_oom(); } else (void) set_remove(m->failed_units, u); diff --git a/src/core/unit.c b/src/core/unit.c index e50080cd97..8e59fafc11 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -1591,7 +1591,6 @@ static int unit_add_mount_dependencies(Unit *u) { static int unit_add_startup_units(Unit *u) { CGroupContext *c; - int r; c = unit_get_cgroup_context(u); if (!c) @@ -1602,11 +1601,7 @@ static int unit_add_startup_units(Unit *u) { c->startup_blockio_weight == CGROUP_BLKIO_WEIGHT_INVALID) return 0; - r = set_ensure_allocated(&u->manager->startup_units, NULL); - if (r < 0) - return r; - - return set_put(u->manager->startup_units, u); + return set_ensure_put(&u->manager->startup_units, NULL, u); } int unit_load(Unit *u) { diff --git a/src/journal/journalctl.c b/src/journal/journalctl.c index 6ba65a1071..8d4897b942 100644 --- a/src/journal/journalctl.c +++ b/src/journal/journalctl.c @@ -875,12 +875,7 @@ static int parse_argv(int argc, char *argv[]) { return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Bad --facility= argument \"%s\".", fac); - r = set_ensure_allocated(&arg_facilities, NULL); - if (r < 0) - return log_oom(); - - r = set_put(arg_facilities, INT_TO_PTR(num)); - if (r < 0) + if (set_ensure_put(&arg_facilities, NULL, INT_TO_PTR(num)) < 0) return log_oom(); } diff --git a/src/libsystemd/sd-device/device-monitor.c b/src/libsystemd/sd-device/device-monitor.c index ee8005833d..ffec11bbd3 100644 --- a/src/libsystemd/sd-device/device-monitor.c +++ b/src/libsystemd/sd-device/device-monitor.c @@ -759,30 +759,13 @@ _public_ int sd_device_monitor_filter_add_match_subsystem_devtype(sd_device_moni } _public_ int sd_device_monitor_filter_add_match_tag(sd_device_monitor *m, const char *tag) { - _cleanup_free_ char *t = NULL; - int r; - assert_return(m, -EINVAL); assert_return(tag, -EINVAL); - t = strdup(tag); - if (!t) - return -ENOMEM; - - r = set_ensure_allocated(&m->tag_filter, &string_hash_ops); - if (r < 0) - return r; - - r = set_put(m->tag_filter, t); - if (r == -EEXIST) - return 0; - if (r < 0) - return r; - - TAKE_PTR(t); - m->filter_uptodate = false; - - return 0; + int r = set_put_strdup(&m->tag_filter, tag); + if (r > 0) + m->filter_uptodate = false; + return r; } _public_ int sd_device_monitor_filter_remove(sd_device_monitor *m) { diff --git a/src/libsystemd/sd-event/sd-event.c b/src/libsystemd/sd-event/sd-event.c index fb9db47105..860eb048ff 100644 --- a/src/libsystemd/sd-event/sd-event.c +++ b/src/libsystemd/sd-event/sd-event.c @@ -1450,10 +1450,6 @@ _public_ int sd_event_add_post( assert_return(e->state != SD_EVENT_FINISHED, -ESTALE); assert_return(!event_pid_changed(e), -ECHILD); - r = set_ensure_allocated(&e->post_sources, NULL); - if (r < 0) - return r; - s = source_new(e, !ret, SOURCE_POST); if (!s) return -ENOMEM; @@ -1462,9 +1458,10 @@ _public_ int sd_event_add_post( s->userdata = userdata; s->enabled = SD_EVENT_ON; - r = set_put(e->post_sources, s); + r = set_ensure_put(&e->post_sources, NULL, s); if (r < 0) return r; + assert(r > 0); if (ret) *ret = s; diff --git a/src/login/logind-brightness.c b/src/login/logind-brightness.c index 3f4b65e1fd..450ec32044 100644 --- a/src/login/logind-brightness.c +++ b/src/login/logind-brightness.c @@ -174,15 +174,11 @@ static int set_add_message(Set **set, sd_bus_message *message) { if (r <= 0) return r; - r = set_ensure_allocated(set, &bus_message_hash_ops); - if (r < 0) - return r; - - r = set_put(*set, message); - if (r < 0) + r = set_ensure_put(set, &bus_message_hash_ops, message); + if (r <= 0) return r; - sd_bus_message_ref(message); + return 1; } diff --git a/src/network/netdev/wireguard.c b/src/network/netdev/wireguard.c index 713cdaa884..bff778179c 100644 --- a/src/network/netdev/wireguard.c +++ b/src/network/netdev/wireguard.c @@ -348,14 +348,7 @@ static int wireguard_resolve_handler(sd_resolve_query *q, if (ret != 0) { log_netdev_error(netdev, "Failed to resolve host '%s:%s': %s", peer->endpoint_host, peer->endpoint_port, gai_strerror(ret)); - r = set_ensure_allocated(&w->peers_with_failed_endpoint, NULL); - if (r < 0) { - log_oom(); - peer->section->invalid = true; - goto resolve_next; - } - - r = set_put(w->peers_with_failed_endpoint, peer); + r = set_ensure_put(&w->peers_with_failed_endpoint, NULL, peer); if (r < 0) { log_netdev_error(netdev, "Failed to save a peer, dropping the peer: %m"); peer->section->invalid = true; @@ -580,7 +573,7 @@ int config_parse_wireguard_preshared_key( void *data, void *userdata) { - _cleanup_(wireguard_peer_free_or_set_invalidp) WireguardPeer *peer = NULL; + WireguardPeer *peer; Wireguard *w; int r; @@ -592,12 +585,7 @@ int config_parse_wireguard_preshared_key( if (r < 0) return r; - r = wireguard_decode_key_and_warn(rvalue, peer->preshared_key, unit, filename, line, lvalue); - if (r < 0) - return r; - - TAKE_PTR(peer); - return 0; + return wireguard_decode_key_and_warn(rvalue, peer->preshared_key, unit, filename, line, lvalue); } int config_parse_wireguard_preshared_key_file( @@ -766,10 +754,6 @@ int config_parse_wireguard_endpoint( w = WIREGUARD(data); assert(w); - r = wireguard_peer_new_static(w, filename, section_line, &peer); - if (r < 0) - return r; - if (rvalue[0] == '[') { begin = &rvalue[1]; end = strchr(rvalue, ']'); @@ -801,6 +785,10 @@ int config_parse_wireguard_endpoint( ++end; } + r = wireguard_peer_new_static(w, filename, section_line, &peer); + if (r < 0) + return r; + r = free_and_strndup(&peer->endpoint_host, begin, len); if (r < 0) return log_oom(); @@ -809,15 +797,11 @@ int config_parse_wireguard_endpoint( if (r < 0) return log_oom(); - r = set_ensure_allocated(&w->peers_with_unresolved_endpoint, NULL); + r = set_ensure_put(&w->peers_with_unresolved_endpoint, NULL, peer); if (r < 0) return log_oom(); + TAKE_PTR(peer); /* The peer may already have been in the hash map, that is fine too. */ - r = set_put(w->peers_with_unresolved_endpoint, peer); - if (r < 0) - return r; - - TAKE_PTR(peer); return 0; } @@ -833,7 +817,7 @@ int config_parse_wireguard_keepalive( void *data, void *userdata) { - _cleanup_(wireguard_peer_free_or_set_invalidp) WireguardPeer *peer = NULL; + WireguardPeer *peer; uint16_t keepalive = 0; Wireguard *w; int r; @@ -854,15 +838,13 @@ int config_parse_wireguard_keepalive( r = safe_atou16(rvalue, &keepalive); if (r < 0) { log_syntax(unit, LOG_ERR, filename, line, r, - "The persistent keepalive interval must be 0-65535. Ignore assignment: %s", + "Failed to parse \"%s\" as keepalive interval (range 0–65535), ignoring assignment: %m", rvalue); return 0; } } peer->persistent_keepalive_interval = keepalive; - - TAKE_PTR(peer); return 0; } diff --git a/src/network/networkd-address.c b/src/network/networkd-address.c index 9b78530334..44e317e990 100644 --- a/src/network/networkd-address.c +++ b/src/network/networkd-address.c @@ -266,11 +266,7 @@ static int address_add_internal(Link *link, Set **addresses, /* Consider address tentative until we get the real flags from the kernel */ address->flags = IFA_F_TENTATIVE; - r = set_ensure_allocated(addresses, &address_hash_ops); - if (r < 0) - return r; - - r = set_put(*addresses, address); + r = set_ensure_put(addresses, &address_hash_ops, address); if (r < 0) return r; if (r == 0) @@ -280,9 +276,7 @@ static int address_add_internal(Link *link, Set **addresses, if (ret) *ret = address; - - address = NULL; - + TAKE_PTR(address); return 0; } @@ -302,11 +296,7 @@ int address_add(Link *link, int family, const union in_addr_union *in_addr, unsi return r; } else if (r == 0) { /* Take over a foreign address */ - r = set_ensure_allocated(&link->addresses, &address_hash_ops); - if (r < 0) - return r; - - r = set_put(link->addresses, address); + r = set_ensure_put(&link->addresses, &address_hash_ops, address); if (r < 0) return r; diff --git a/src/network/networkd-dhcp-common.c b/src/network/networkd-dhcp-common.c index 606ae9f3c2..5b1acde297 100644 --- a/src/network/networkd-dhcp-common.c +++ b/src/network/networkd-dhcp-common.c @@ -664,17 +664,8 @@ int config_parse_dhcp_request_options( continue; } - if (ltype == AF_INET) - r = set_ensure_allocated(&network->dhcp_request_options, NULL); - else - r = set_ensure_allocated(&network->dhcp6_request_options, NULL); - if (r < 0) - return log_oom(); - - if (ltype == AF_INET) - r = set_put(network->dhcp_request_options, UINT32_TO_PTR(i)); - else - r = set_put(network->dhcp6_request_options, UINT32_TO_PTR(i)); + r = set_ensure_put(ltype == AF_INET ? &network->dhcp_request_options : &network->dhcp6_request_options, + NULL, UINT32_TO_PTR(i)); if (r < 0) log_syntax(unit, LOG_ERR, filename, line, r, "Failed to store DHCP request option '%s', ignoring assignment: %m", n); diff --git a/src/network/networkd-dhcp4.c b/src/network/networkd-dhcp4.c index 8e2c775fcf..6103792771 100644 --- a/src/network/networkd-dhcp4.c +++ b/src/network/networkd-dhcp4.c @@ -1564,7 +1564,6 @@ int config_parse_dhcp_black_listed_ip_address( void *userdata) { Network *network = data; - const char *p; int r; assert(filename); @@ -1577,7 +1576,7 @@ int config_parse_dhcp_black_listed_ip_address( return 0; } - for (p = rvalue;;) { + for (const char *p = rvalue;;) { _cleanup_free_ char *n = NULL; union in_addr_union ip; @@ -1598,11 +1597,7 @@ int config_parse_dhcp_black_listed_ip_address( continue; } - r = set_ensure_allocated(&network->dhcp_black_listed_ip, NULL); - if (r < 0) - return log_oom(); - - r = set_put(network->dhcp_black_listed_ip, UINT32_TO_PTR(ip.in.s_addr)); + r = set_ensure_put(&network->dhcp_black_listed_ip, NULL, UINT32_TO_PTR(ip.in.s_addr)); if (r < 0) log_syntax(unit, LOG_ERR, filename, line, r, "Failed to store DHCP black listed ip address '%s', ignoring assignment: %m", n); diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c index 94806be5e6..06a467d15b 100644 --- a/src/network/networkd-link.c +++ b/src/network/networkd-link.c @@ -2125,11 +2125,7 @@ static int link_append_to_master(Link *link, NetDev *netdev) { if (r < 0) return r; - r = set_ensure_allocated(&master->slaves, NULL); - if (r < 0) - return r; - - r = set_put(master->slaves, link); + r = set_ensure_put(&master->slaves, NULL, link); if (r <= 0) return r; @@ -3133,12 +3129,12 @@ static int link_configure_duid(Link *link) { r = set_put(m->links_requesting_uuid, link); if (r < 0) return log_oom(); + if (r > 0) + link_ref(link); r = set_put(m->duids_requesting_uuid, duid); if (r < 0) return log_oom(); - - link_ref(link); } return 0; @@ -4413,16 +4409,10 @@ void link_dirty(Link *link) { /* mark manager dirty as link is dirty */ manager_dirty(link->manager); - r = set_ensure_allocated(&link->manager->dirty_links, NULL); - if (r < 0) - /* allocation errors are ignored */ - return; - - r = set_put(link->manager->dirty_links, link); + r = set_ensure_put(&link->manager->dirty_links, NULL, link); if (r <= 0) - /* don't take another ref if the link was already dirty */ + /* Ignore allocation errors and don't take another ref if the link was already dirty */ return; - link_ref(link); } diff --git a/src/network/networkd-manager.c b/src/network/networkd-manager.c index 8692b9411e..08666fd92d 100644 --- a/src/network/networkd-manager.c +++ b/src/network/networkd-manager.c @@ -2309,23 +2309,15 @@ int manager_request_product_uuid(Manager *m, Link *link) { assert_se(duid = link_get_duid(link)); - r = set_ensure_allocated(&m->links_requesting_uuid, NULL); + r = set_ensure_put(&m->links_requesting_uuid, NULL, link); if (r < 0) return log_oom(); + if (r > 0) + link_ref(link); - r = set_ensure_allocated(&m->duids_requesting_uuid, NULL); + r = set_ensure_put(&m->duids_requesting_uuid, NULL, duid); if (r < 0) return log_oom(); - - r = set_put(m->links_requesting_uuid, link); - if (r < 0) - return log_oom(); - - r = set_put(m->duids_requesting_uuid, duid); - if (r < 0) - return log_oom(); - - link_ref(link); } if (!m->bus || sd_bus_is_ready(m->bus) <= 0) { diff --git a/src/network/networkd-ndisc.c b/src/network/networkd-ndisc.c index 19a655b2d7..7889bfb191 100644 --- a/src/network/networkd-ndisc.c +++ b/src/network/networkd-ndisc.c @@ -591,10 +591,6 @@ static int ndisc_router_process_rdnss(Link *link, sd_ndisc_router *rt) { continue; } - r = set_ensure_allocated(&link->ndisc_rdnss, &ndisc_rdnss_hash_ops); - if (r < 0) - return log_oom(); - x = new(NDiscRDNSS, 1); if (!x) return log_oom(); @@ -604,13 +600,11 @@ static int ndisc_router_process_rdnss(Link *link, sd_ndisc_router *rt) { .valid_until = time_now + lifetime * USEC_PER_SEC, }; - r = set_put(link->ndisc_rdnss, x); + r = set_ensure_consume(&link->ndisc_rdnss, &ndisc_rdnss_hash_ops, TAKE_PTR(x)); if (r < 0) return log_oom(); - - TAKE_PTR(x); - assert(r > 0); + link_dirty(link); } @@ -686,22 +680,15 @@ static void ndisc_router_process_dnssl(Link *link, sd_ndisc_router *rt) { continue; } - r = set_ensure_allocated(&link->ndisc_dnssl, &ndisc_dnssl_hash_ops); - if (r < 0) { - log_oom(); - return; - } - s->valid_until = time_now + lifetime * USEC_PER_SEC; - r = set_put(link->ndisc_dnssl, s); + r = set_ensure_consume(&link->ndisc_dnssl, &ndisc_dnssl_hash_ops, TAKE_PTR(s)); if (r < 0) { log_oom(); return; } - - s = NULL; assert(r > 0); + link_dirty(link); } } @@ -979,22 +966,13 @@ int config_parse_ndisc_black_listed_prefix( if (set_contains(network->ndisc_black_listed_prefix, &ip.in6)) continue; - r = set_ensure_allocated(&network->ndisc_black_listed_prefix, &in6_addr_hash_ops); - if (r < 0) - return log_oom(); - a = newdup(struct in6_addr, &ip.in6, 1); if (!a) return log_oom(); - r = set_put(network->ndisc_black_listed_prefix, a); - if (r < 0) { - log_syntax(unit, LOG_ERR, filename, line, r, - "Failed to store NDISC black listed prefix '%s', ignoring assignment: %m", n); - continue; - } - - TAKE_PTR(a); + r = set_ensure_consume(&network->ndisc_black_listed_prefix, &in6_addr_hash_ops, TAKE_PTR(a)); + if (r < 0) + return log_oom(); } return 0; diff --git a/src/network/networkd-neighbor.c b/src/network/networkd-neighbor.c index fd6219fcce..6d91f70511 100644 --- a/src/network/networkd-neighbor.c +++ b/src/network/networkd-neighbor.c @@ -300,11 +300,7 @@ static int neighbor_add_internal(Link *link, Set **neighbors, int family, const .lladdr_size = lladdr_size, }; - r = set_ensure_allocated(neighbors, &neighbor_hash_ops); - if (r < 0) - return r; - - r = set_put(*neighbors, neighbor); + r = set_ensure_put(neighbors, &neighbor_hash_ops, neighbor); if (r < 0) return r; if (r == 0) @@ -314,8 +310,7 @@ static int neighbor_add_internal(Link *link, Set **neighbors, int family, const if (ret) *ret = neighbor; - - neighbor = NULL; + TAKE_PTR(neighbor); return 0; } @@ -332,11 +327,7 @@ int neighbor_add(Link *link, int family, const union in_addr_union *addr, const return r; } else if (r == 0) { /* Neighbor is foreign, claim it as recognized */ - r = set_ensure_allocated(&link->neighbors, &neighbor_hash_ops); - if (r < 0) - return r; - - r = set_put(link->neighbors, neighbor); + r = set_ensure_put(&link->neighbors, &neighbor_hash_ops, neighbor); if (r < 0) return r; diff --git a/src/network/networkd-network.c b/src/network/networkd-network.c index f0fa5d0427..7fa359a542 100644 --- a/src/network/networkd-network.c +++ b/src/network/networkd-network.c @@ -1268,15 +1268,9 @@ int config_parse_dnssec_negative_trust_anchors( continue; } - r = set_ensure_allocated(&n->dnssec_negative_trust_anchors, &dns_name_hash_ops); + r = set_ensure_consume(&n->dnssec_negative_trust_anchors, &dns_name_hash_ops, TAKE_PTR(w)); if (r < 0) return log_oom(); - - r = set_put(n->dnssec_negative_trust_anchors, w); - if (r < 0) - return log_oom(); - if (r > 0) - w = NULL; } return 0; diff --git a/src/network/networkd-nexthop.c b/src/network/networkd-nexthop.c index 45c13ca88f..5d91d791d1 100644 --- a/src/network/networkd-nexthop.c +++ b/src/network/networkd-nexthop.c @@ -209,11 +209,7 @@ static int nexthop_add_internal(Link *link, Set **nexthops, NextHop *in, NextHop nexthop->family = in->family; nexthop->gw = in->gw; - r = set_ensure_allocated(nexthops, &nexthop_hash_ops); - if (r < 0) - return r; - - r = set_put(*nexthops, nexthop); + r = set_ensure_put(nexthops, &nexthop_hash_ops, nexthop); if (r < 0) return r; if (r == 0) @@ -245,11 +241,7 @@ int nexthop_add(Link *link, NextHop *in, NextHop **ret) { return r; } else if (r == 0) { /* Take over a foreign nexthop */ - r = set_ensure_allocated(&link->nexthops, &nexthop_hash_ops); - if (r < 0) - return r; - - r = set_put(link->nexthops, nexthop); + r = set_ensure_put(&link->nexthops, &nexthop_hash_ops, nexthop); if (r < 0) return r; diff --git a/src/network/networkd-route.c b/src/network/networkd-route.c index de532eee7a..c93bf9feac 100644 --- a/src/network/networkd-route.c +++ b/src/network/networkd-route.c @@ -331,11 +331,7 @@ static int route_add_internal(Link *link, Set **routes, Route *in, Route **ret) route->initrwnd = in->initrwnd; route->lifetime = in->lifetime; - r = set_ensure_allocated(routes, &route_hash_ops); - if (r < 0) - return r; - - r = set_put(*routes, route); + r = set_ensure_put(routes, &route_hash_ops, route); if (r < 0) return r; if (r == 0) @@ -368,11 +364,7 @@ int route_add(Link *link, Route *in, Route **ret) { return r; } else if (r == 0) { /* Take over a foreign route */ - r = set_ensure_allocated(&link->routes, &route_hash_ops); - if (r < 0) - return r; - - r = set_put(link->routes, route); + r = set_ensure_put(&link->routes, &route_hash_ops, route); if (r < 0) return r; diff --git a/src/network/networkd-routing-policy-rule.c b/src/network/networkd-routing-policy-rule.c index 641f884084..c4124c0906 100644 --- a/src/network/networkd-routing-policy-rule.c +++ b/src/network/networkd-routing-policy-rule.c @@ -263,11 +263,7 @@ int routing_policy_rule_make_local(Manager *m, RoutingPolicyRule *rule) { if (set_contains(m->rules_foreign, rule)) { set_remove(m->rules_foreign, rule); - r = set_ensure_allocated(&m->rules, &routing_policy_rule_hash_ops); - if (r < 0) - return r; - - r = set_put(m->rules, rule); + r = set_ensure_put(&m->rules, &routing_policy_rule_hash_ops, rule); if (r < 0) return r; if (r == 0) @@ -295,11 +291,7 @@ static int routing_policy_rule_add_internal(Manager *m, Set **rules, RoutingPoli if (r < 0) return r; - r = set_ensure_allocated(rules, &routing_policy_rule_hash_ops); - if (r < 0) - return r; - - r = set_put(*rules, rule); + r = set_ensure_put(rules, &routing_policy_rule_hash_ops, rule); if (r < 0) return r; if (r == 0) @@ -1328,10 +1320,6 @@ int routing_policy_load_rules(const char *state_file, Set **rules) { if (!l) return -ENOMEM; - r = set_ensure_allocated(rules, &routing_policy_rule_hash_ops); - if (r < 0) - return r; - STRV_FOREACH(i, l) { _cleanup_(routing_policy_rule_freep) RoutingPolicyRule *rule = NULL; @@ -1461,7 +1449,7 @@ int routing_policy_load_rules(const char *state_file, Set **rules) { } } - r = set_put(*rules, rule); + r = set_ensure_put(rules, &routing_policy_rule_hash_ops, rule); if (r < 0) { log_warning_errno(r, "Failed to add RPDB rule to saved DB, ignoring: %s", p); continue; diff --git a/src/portable/portable.c b/src/portable/portable.c index 7917ea3902..3dc6d9e422 100644 --- a/src/portable/portable.c +++ b/src/portable/portable.c @@ -1125,7 +1125,7 @@ int portable_detach( sd_bus_error *error) { _cleanup_(lookup_paths_free) LookupPaths paths = {}; - _cleanup_set_free_free_ Set *unit_files = NULL, *markers = NULL; + _cleanup_set_free_ Set *unit_files = NULL, *markers = NULL; _cleanup_closedir_ DIR *d = NULL; const char *where, *item; Iterator iterator; @@ -1149,10 +1149,6 @@ int portable_detach( return log_debug_errno(errno, "Failed to open '%s' directory: %m", where); } - markers = set_new(&path_hash_ops); - if (!markers) - return -ENOMEM; - FOREACH_DIRENT(de, d, return log_debug_errno(errno, "Failed to enumerate '%s' directory: %m", where)) { _cleanup_free_ char *marker = NULL; UnitFileState state; @@ -1193,15 +1189,9 @@ int portable_detach( if (path_is_absolute(marker) && !image_in_search_path(IMAGE_PORTABLE, marker)) { - r = set_ensure_allocated(&markers, &path_hash_ops); + r = set_ensure_consume(&markers, &path_hash_ops_free, TAKE_PTR(marker)); if (r < 0) return r; - - r = set_put(markers, marker); - if (r >= 0) - marker = NULL; - else if (r != -EEXIST) - return r; } } diff --git a/src/resolve/resolved-dns-query.c b/src/resolve/resolved-dns-query.c index 914f464dd7..906158c5ce 100644 --- a/src/resolve/resolved-dns-query.c +++ b/src/resolve/resolved-dns-query.c @@ -94,7 +94,7 @@ static int dns_query_candidate_next_search_domain(DnsQueryCandidate *c) { } static int dns_query_candidate_add_transaction(DnsQueryCandidate *c, DnsResourceKey *key) { - DnsTransaction *t; + _cleanup_(dns_transaction_gcp) DnsTransaction *t = NULL; int r; assert(c); @@ -105,39 +105,26 @@ static int dns_query_candidate_add_transaction(DnsQueryCandidate *c, DnsResource r = dns_transaction_new(&t, c->scope, key); if (r < 0) return r; - } else { - if (set_contains(c->transactions, t)) - return 0; - } - - r = set_ensure_allocated(&c->transactions, NULL); - if (r < 0) - goto gc; - - r = set_ensure_allocated(&t->notify_query_candidates, NULL); - if (r < 0) - goto gc; + } else if (set_contains(c->transactions, t)) + return 0; r = set_ensure_allocated(&t->notify_query_candidates_done, NULL); if (r < 0) - goto gc; + return r; - r = set_put(t->notify_query_candidates, c); + r = set_ensure_put(&t->notify_query_candidates, NULL, c); if (r < 0) - goto gc; + return r; - r = set_put(c->transactions, t); + r = set_ensure_put(&c->transactions, NULL, t); if (r < 0) { (void) set_remove(t->notify_query_candidates, c); - goto gc; + return r; } t->clamp_ttl = c->query->clamp_ttl; + TAKE_PTR(t); return 1; - -gc: - dns_transaction_gc(t); - return r; } static int dns_query_candidate_go(DnsQueryCandidate *c) { diff --git a/src/resolve/resolved-dns-scope.c b/src/resolve/resolved-dns-scope.c index 1a5fef13dc..764ccee0e0 100644 --- a/src/resolve/resolved-dns-scope.c +++ b/src/resolve/resolved-dns-scope.c @@ -1254,11 +1254,7 @@ int dns_scope_announce(DnsScope *scope, bool goodbye) { if (!scope->announced && dns_resource_key_is_dnssd_ptr(z->rr->key)) { if (!set_contains(types, dns_resource_key_name(z->rr->key))) { - r = set_ensure_allocated(&types, &dns_name_hash_ops); - if (r < 0) - return log_debug_errno(r, "Failed to allocate set: %m"); - - r = set_put(types, dns_resource_key_name(z->rr->key)); + r = set_ensure_put(&types, &dns_name_hash_ops, dns_resource_key_name(z->rr->key)); if (r < 0) return log_debug_errno(r, "Failed to add item to set: %m"); } diff --git a/src/resolve/resolved-dns-stub.c b/src/resolve/resolved-dns-stub.c index ce994a7ee0..03edbe26dc 100644 --- a/src/resolve/resolved-dns-stub.c +++ b/src/resolve/resolved-dns-stub.c @@ -278,7 +278,7 @@ static int dns_stub_stream_complete(DnsStream *s, int error) { } static void dns_stub_process_query(Manager *m, DnsStream *s, DnsPacket *p) { - DnsQuery *q = NULL; + _cleanup_(dns_query_freep) DnsQuery *q = NULL; int r; assert(m); @@ -289,52 +289,52 @@ static void dns_stub_process_query(Manager *m, DnsStream *s, DnsPacket *p) { in_addr_is_localhost(p->family, &p->destination) <= 0) { log_error("Got packet on unexpected IP range, refusing."); dns_stub_send_failure(m, s, p, DNS_RCODE_SERVFAIL, false); - goto fail; + return; } r = dns_packet_extract(p); if (r < 0) { log_debug_errno(r, "Failed to extract resources from incoming packet, ignoring packet: %m"); dns_stub_send_failure(m, s, p, DNS_RCODE_FORMERR, false); - goto fail; + return; } if (!DNS_PACKET_VERSION_SUPPORTED(p)) { log_debug("Got EDNS OPT field with unsupported version number."); dns_stub_send_failure(m, s, p, DNS_RCODE_BADVERS, false); - goto fail; + return; } if (dns_type_is_obsolete(p->question->keys[0]->type)) { log_debug("Got message with obsolete key type, refusing."); dns_stub_send_failure(m, s, p, DNS_RCODE_NOTIMP, false); - goto fail; + return; } if (dns_type_is_zone_transer(p->question->keys[0]->type)) { log_debug("Got request for zone transfer, refusing."); dns_stub_send_failure(m, s, p, DNS_RCODE_NOTIMP, false); - goto fail; + return; } if (!DNS_PACKET_RD(p)) { /* If the "rd" bit is off (i.e. recursion was not requested), then refuse operation */ log_debug("Got request with recursion disabled, refusing."); dns_stub_send_failure(m, s, p, DNS_RCODE_REFUSED, false); - goto fail; + return; } if (DNS_PACKET_DO(p) && DNS_PACKET_CD(p)) { log_debug("Got request with DNSSEC CD bit set, refusing."); dns_stub_send_failure(m, s, p, DNS_RCODE_NOTIMP, false); - goto fail; + return; } r = dns_query_new(m, &q, p->question, p->question, 0, SD_RESOLVED_PROTOCOLS_ALL|SD_RESOLVED_NO_SEARCH); if (r < 0) { log_error_errno(r, "Failed to generate query object: %m"); dns_stub_send_failure(m, s, p, DNS_RCODE_SERVFAIL, false); - goto fail; + return; } /* Request that the TTL is corrected by the cached time for this lookup, so that we return vaguely useful TTLs */ @@ -348,30 +348,23 @@ static void dns_stub_process_query(Manager *m, DnsStream *s, DnsPacket *p) { /* Remember which queries belong to this stream, so that we can cancel them when the stream * is disconnected early */ - r = set_ensure_allocated(&s->queries, &trivial_hash_ops); + r = set_ensure_put(&s->queries, NULL, q); if (r < 0) { log_oom(); - goto fail; - } - - if (set_put(s->queries, q) < 0) { - log_oom(); - goto fail; + return; } + assert(r > 0); } r = dns_query_go(q); if (r < 0) { log_error_errno(r, "Failed to start query: %m"); dns_stub_send_failure(m, s, p, DNS_RCODE_SERVFAIL, false); - goto fail; + return; } log_debug("Processing query..."); - return; - -fail: - dns_query_free(q); + TAKE_PTR(q); } static int on_dns_stub_packet(sd_event_source *s, int fd, uint32_t revents, void *userdata) { diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c index 5898308d5f..cd5a0e3dd9 100644 --- a/src/resolve/resolved-dns-transaction.c +++ b/src/resolve/resolved-dns-transaction.c @@ -1501,11 +1501,7 @@ static int dns_transaction_make_packet_mdns(DnsTransaction *t) { add_known_answers = true; if (t->key->type == DNS_TYPE_ANY) { - r = set_ensure_allocated(&keys, &dns_resource_key_hash_ops); - if (r < 0) - return r; - - r = set_put(keys, t->key); + r = set_ensure_put(&keys, &dns_resource_key_hash_ops, t->key); if (r < 0) return r; } @@ -1571,11 +1567,7 @@ static int dns_transaction_make_packet_mdns(DnsTransaction *t) { add_known_answers = true; if (other->key->type == DNS_TYPE_ANY) { - r = set_ensure_allocated(&keys, &dns_resource_key_hash_ops); - if (r < 0) - return r; - - r = set_put(keys, other->key); + r = set_ensure_put(&keys, &dns_resource_key_hash_ops, other->key); if (r < 0) return r; } @@ -1800,7 +1792,7 @@ static int dns_transaction_find_cyclic(DnsTransaction *t, DnsTransaction *aux) { } static int dns_transaction_add_dnssec_transaction(DnsTransaction *t, DnsResourceKey *key, DnsTransaction **ret) { - DnsTransaction *aux; + _cleanup_(dns_transaction_gcp) DnsTransaction *aux = NULL; int r; assert(t); @@ -1833,34 +1825,22 @@ static int dns_transaction_add_dnssec_transaction(DnsTransaction *t, DnsResource } } - r = set_ensure_allocated(&t->dnssec_transactions, NULL); - if (r < 0) - goto gc; - - r = set_ensure_allocated(&aux->notify_transactions, NULL); - if (r < 0) - goto gc; - r = set_ensure_allocated(&aux->notify_transactions_done, NULL); if (r < 0) - goto gc; + return r; - r = set_put(t->dnssec_transactions, aux); + r = set_ensure_put(&t->dnssec_transactions, NULL, aux); if (r < 0) - goto gc; + return r;; - r = set_put(aux->notify_transactions, t); + r = set_ensure_put(&aux->notify_transactions, NULL, t); if (r < 0) { (void) set_remove(t->dnssec_transactions, aux); - goto gc; + return r; } - *ret = aux; + *ret = TAKE_PTR(aux); return 1; - -gc: - dns_transaction_gc(aux); - return r; } static int dns_transaction_request_dnssec_rr(DnsTransaction *t, DnsResourceKey *key) { diff --git a/src/resolve/resolved-dns-transaction.h b/src/resolve/resolved-dns-transaction.h index b1d4348409..167541806a 100644 --- a/src/resolve/resolved-dns-transaction.h +++ b/src/resolve/resolved-dns-transaction.h @@ -138,6 +138,8 @@ int dns_transaction_new(DnsTransaction **ret, DnsScope *s, DnsResourceKey *key); DnsTransaction* dns_transaction_free(DnsTransaction *t); bool dns_transaction_gc(DnsTransaction *t); +DEFINE_TRIVIAL_CLEANUP_FUNC(DnsTransaction*, dns_transaction_gc); + int dns_transaction_go(DnsTransaction *t); void dns_transaction_process_reply(DnsTransaction *t, DnsPacket *p); diff --git a/src/resolve/resolved-dns-trust-anchor.c b/src/resolve/resolved-dns-trust-anchor.c index 843f4c0f45..d68d0c3ba1 100644 --- a/src/resolve/resolved-dns-trust-anchor.c +++ b/src/resolve/resolved-dns-trust-anchor.c @@ -393,15 +393,9 @@ static int dns_trust_anchor_load_negative(DnsTrustAnchor *d, const char *path, u return -EINVAL; } - r = set_ensure_allocated(&d->negative_by_name, &dns_name_hash_ops); - if (r < 0) - return log_oom(); - - r = set_put(d->negative_by_name, domain); + r = set_ensure_consume(&d->negative_by_name, &dns_name_hash_ops, TAKE_PTR(domain)); if (r < 0) return log_oom(); - if (r > 0) - domain = NULL; return 0; } @@ -592,11 +586,7 @@ static int dns_trust_anchor_revoked_put(DnsTrustAnchor *d, DnsResourceRecord *rr assert(d); - r = set_ensure_allocated(&d->revoked_by_rr, &dns_resource_record_hash_ops); - if (r < 0) - return r; - - r = set_put(d->revoked_by_rr, rr); + r = set_ensure_put(&d->revoked_by_rr, &dns_resource_record_hash_ops, rr); if (r < 0) return r; if (r > 0) diff --git a/src/resolve/resolved-dns-zone.c b/src/resolve/resolved-dns-zone.c index 0ef4c892f7..33879d6142 100644 --- a/src/resolve/resolved-dns-zone.c +++ b/src/resolve/resolved-dns-zone.c @@ -162,7 +162,7 @@ static int dns_zone_link_item(DnsZone *z, DnsZoneItem *i) { } static int dns_zone_item_probe_start(DnsZoneItem *i) { - DnsTransaction *t; + _cleanup_(dns_transaction_gcp) DnsTransaction *t = NULL; int r; assert(i); @@ -183,25 +183,20 @@ static int dns_zone_item_probe_start(DnsZoneItem *i) { return r; } - r = set_ensure_allocated(&t->notify_zone_items, NULL); - if (r < 0) - goto gc; - r = set_ensure_allocated(&t->notify_zone_items_done, NULL); if (r < 0) - goto gc; + return r; - r = set_put(t->notify_zone_items, i); + r = set_ensure_put(&t->notify_zone_items, NULL, i); if (r < 0) - goto gc; + return r; - i->probe_transaction = t; t->probing = true; + i->probe_transaction = TAKE_PTR(t); - if (t->state == DNS_TRANSACTION_NULL) { - + if (i->probe_transaction->state == DNS_TRANSACTION_NULL) { i->block_ready++; - r = dns_transaction_go(t); + r = dns_transaction_go(i->probe_transaction); i->block_ready--; if (r < 0) { @@ -212,10 +207,6 @@ static int dns_zone_item_probe_start(DnsZoneItem *i) { dns_zone_item_notify(i); return 0; - -gc: - dns_transaction_gc(t); - return r; } int dns_zone_put(DnsZone *z, DnsScope *s, DnsResourceRecord *rr, bool probe) { diff --git a/src/resolve/resolved-etc-hosts.c b/src/resolve/resolved-etc-hosts.c index 2cb06c098d..6a7f749957 100644 --- a/src/resolve/resolved-etc-hosts.c +++ b/src/resolve/resolved-etc-hosts.c @@ -120,15 +120,10 @@ static int parse_line(EtcHosts *hosts, unsigned nr, const char *line) { /* Optimize the case where we don't need to store any addresses, by storing * only the name in a dedicated Set instead of the hashmap */ - r = set_ensure_allocated(&hosts->no_address, &dns_name_hash_ops); - if (r < 0) - return log_oom(); - - r = set_put(hosts->no_address, name); + r = set_ensure_consume(&hosts->no_address, &dns_name_hash_ops, TAKE_PTR(name)); if (r < 0) return r; - TAKE_PTR(name); continue; } diff --git a/src/shared/bus-util.c b/src/shared/bus-util.c index a61f8e70da..f64e200731 100644 --- a/src/shared/bus-util.c +++ b/src/shared/bus-util.c @@ -613,13 +613,9 @@ int bus_message_print_all_properties( return r; if (found_properties) { - r = set_ensure_allocated(found_properties, &string_hash_ops); + r = set_ensure_put(found_properties, &string_hash_ops, name); if (r < 0) return log_oom(); - - r = set_put(*found_properties, name); - if (r < 0 && r != -EEXIST) - return log_oom(); } name_with_equal = strjoin(name, "="); diff --git a/src/shared/seccomp-util.c b/src/shared/seccomp-util.c index 6a3cfe770c..de05fb092c 100644 --- a/src/shared/seccomp-util.c +++ b/src/shared/seccomp-util.c @@ -1742,17 +1742,13 @@ int seccomp_restrict_archs(Set *archs) { return 0; } -int parse_syscall_archs(char **l, Set **archs) { - _cleanup_set_free_ Set *_archs = NULL; +int parse_syscall_archs(char **l, Set **ret_archs) { + _cleanup_set_free_ Set *archs = NULL; char **s; int r; assert(l); - assert(archs); - - r = set_ensure_allocated(&_archs, NULL); - if (r < 0) - return r; + assert(ret_archs); STRV_FOREACH(s, l) { uint32_t a; @@ -1761,13 +1757,12 @@ int parse_syscall_archs(char **l, Set **archs) { if (r < 0) return -EINVAL; - r = set_put(_archs, UINT32_TO_PTR(a + 1)); + r = set_ensure_put(&archs, NULL, UINT32_TO_PTR(a + 1)); if (r < 0) return -ENOMEM; } - *archs = TAKE_PTR(_archs); - + *ret_archs = TAKE_PTR(archs); return 0; } diff --git a/src/shared/seccomp-util.h b/src/shared/seccomp-util.h index 0b48e74a87..9580f9268d 100644 --- a/src/shared/seccomp-util.h +++ b/src/shared/seccomp-util.h @@ -105,6 +105,6 @@ extern const uint32_t seccomp_local_archs[]; DEFINE_TRIVIAL_CLEANUP_FUNC(scmp_filter_ctx, seccomp_release); -int parse_syscall_archs(char **l, Set **archs); +int parse_syscall_archs(char **l, Set **ret_archs); uint32_t scmp_act_kill_process(void); diff --git a/src/shared/userdb.c b/src/shared/userdb.c index 6c955ed523..6fedbd6ebe 100644 --- a/src/shared/userdb.c +++ b/src/shared/userdb.c @@ -377,15 +377,9 @@ static int userdb_connect( if (r < 0) return log_debug_errno(r, "Failed to invoke varlink method: %m"); - r = set_ensure_allocated(&iterator->links, &link_hash_ops); - if (r < 0) - return log_debug_errno(r, "Failed to allocate set: %m"); - - r = set_put(iterator->links, vl); + r = set_ensure_consume(&iterator->links, &link_hash_ops, TAKE_PTR(vl)); if (r < 0) return log_debug_errno(r, "Failed to add varlink connection to set: %m"); - - TAKE_PTR(vl); return r; } diff --git a/src/socket-proxy/socket-proxyd.c b/src/socket-proxy/socket-proxyd.c index 7b548c5076..b461aead60 100644 --- a/src/socket-proxy/socket-proxyd.c +++ b/src/socket-proxy/socket-proxyd.c @@ -480,25 +480,21 @@ static int add_connection_socket(Context *context, int fd) { log_warning_errno(r, "Unable to disable idle timer, continuing: %m"); } - r = set_ensure_allocated(&context->connections, NULL); - if (r < 0) { - log_oom(); - return 0; - } - - c = new0(Connection, 1); + c = new(Connection, 1); if (!c) { log_oom(); return 0; } - c->context = context; - c->server_fd = fd; - c->client_fd = -1; - c->server_to_client_buffer[0] = c->server_to_client_buffer[1] = -1; - c->client_to_server_buffer[0] = c->client_to_server_buffer[1] = -1; + *c = (Connection) { + .context = context, + .server_fd = fd, + .client_fd = -1, + .server_to_client_buffer = {-1, -1}, + .client_to_server_buffer = {-1, -1}, + }; - r = set_put(context->connections, c); + r = set_ensure_put(&context->connections, NULL, c); if (r < 0) { free(c); log_oom(); @@ -550,12 +546,6 @@ static int add_listen_socket(Context *context, int fd) { assert(context); assert(fd >= 0); - r = set_ensure_allocated(&context->listen, NULL); - if (r < 0) { - log_oom(); - return r; - } - r = sd_is_socket(fd, 0, SOCK_STREAM, 1); if (r < 0) return log_error_errno(r, "Failed to determine socket type: %m"); @@ -571,7 +561,7 @@ static int add_listen_socket(Context *context, int fd) { if (r < 0) return log_error_errno(r, "Failed to add event source: %m"); - r = set_put(context->listen, source); + r = set_ensure_put(&context->listen, NULL, source); if (r < 0) { log_error_errno(r, "Failed to add source to set: %m"); sd_event_source_unref(source); diff --git a/src/sysv-generator/sysv-generator.c b/src/sysv-generator/sysv-generator.c index eb6b2d084e..a2c72d1009 100644 --- a/src/sysv-generator/sysv-generator.c +++ b/src/sysv-generator/sysv-generator.c @@ -817,7 +817,6 @@ static int set_dependencies_from_rcnd(const LookupPaths *lp, Hashmap *all_servic Set *runlevel_services[ELEMENTSOF(rcnd_table)] = {}; _cleanup_strv_free_ char **sysvrcnd_path = NULL; SysvStub *service; - unsigned i; Iterator j; char **p; int r; @@ -828,9 +827,8 @@ static int set_dependencies_from_rcnd(const LookupPaths *lp, Hashmap *all_servic if (r < 0) return r; - STRV_FOREACH(p, sysvrcnd_path) { - for (i = 0; i < ELEMENTSOF(rcnd_table); i ++) { - + STRV_FOREACH(p, sysvrcnd_path) + for (unsigned i = 0; i < ELEMENTSOF(rcnd_table); i ++) { _cleanup_closedir_ DIR *d = NULL; _cleanup_free_ char *path = NULL; struct dirent *de; @@ -885,22 +883,15 @@ static int set_dependencies_from_rcnd(const LookupPaths *lp, Hashmap *all_servic service->sysv_start_priority = MAX(a*10 + b, service->sysv_start_priority); - r = set_ensure_allocated(&runlevel_services[i], NULL); - if (r < 0) { - log_oom(); - goto finish; - } - - r = set_put(runlevel_services[i], service); + r = set_ensure_put(&runlevel_services[i], NULL, service); if (r < 0) { log_oom(); goto finish; } } } - } - for (i = 0; i < ELEMENTSOF(rcnd_table); i ++) + for (unsigned i = 0; i < ELEMENTSOF(rcnd_table); i++) SET_FOREACH(service, runlevel_services[i], j) { r = strv_extend(&service->before, rcnd_table[i].target); if (r < 0) { @@ -917,7 +908,7 @@ static int set_dependencies_from_rcnd(const LookupPaths *lp, Hashmap *all_servic r = 0; finish: - for (i = 0; i < ELEMENTSOF(rcnd_table); i++) + for (unsigned i = 0; i < ELEMENTSOF(rcnd_table); i++) set_free(runlevel_services[i]); return r; diff --git a/src/test/test-set.c b/src/test/test-set.c index 9c93685dbc..d3e6de7978 100644 --- a/src/test/test-set.c +++ b/src/test/test-set.c @@ -107,6 +107,49 @@ static void test_set_put_strdupv(void) { assert_se(set_size(m) == 3); } +static void test_set_ensure_allocated(void) { + _cleanup_set_free_ Set *m = NULL; + + assert_se(set_ensure_allocated(&m, &string_hash_ops) == 1); + assert_se(set_ensure_allocated(&m, &string_hash_ops) == 0); + assert_se(set_ensure_allocated(&m, NULL) == 0); + assert_se(set_size(m) == 0); +} + +static void test_set_ensure_put(void) { + _cleanup_set_free_ Set *m = NULL; + + assert_se(set_ensure_put(&m, &string_hash_ops, "a") == 1); + assert_se(set_ensure_put(&m, &string_hash_ops, "a") == 0); + assert_se(set_ensure_put(&m, NULL, "a") == 0); + assert_se(set_ensure_put(&m, &string_hash_ops, "b") == 1); + assert_se(set_ensure_put(&m, &string_hash_ops, "b") == 0); + assert_se(set_ensure_put(&m, &string_hash_ops, "a") == 0); + assert_se(set_size(m) == 2); +} + +static void test_set_ensure_consume(void) { + _cleanup_set_free_ Set *m = NULL; + char *s, *t; + + assert_se(s = strdup("a")); + assert_se(set_ensure_consume(&m, &string_hash_ops_free, s) == 1); + + assert_se(t = strdup("a")); + assert_se(set_ensure_consume(&m, &string_hash_ops_free, t) == 0); + + assert_se(t = strdup("a")); + assert_se(set_ensure_consume(&m, &string_hash_ops_free, t) == 0); + + assert_se(t = strdup("b")); + assert_se(set_ensure_consume(&m, &string_hash_ops_free, t) == 1); + + assert_se(t = strdup("b")); + assert_se(set_ensure_consume(&m, &string_hash_ops_free, t) == 0); + + assert_se(set_size(m) == 2); +} + int main(int argc, const char *argv[]) { test_set_steal_first(); test_set_free_with_destructor(); @@ -114,6 +157,9 @@ int main(int argc, const char *argv[]) { test_set_put(); test_set_put_strdup(); test_set_put_strdupv(); + test_set_ensure_allocated(); + test_set_ensure_put(); + test_set_ensure_consume(); return 0; } diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c index 8a0b8d3b84..7ad6f13b66 100644 --- a/src/tmpfiles/tmpfiles.c +++ b/src/tmpfiles/tmpfiles.c @@ -3196,11 +3196,7 @@ static int link_parent(ItemArray *a) { if (!j) j = ordered_hashmap_get(globs, prefix); if (j) { - r = set_ensure_allocated(&j->children, NULL); - if (r < 0) - return log_oom(); - - r = set_put(j->children, a); + r = set_ensure_put(&j->children, NULL, a); if (r < 0) return log_oom(); diff --git a/src/udev/udevadm-monitor.c b/src/udev/udevadm-monitor.c index 2ca98a729f..8cd8ed1a1e 100644 --- a/src/udev/udevadm-monitor.c +++ b/src/udev/udevadm-monitor.c @@ -169,24 +169,13 @@ static int parse_argv(int argc, char *argv[]) { subsystem = devtype = NULL; break; } - case 't': { - _cleanup_free_ char *tag = NULL; - - r = set_ensure_allocated(&arg_tag_filter, &string_hash_ops); + 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); if (r < 0) return r; - - tag = strdup(optarg); - if (!tag) - return -ENOMEM; - - r = set_put(arg_tag_filter, tag); - if (r < 0) - return r; - - tag = NULL; break; - } + case 'V': return print_version(); case 'h': @@ -260,7 +249,7 @@ int monitor_main(int argc, char *argv[], void *userdata) { finalize: hashmap_free_free_free(arg_subsystem_filter); - set_free_free(arg_tag_filter); + set_free(arg_tag_filter); return r; } diff --git a/test/fuzz/fuzz-netdev-parser/wireguard-duplicated-endpoint b/test/fuzz/fuzz-netdev-parser/wireguard-duplicated-endpoint new file mode 100644 index 0000000000..adff4c186b --- /dev/null +++ b/test/fuzz/fuzz-netdev-parser/wireguard-duplicated-endpoint @@ -0,0 +1,6 @@ +[NetDev] +Name=w +Kind=wireguard +[WireGuardPeer] +Endpoint=:0 +Endpoint=:8
\ No newline at end of file diff --git a/test/fuzz/fuzz-network-parser/dns-trust-anchor-duplicate.network b/test/fuzz/fuzz-network-parser/dns-trust-anchor-duplicate.network new file mode 100644 index 0000000000..ed7bdabfd1 --- /dev/null +++ b/test/fuzz/fuzz-network-parser/dns-trust-anchor-duplicate.network @@ -0,0 +1,2 @@ +[Network] +DNSSECNegativeTrustAnchors=i i
\ No newline at end of file |