From 29bd6012f11a9d375ebe3ba393a9a3e8b88a245f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 3 Jun 2020 14:25:18 +0200 Subject: [PATCH 01/20] resolved: introduce dns_transaction_gcp() --- src/resolve/resolved-dns-query.c | 23 +++++++++-------------- src/resolve/resolved-dns-transaction.c | 18 +++++++----------- src/resolve/resolved-dns-transaction.h | 2 ++ src/resolve/resolved-dns-zone.c | 19 +++++++------------ 4 files changed, 25 insertions(+), 37 deletions(-) diff --git a/src/resolve/resolved-dns-query.c b/src/resolve/resolved-dns-query.c index d6eca6dfdd..89e5d3bf67 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,34 @@ 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; - } + } else if (set_contains(c->transactions, t)) + return 0; r = set_ensure_allocated(&c->transactions, NULL); if (r < 0) - goto gc; + return r; r = set_ensure_allocated(&t->notify_query_candidates, NULL); if (r < 0) - goto gc; + return r; 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); if (r < 0) - goto gc; + return r; r = set_put(c->transactions, 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-transaction.c b/src/resolve/resolved-dns-transaction.c index 5898308d5f..46854a5ad4 100644 --- a/src/resolve/resolved-dns-transaction.c +++ b/src/resolve/resolved-dns-transaction.c @@ -1800,7 +1800,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); @@ -1835,32 +1835,28 @@ static int dns_transaction_add_dnssec_transaction(DnsTransaction *t, DnsResource r = set_ensure_allocated(&t->dnssec_transactions, NULL); if (r < 0) - goto gc; + return r;; r = set_ensure_allocated(&aux->notify_transactions, NULL); if (r < 0) - goto gc; + return r; r = set_ensure_allocated(&aux->notify_transactions_done, NULL); if (r < 0) - goto gc; + return r; r = set_put(t->dnssec_transactions, aux); if (r < 0) - goto gc; + return r; r = set_put(aux->notify_transactions, 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-zone.c b/src/resolve/resolved-dns-zone.c index 0ef4c892f7..827ad529e1 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); @@ -185,23 +185,22 @@ static int dns_zone_item_probe_start(DnsZoneItem *i) { r = set_ensure_allocated(&t->notify_zone_items, NULL); if (r < 0) - goto gc; + return r; 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); 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 +211,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) { From aaffd349339ea3b016eea10a0daa63e892b2df7c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 3 Jun 2020 14:58:01 +0200 Subject: [PATCH 02/20] sd-device: use set_put_strdup() in one more place --- src/libsystemd/sd-device/device-monitor.c | 25 ++++------------------- 1 file changed, 4 insertions(+), 21 deletions(-) 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) { From 0f9ccd95525c9869b6a23b71f6b7dbb2a7038674 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 3 Jun 2020 14:25:18 +0200 Subject: [PATCH 03/20] basic/set: add set_ensure_put() It's such a common operation to allocate the set and put an item in it, that it deserves a helper. set_ensure_put() has the same return values as set_put(). Comes with tests! --- src/basic/hashmap.c | 10 ++++++++++ src/basic/set.h | 3 +++ src/test/test-set.c | 23 +++++++++++++++++++++++ 3 files changed, 36 insertions(+) diff --git a/src/basic/hashmap.c b/src/basic/hashmap.c index 15c8c4723c..c876662a0b 100644 --- a/src/basic/hashmap.c +++ b/src/basic/hashmap.c @@ -1247,6 +1247,16 @@ 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 hashmap_replace(Hashmap *h, const void *key, void *value) { struct swap_entries swap; struct plain_hashmap_entry *e; diff --git a/src/basic/set.h b/src/basic/set.h index 621e83bf27..cdc4abaa42 100644 --- a/src/basic/set.h +++ b/src/basic/set.h @@ -120,6 +120,9 @@ 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_consume(Set *s, void *value); int set_put_strdup(Set **s, const char *p); int set_put_strdupv(Set **s, char **l); diff --git a/src/test/test-set.c b/src/test/test-set.c index 9c93685dbc..7213abda01 100644 --- a/src/test/test-set.c +++ b/src/test/test-set.c @@ -107,6 +107,27 @@ 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); +} + int main(int argc, const char *argv[]) { test_set_steal_first(); test_set_free_with_destructor(); @@ -114,6 +135,8 @@ 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(); return 0; } From de7fef4b6eb4f5ca1cd21a309c37d0a74d4d305b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 5 Jun 2020 15:12:29 +0200 Subject: [PATCH 04/20] tree-wide: use set_ensure_put() Patch contains a coccinelle script, but it only works in some cases. Many parts were converted by hand. Note: I did not fix errors in return value handing. This will be done separate to keep the patch comprehensible. No functional change is intended in this patch. --- coccinelle/set_ensure_put.cocci | 18 ++++++++++++ src/core/automount.c | 32 +++------------------- src/core/bpf-firewall.c | 13 ++------- src/core/dbus-execute.c | 6 +--- src/core/load-fragment.c | 6 +--- src/core/manager.c | 5 +--- src/core/unit.c | 7 +---- src/journal/journalctl.c | 7 +---- src/libsystemd/sd-event/sd-event.c | 7 ++--- src/login/logind-brightness.c | 7 ++--- src/network/netdev/wireguard.c | 17 ++---------- src/network/networkd-address.c | 16 ++--------- src/network/networkd-dhcp-common.c | 13 ++------- src/network/networkd-dhcp4.c | 9 ++---- src/network/networkd-link.c | 16 ++--------- src/network/networkd-manager.c | 12 ++------ src/network/networkd-ndisc.c | 26 ++++-------------- src/network/networkd-neighbor.c | 15 ++-------- src/network/networkd-network.c | 8 ++---- src/network/networkd-nexthop.c | 12 ++------ src/network/networkd-route.c | 12 ++------ src/network/networkd-routing-policy-rule.c | 18 ++---------- src/portable/portable.c | 6 +--- src/resolve/resolved-dns-query.c | 12 ++------ src/resolve/resolved-dns-scope.c | 6 +--- src/resolve/resolved-dns-stub.c | 7 +---- src/resolve/resolved-dns-transaction.c | 26 ++++-------------- src/resolve/resolved-dns-trust-anchor.c | 12 ++------ src/resolve/resolved-dns-zone.c | 6 +--- src/resolve/resolved-etc-hosts.c | 6 +--- src/shared/bus-util.c | 6 +--- src/shared/seccomp-util.c | 15 ++++------ src/shared/seccomp-util.h | 2 +- src/shared/userdb.c | 6 +--- src/socket-proxy/socket-proxyd.c | 16 ++--------- src/sysv-generator/sysv-generator.c | 8 +----- src/tmpfiles/tmpfiles.c | 6 +--- 37 files changed, 92 insertions(+), 330 deletions(-) create mode 100644 coccinelle/set_ensure_put.cocci 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/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..a05ac8122d 100644 --- a/src/core/bpf-firewall.c +++ b/src/core/bpf-firewall.c @@ -606,11 +606,7 @@ 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_put(set, &filter_prog_hash_ops, prog); if (r < 0) return log_unit_error_errno(u, r, "Can't add program to BPF program set: %m"); TAKE_PTR(prog); @@ -662,12 +658,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 06b6b95d69..5b217c059d 100644 --- a/src/core/dbus-execute.c +++ b/src/core/dbus-execute.c @@ -1697,10 +1697,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; @@ -1708,7 +1704,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 c1a4eb96cb..c5664f968b 100644 --- a/src/core/load-fragment.c +++ b/src/core/load-fragment.c @@ -3043,10 +3043,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; @@ -3069,7 +3065,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 72dd93fa95..48bd86e54f 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -4421,12 +4421,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 859f4bbd44..36bd9a0148 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-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..5c0c55fc36 100644 --- a/src/login/logind-brightness.c +++ b/src/login/logind-brightness.c @@ -174,15 +174,12 @@ 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); + 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..757446aeec 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; @@ -809,15 +802,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(); - - r = set_put(w->peers_with_unresolved_endpoint, peer); - if (r < 0) - return r; - 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 afeb36d315..baa1cd3f7f 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; @@ -4440,16 +4436,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..14a883aa30 100644 --- a/src/network/networkd-manager.c +++ b/src/network/networkd-manager.c @@ -2309,19 +2309,11 @@ 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(); - r = set_ensure_allocated(&m->duids_requesting_uuid, NULL); - 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); + r = set_ensure_put(&m->duids_requesting_uuid, NULL, duid); if (r < 0) return log_oom(); diff --git a/src/network/networkd-ndisc.c b/src/network/networkd-ndisc.c index 19a655b2d7..beeb343756 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,10 +600,9 @@ 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_put(&link->ndisc_rdnss, &ndisc_rdnss_hash_ops, x); if (r < 0) return log_oom(); - TAKE_PTR(x); assert(r > 0); @@ -686,22 +681,16 @@ 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_put(&link->ndisc_dnssl, &ndisc_dnssl_hash_ops, s); if (r < 0) { log_oom(); return; } - - s = NULL; + TAKE_PTR(s); assert(r > 0); + link_dirty(link); } } @@ -979,21 +968,16 @@ 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); + r = set_ensure_put(&network->ndisc_black_listed_prefix, &in6_addr_hash_ops, 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); } 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..01f08a5552 100644 --- a/src/network/networkd-network.c +++ b/src/network/networkd-network.c @@ -1268,15 +1268,11 @@ int config_parse_dnssec_negative_trust_anchors( continue; } - r = set_ensure_allocated(&n->dnssec_negative_trust_anchors, &dns_name_hash_ops); - if (r < 0) - return log_oom(); - - r = set_put(n->dnssec_negative_trust_anchors, w); + r = set_ensure_put(&n->dnssec_negative_trust_anchors, &dns_name_hash_ops, w); if (r < 0) return log_oom(); if (r > 0) - w = NULL; + TAKE_PTR(w); } 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..57c224decd 100644 --- a/src/portable/portable.c +++ b/src/portable/portable.c @@ -1193,11 +1193,7 @@ int portable_detach( if (path_is_absolute(marker) && !image_in_search_path(IMAGE_PORTABLE, marker)) { - r = set_ensure_allocated(&markers, &path_hash_ops); - if (r < 0) - return r; - - r = set_put(markers, marker); + r = set_ensure_put(&markers, &path_hash_ops, marker); if (r >= 0) marker = NULL; else if (r != -EEXIST) diff --git a/src/resolve/resolved-dns-query.c b/src/resolve/resolved-dns-query.c index 89e5d3bf67..cf332b8cd5 100644 --- a/src/resolve/resolved-dns-query.c +++ b/src/resolve/resolved-dns-query.c @@ -108,23 +108,15 @@ static int dns_query_candidate_add_transaction(DnsQueryCandidate *c, DnsResource } else if (set_contains(c->transactions, t)) return 0; - r = set_ensure_allocated(&c->transactions, NULL); - if (r < 0) - return r; - - r = set_ensure_allocated(&t->notify_query_candidates, NULL); - if (r < 0) - return r; - r = set_ensure_allocated(&t->notify_query_candidates_done, NULL); if (r < 0) return r; - r = set_put(t->notify_query_candidates, c); + r = set_ensure_put(&t->notify_query_candidates, NULL, c); if (r < 0) 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); return r; diff --git a/src/resolve/resolved-dns-scope.c b/src/resolve/resolved-dns-scope.c index d06e428011..140cd25586 100644 --- a/src/resolve/resolved-dns-scope.c +++ b/src/resolve/resolved-dns-scope.c @@ -1253,11 +1253,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..d9e6218081 100644 --- a/src/resolve/resolved-dns-stub.c +++ b/src/resolve/resolved-dns-stub.c @@ -348,16 +348,11 @@ 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, &trivial_hash_ops, q); if (r < 0) { log_oom(); goto fail; } - - if (set_put(s->queries, q) < 0) { - log_oom(); - goto fail; - } } r = dns_query_go(q); diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c index 46854a5ad4..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; } @@ -1833,23 +1825,15 @@ static int dns_transaction_add_dnssec_transaction(DnsTransaction *t, DnsResource } } - r = set_ensure_allocated(&t->dnssec_transactions, NULL); - if (r < 0) - return r;; - - r = set_ensure_allocated(&aux->notify_transactions, NULL); - if (r < 0) - return r; - r = set_ensure_allocated(&aux->notify_transactions_done, NULL); if (r < 0) return r; - r = set_put(t->dnssec_transactions, aux); + r = set_ensure_put(&t->dnssec_transactions, NULL, aux); if (r < 0) - return r; + 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); return r; diff --git a/src/resolve/resolved-dns-trust-anchor.c b/src/resolve/resolved-dns-trust-anchor.c index 843f4c0f45..1cb7492905 100644 --- a/src/resolve/resolved-dns-trust-anchor.c +++ b/src/resolve/resolved-dns-trust-anchor.c @@ -393,11 +393,7 @@ 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_put(&d->negative_by_name, &dns_name_hash_ops, domain); if (r < 0) return log_oom(); if (r > 0) @@ -592,11 +588,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 827ad529e1..33879d6142 100644 --- a/src/resolve/resolved-dns-zone.c +++ b/src/resolve/resolved-dns-zone.c @@ -183,15 +183,11 @@ static int dns_zone_item_probe_start(DnsZoneItem *i) { return r; } - r = set_ensure_allocated(&t->notify_zone_items, NULL); - if (r < 0) - return r; - r = set_ensure_allocated(&t->notify_zone_items_done, NULL); if (r < 0) return r; - r = set_put(t->notify_zone_items, i); + r = set_ensure_put(&t->notify_zone_items, NULL, i); if (r < 0) return r; diff --git a/src/resolve/resolved-etc-hosts.c b/src/resolve/resolved-etc-hosts.c index 2cb06c098d..a2c84c17a4 100644 --- a/src/resolve/resolved-etc-hosts.c +++ b/src/resolve/resolved-etc-hosts.c @@ -120,11 +120,7 @@ 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_put(&hosts->no_address, &dns_name_hash_ops, name); if (r < 0) return r; diff --git a/src/shared/bus-util.c b/src/shared/bus-util.c index a61f8e70da..a77c736fcb 100644 --- a/src/shared/bus-util.c +++ b/src/shared/bus-util.c @@ -613,11 +613,7 @@ int bus_message_print_all_properties( return r; if (found_properties) { - r = set_ensure_allocated(found_properties, &string_hash_ops); - if (r < 0) - return log_oom(); - - r = set_put(*found_properties, name); + r = set_ensure_put(found_properties, &string_hash_ops, name); if (r < 0 && r != -EEXIST) return log_oom(); } 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 c3a6e02e5a..f3b05a7ad3 100644 --- a/src/shared/userdb.c +++ b/src/shared/userdb.c @@ -359,11 +359,7 @@ 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_put(&iterator->links, &link_hash_ops, vl); if (r < 0) return log_debug_errno(r, "Failed to add varlink connection to set: %m"); diff --git a/src/socket-proxy/socket-proxyd.c b/src/socket-proxy/socket-proxyd.c index 7b548c5076..3b7c5799c8 100644 --- a/src/socket-proxy/socket-proxyd.c +++ b/src/socket-proxy/socket-proxyd.c @@ -480,12 +480,6 @@ 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); if (!c) { log_oom(); @@ -498,7 +492,7 @@ static int add_connection_socket(Context *context, int fd) { 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; - r = set_put(context->connections, c); + r = set_ensure_put(&context->connections, NULL, c); if (r < 0) { free(c); log_oom(); @@ -550,12 +544,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 +559,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..84f329e37d 100644 --- a/src/sysv-generator/sysv-generator.c +++ b/src/sysv-generator/sysv-generator.c @@ -885,13 +885,7 @@ 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; diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c index e827de1b06..7938cb3d73 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(); From 44e9342078c3445d518271ccb8097f34d100613a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 22 Jun 2020 15:55:04 +0200 Subject: [PATCH 05/20] netdev/wireguard: do not invalidate peer on invalid syntax We would say "ignoring", but invalidate the peer anyway. Let's only do that if we modified the peer irreperably. Also add comments explaining allocation handling. --- src/network/netdev/wireguard.c | 25 +++++++++---------------- 1 file changed, 9 insertions(+), 16 deletions(-) diff --git a/src/network/netdev/wireguard.c b/src/network/netdev/wireguard.c index 757446aeec..bff778179c 100644 --- a/src/network/netdev/wireguard.c +++ b/src/network/netdev/wireguard.c @@ -573,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; @@ -585,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( @@ -759,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, ']'); @@ -794,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(); @@ -805,7 +800,7 @@ int config_parse_wireguard_endpoint( r = set_ensure_put(&w->peers_with_unresolved_endpoint, NULL, peer); if (r < 0) return log_oom(); - TAKE_PTR(peer); + TAKE_PTR(peer); /* The peer may already have been in the hash map, that is fine too. */ return 0; } @@ -822,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; @@ -843,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; } From 804a436582e448c2becfd7bb047212341d56151e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 22 Jun 2020 16:27:02 +0200 Subject: [PATCH 06/20] fuzz-netdev-parser: add test case for earlier version of preceding patches AddressSanitizer:DEADLYSIGNAL ================================================================= ==12==ERROR: AddressSanitizer: ABRT on unknown address 0x00000000000c (pc 0x7f0a518b3428 bp 0x7fffa463bfd0 sp 0x7fffa463be68 T0) SCARINESS: 10 (signal) #0 0x7f0a518b3428 in raise (/lib/x86_64-linux-gnu/libc.so.6+0x35428) #1 0x7f0a518b5029 in abort (/lib/x86_64-linux-gnu/libc.so.6+0x37029) #2 0x7f0a52ca635a in log_assert_failed_realm /work/build/../../src/systemd/src/basic/log.c:819:9 #3 0x4eea92 in config_parse_wireguard_endpoint /work/build/../../src/systemd/src/network/netdev/wireguard.c:808:9 #4 0x7f0a52b2f74e in next_assignment /work/build/../../src/systemd/src/shared/conf-parser.c:133:32 #5 0x7f0a52b2954e in parse_line /work/build/../../src/systemd/src/shared/conf-parser.c:242:16 #6 0x7f0a52b28911 in config_parse /work/build/../../src/systemd/src/shared/conf-parser.c:377:21 #7 0x7f0a52b29ec6 in config_parse_many_files /work/build/../../src/systemd/src/shared/conf-parser.c:439:21 #8 0x7f0a52b2a5a6 in config_parse_many /work/build/../../src/systemd/src/shared/conf-parser.c:507:16 #9 0x4d8d6c in netdev_load_one /work/build/../../src/systemd/src/network/netdev/netdev.c:732:13 #10 0x4d3e2b in LLVMFuzzerTestOneInput /work/build/../../src/systemd/src/network/fuzz-netdev-parser.c:23:16 #11 0x6b3266 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /src/libfuzzer/FuzzerLoop.cpp:558:15 #12 0x6af860 in fuzzer::Fuzzer::RunOne(unsigned char const*, unsigned long, bool, fuzzer::InputInfo*, bool*) /src/libfuzzer/FuzzerLoop.cpp:470:3 #13 0x6b6970 in fuzzer::Fuzzer::ReadAndExecuteSeedCorpora(std::__1::vector >&) /src/libfuzzer/FuzzerLoop.cpp:770:7 #14 0x6b7376 in fuzzer::Fuzzer::Loop(std::__1::vector >&) /src/libfuzzer/FuzzerLoop.cpp:799:3 #15 0x67573f in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /src/libfuzzer/FuzzerDriver.cpp:846:6 #16 0x667097 in main /src/libfuzzer/FuzzerMain.cpp:19:10 #17 0x7f0a5189e82f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f) #18 0x4295a8 in _start (out/fuzz-netdev-parser+0x4295a8) DEDUP_TOKEN: raise--abort--log_assert_failed_realm AddressSanitizer can not provide additional info. SUMMARY: AddressSanitizer: ABRT (/lib/x86_64-linux-gnu/libc.so.6+0x35428) in raise ==12==ABORTING --- test/fuzz/fuzz-netdev-parser/wireguard-duplicated-endpoint | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 test/fuzz/fuzz-netdev-parser/wireguard-duplicated-endpoint 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 From e2ab8e09c8f5005f98e7ae6b4655f12514bae36a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 4 Jun 2020 17:21:42 +0200 Subject: [PATCH 07/20] udevadm-monitor: simplify creation of arg_tag_filter set This version is more efficient, which doesn't matter, but it allows us to remove a bunch of error handling, which is always nice. --- src/udev/udevadm-monitor.c | 21 +++++---------------- 1 file changed, 5 insertions(+), 16 deletions(-) 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; } From 6cd55d6f74a49ebf6532f913f6a9cde3efcd34cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 4 Jun 2020 17:41:18 +0200 Subject: [PATCH 08/20] shared/bus-util: fix misleading error handling set_put()/set_ensure_put() return 0, not -EEXIST, if the entry is already found in the set. In this case this does not make any difference, but let's not confuse the reader. --- src/shared/bus-util.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/shared/bus-util.c b/src/shared/bus-util.c index a77c736fcb..f64e200731 100644 --- a/src/shared/bus-util.c +++ b/src/shared/bus-util.c @@ -614,7 +614,7 @@ int bus_message_print_all_properties( if (found_properties) { r = set_ensure_put(found_properties, &string_hash_ops, name); - if (r < 0 && r != -EEXIST) + if (r < 0) return log_oom(); } From fcc1d0315d335ba31d85d6023c79d6404c62e167 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 4 Jun 2020 19:46:14 +0200 Subject: [PATCH 09/20] basic/set: add set_ensure_consume() This combines set_ensure_allocated() with set_consume(). The cool thing is that because we know the hash ops, we can correctly free the item if appropriate. Similarly to set_consume(), the goal is to simplify handling of the case where the item needs to be freed on error and if already present in the set. --- src/basic/hashmap.c | 14 +++++++++++ src/basic/set.h | 3 +++ src/test/test-set.c | 23 +++++++++++++++++++ .../dns-trust-anchor-duplicate.network | 2 ++ 4 files changed, 42 insertions(+) create mode 100644 test/fuzz/fuzz-network-parser/dns-trust-anchor-duplicate.network diff --git a/src/basic/hashmap.c b/src/basic/hashmap.c index c876662a0b..ae3235d518 100644 --- a/src/basic/hashmap.c +++ b/src/basic/hashmap.c @@ -1257,6 +1257,20 @@ int _set_ensure_put(Set **s, const struct hash_ops *hash_ops, const void *key H 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; diff --git a/src/basic/set.h b/src/basic/set.h index cdc4abaa42..3684a00f78 100644 --- a/src/basic/set.h +++ b/src/basic/set.h @@ -123,6 +123,9 @@ static inline char **set_get_strv(Set *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); diff --git a/src/test/test-set.c b/src/test/test-set.c index 7213abda01..d3e6de7978 100644 --- a/src/test/test-set.c +++ b/src/test/test-set.c @@ -128,6 +128,28 @@ static void test_set_ensure_put(void) { 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(); @@ -137,6 +159,7 @@ int main(int argc, const char *argv[]) { test_set_put_strdupv(); test_set_ensure_allocated(); test_set_ensure_put(); + test_set_ensure_consume(); return 0; } 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 From b8b46b1ce5e1528522c3ebb752721f0bc3250961 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 4 Jun 2020 19:58:18 +0200 Subject: [PATCH 10/20] basic/set,hashmap: pass through allocation info in more cases --- src/basic/hashmap.c | 12 ++++++------ src/basic/hashmap.h | 3 ++- src/basic/set.h | 8 ++++++-- 3 files changed, 14 insertions(+), 9 deletions(-) diff --git a/src/basic/hashmap.c b/src/basic/hashmap.c index ae3235d518..b98696a136 100644 --- a/src/basic/hashmap.c +++ b/src/basic/hashmap.c @@ -1789,10 +1789,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; @@ -1823,14 +1823,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; @@ -1844,14 +1844,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..bb11069508 100644 --- a/src/basic/hashmap.h +++ b/src/basic/hashmap.h @@ -154,7 +154,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 3684a00f78..696de8ad57 100644 --- a/src/basic/set.h +++ b/src/basic/set.h @@ -127,8 +127,12 @@ int _set_ensure_consume(Set **s, const struct hash_ops *hash_ops, void *key HAS #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) \ From ceb17827fa9e5014c9750021bb1a98c326cbef34 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 5 Jun 2020 08:46:08 +0200 Subject: [PATCH 11/20] resolved: simplify allocation failure handling in dns_stub_process_query() Old code was correct, but let's make things more explicit. --- src/resolve/resolved-dns-stub.c | 30 ++++++++++++++---------------- 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/src/resolve/resolved-dns-stub.c b/src/resolve/resolved-dns-stub.c index d9e6218081..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,25 +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_put(&s->queries, &trivial_hash_ops, q); + r = set_ensure_put(&s->queries, NULL, q); if (r < 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) { From ab9dd0b9975c69953cb73d6c04f1a6af0bb9d5fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 5 Jun 2020 14:03:30 +0200 Subject: [PATCH 12/20] resolved: fix memleak on duplicate host lines in /etc/hosts --- src/resolve/resolved-etc-hosts.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/resolve/resolved-etc-hosts.c b/src/resolve/resolved-etc-hosts.c index a2c84c17a4..6a7f749957 100644 --- a/src/resolve/resolved-etc-hosts.c +++ b/src/resolve/resolved-etc-hosts.c @@ -120,11 +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_put(&hosts->no_address, &dns_name_hash_ops, name); + r = set_ensure_consume(&hosts->no_address, &dns_name_hash_ops, TAKE_PTR(name)); if (r < 0) return r; - TAKE_PTR(name); continue; } From d378ef708b123b5da9ce2a52ea9e8a838f63bddb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 5 Jun 2020 14:06:02 +0200 Subject: [PATCH 13/20] portable: rework set handling in portable_detach() _cleanup_set_free_ is enough for unit_files, because unit_files is allocated in set_put_strdup(), which uses string_hash_ops_free. This fixes a leak if marker was already present in the table. --- src/portable/portable.c | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/src/portable/portable.c b/src/portable/portable.c index 57c224decd..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,10 +1189,8 @@ int portable_detach( if (path_is_absolute(marker) && !image_in_search_path(IMAGE_PORTABLE, marker)) { - r = set_ensure_put(&markers, &path_hash_ops, marker); - if (r >= 0) - marker = NULL; - else if (r != -EEXIST) + r = set_ensure_consume(&markers, &path_hash_ops_free, TAKE_PTR(marker)); + if (r < 0) return r; } } From 648c339c49398e9f1961399a672759ee2c7a9758 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 5 Jun 2020 14:07:44 +0200 Subject: [PATCH 14/20] logind: fix possible memleak of message if the message was already in the set I'm not sure if it is actually possible to encounter this condition. But let's make the handling correct regardless. --- src/login/logind-brightness.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/login/logind-brightness.c b/src/login/logind-brightness.c index 5c0c55fc36..450ec32044 100644 --- a/src/login/logind-brightness.c +++ b/src/login/logind-brightness.c @@ -175,9 +175,8 @@ static int set_add_message(Set **set, sd_bus_message *message) { return r; r = set_ensure_put(set, &bus_message_hash_ops, message); - if (r < 0) + if (r <= 0) return r; - sd_bus_message_ref(message); return 1; From d02fd8b1c6e1eb5e6c587bf9098c55e7d01f99d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 5 Jun 2020 14:16:04 +0200 Subject: [PATCH 15/20] core/bpf-firewall: use the correct cleanup function On error, we'd just free the object, and not close the fd. While at it, let's use set_ensure_consume() to make sure we don't leak the object if it was already in the set. I'm not sure if that condition can be achieved. --- src/core/bpf-firewall.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/core/bpf-firewall.c b/src/core/bpf-firewall.c index a05ac8122d..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,10 +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_put(set, &filter_prog_hash_ops, 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; From 39dbd0c74c4a686e37602d38bfb8bb1d4bbc7a71 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 5 Jun 2020 14:24:57 +0200 Subject: [PATCH 16/20] networkd: take ref immediately after storing item in set I'm not sure if I understand the code correctly, but it seems that if storig in the second set failed, we'd return with the first set having no reference on the link object, and the link object could be freed in the future, leaving the set with a dangling reference. --- src/network/networkd-link.c | 4 ++-- src/network/networkd-manager.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c index baa1cd3f7f..c2fb21c736 100644 --- a/src/network/networkd-link.c +++ b/src/network/networkd-link.c @@ -3129,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; diff --git a/src/network/networkd-manager.c b/src/network/networkd-manager.c index 14a883aa30..08666fd92d 100644 --- a/src/network/networkd-manager.c +++ b/src/network/networkd-manager.c @@ -2312,12 +2312,12 @@ int manager_request_product_uuid(Manager *m, Link *link) { 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_put(&m->duids_requesting_uuid, NULL, duid); if (r < 0) return log_oom(); - - link_ref(link); } if (!m->bus || sd_bus_is_ready(m->bus) <= 0) { From 35e601d4f92110e7da0dc5c950250b424f4fdb11 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 5 Jun 2020 14:40:08 +0200 Subject: [PATCH 17/20] tree-wide: use set_ensure_consume() in various places No funtional change. --- src/network/networkd-ndisc.c | 18 ++++++------------ src/network/networkd-network.c | 4 +--- src/resolve/resolved-dns-trust-anchor.c | 4 +--- src/shared/userdb.c | 4 +--- 4 files changed, 9 insertions(+), 21 deletions(-) diff --git a/src/network/networkd-ndisc.c b/src/network/networkd-ndisc.c index beeb343756..7889bfb191 100644 --- a/src/network/networkd-ndisc.c +++ b/src/network/networkd-ndisc.c @@ -600,12 +600,11 @@ static int ndisc_router_process_rdnss(Link *link, sd_ndisc_router *rt) { .valid_until = time_now + lifetime * USEC_PER_SEC, }; - r = set_ensure_put(&link->ndisc_rdnss, &ndisc_rdnss_hash_ops, 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); } @@ -683,12 +682,11 @@ static void ndisc_router_process_dnssl(Link *link, sd_ndisc_router *rt) { s->valid_until = time_now + lifetime * USEC_PER_SEC; - r = set_ensure_put(&link->ndisc_dnssl, &ndisc_dnssl_hash_ops, s); + r = set_ensure_consume(&link->ndisc_dnssl, &ndisc_dnssl_hash_ops, TAKE_PTR(s)); if (r < 0) { log_oom(); return; } - TAKE_PTR(s); assert(r > 0); link_dirty(link); @@ -972,13 +970,9 @@ int config_parse_ndisc_black_listed_prefix( if (!a) return log_oom(); - r = set_ensure_put(&network->ndisc_black_listed_prefix, &in6_addr_hash_ops, 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-network.c b/src/network/networkd-network.c index 01f08a5552..7fa359a542 100644 --- a/src/network/networkd-network.c +++ b/src/network/networkd-network.c @@ -1268,11 +1268,9 @@ int config_parse_dnssec_negative_trust_anchors( continue; } - r = set_ensure_put(&n->dnssec_negative_trust_anchors, &dns_name_hash_ops, w); + r = set_ensure_consume(&n->dnssec_negative_trust_anchors, &dns_name_hash_ops, TAKE_PTR(w)); if (r < 0) return log_oom(); - if (r > 0) - TAKE_PTR(w); } return 0; diff --git a/src/resolve/resolved-dns-trust-anchor.c b/src/resolve/resolved-dns-trust-anchor.c index 1cb7492905..d68d0c3ba1 100644 --- a/src/resolve/resolved-dns-trust-anchor.c +++ b/src/resolve/resolved-dns-trust-anchor.c @@ -393,11 +393,9 @@ static int dns_trust_anchor_load_negative(DnsTrustAnchor *d, const char *path, u return -EINVAL; } - r = set_ensure_put(&d->negative_by_name, &dns_name_hash_ops, 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; } diff --git a/src/shared/userdb.c b/src/shared/userdb.c index f3b05a7ad3..cd9873c06e 100644 --- a/src/shared/userdb.c +++ b/src/shared/userdb.c @@ -359,11 +359,9 @@ static int userdb_connect( if (r < 0) return log_debug_errno(r, "Failed to invoke varlink method: %m"); - r = set_ensure_put(&iterator->links, &link_hash_ops, 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; } From add74e8929d522d7b74ea3f4890da391a3be83df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 5 Jun 2020 14:55:41 +0200 Subject: [PATCH 18/20] basic/hashmap,set: propagate allocation location info in _copy() Also use double space before the tracking args at the end. Without the comma this looks ugly, but it's a bit better with the double space. At least it doesn't look like a variable with a type. --- src/basic/hashmap.c | 28 +++++++++++++--------------- src/basic/hashmap.h | 10 +++------- src/basic/set.h | 4 +--- 3 files changed, 17 insertions(+), 25 deletions(-) diff --git a/src/basic/hashmap.c b/src/basic/hashmap.c index b98696a136..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) { @@ -1711,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; @@ -1733,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; } diff --git a/src/basic/hashmap.h b/src/basic/hashmap.h index bb11069508..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); diff --git a/src/basic/set.h b/src/basic/set.h index 696de8ad57..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) From 80ce54adaf040dc1bcaf755fbfc2431ab0a4bc82 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 5 Jun 2020 15:53:44 +0200 Subject: [PATCH 19/20] socket-proxy: use structured initialization in one place --- src/socket-proxy/socket-proxyd.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/socket-proxy/socket-proxyd.c b/src/socket-proxy/socket-proxyd.c index 3b7c5799c8..b461aead60 100644 --- a/src/socket-proxy/socket-proxyd.c +++ b/src/socket-proxy/socket-proxyd.c @@ -480,17 +480,19 @@ static int add_connection_socket(Context *context, int fd) { log_warning_errno(r, "Unable to disable idle timer, continuing: %m"); } - 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_ensure_put(&context->connections, NULL, c); if (r < 0) { From e198eba7c231b7ab9883e81d841631d7a235be84 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 5 Jun 2020 15:53:55 +0200 Subject: [PATCH 20/20] sysv-generator: reduce scope of variables --- src/sysv-generator/sysv-generator.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/sysv-generator/sysv-generator.c b/src/sysv-generator/sysv-generator.c index 84f329e37d..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; @@ -892,9 +890,8 @@ static int set_dependencies_from_rcnd(const LookupPaths *lp, Hashmap *all_servic } } } - } - 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) { @@ -911,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;