From 64a65bab590f79f6520756d04e2a820a389a3209 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 9 Mar 2021 17:13:20 +0100 Subject: [PATCH 1/3] sd-netlink: use structured initialization The casts look somewhat ugly and type-unsafe, but they are equivalent to what was there before (we initialized a variable from a void*). --- src/libsystemd/sd-netlink/generic-netlink.c | 9 +++++---- src/libsystemd/sd-netlink/nfnl-message.c | 20 ++++++++++---------- 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/src/libsystemd/sd-netlink/generic-netlink.c b/src/libsystemd/sd-netlink/generic-netlink.c index 9710ed0f16..cd5a0104a6 100644 --- a/src/libsystemd/sd-netlink/generic-netlink.c +++ b/src/libsystemd/sd-netlink/generic-netlink.c @@ -32,7 +32,6 @@ static int genl_message_new(sd_netlink *nl, sd_genl_family_t family, uint16_t nl _cleanup_(sd_netlink_message_unrefp) sd_netlink_message *m = NULL; const NLType *genl_cmd_type, *nl_type; const NLTypeSystem *type_system; - struct genlmsghdr *genl; size_t size; int r; @@ -63,9 +62,11 @@ static int genl_message_new(sd_netlink *nl, sd_genl_family_t family, uint16_t nl m->hdr->nlmsg_type = nlmsg_type; type_get_type_system(nl_type, &m->containers[0].type_system); - genl = NLMSG_DATA(m->hdr); - genl->cmd = cmd; - genl->version = genl_families[family].version; + + *(struct genlmsghdr *) NLMSG_DATA(m->hdr) = (struct genlmsghdr) { + .cmd = cmd, + .version = genl_families[family].version, + }; *ret = TAKE_PTR(m); diff --git a/src/libsystemd/sd-netlink/nfnl-message.c b/src/libsystemd/sd-netlink/nfnl-message.c index 47e107a313..5f669f750b 100644 --- a/src/libsystemd/sd-netlink/nfnl-message.c +++ b/src/libsystemd/sd-netlink/nfnl-message.c @@ -19,7 +19,6 @@ static int nft_message_new(sd_netlink *nfnl, sd_netlink_message **ret, int family, uint16_t type, uint16_t flags) { _cleanup_(sd_netlink_message_unrefp) sd_netlink_message *m = NULL; - struct nfgenmsg *nfh; const NLType *nl_type; size_t size; int r; @@ -57,10 +56,11 @@ static int nft_message_new(sd_netlink *nfnl, sd_netlink_message **ret, int famil m->hdr->nlmsg_len = size; m->hdr->nlmsg_type = NFNL_SUBSYS_NFTABLES << 8 | type; - nfh = NLMSG_DATA(m->hdr); - nfh->nfgen_family = family; - nfh->version = NFNETLINK_V0; - nfh->res_id = nfnl->serial; + *(struct nfgenmsg*) NLMSG_DATA(m->hdr) = (struct nfgenmsg) { + .nfgen_family = family, + .version = NFNETLINK_V0, + .res_id = nfnl->serial, + }; *ret = TAKE_PTR(m); return 0; @@ -68,17 +68,17 @@ static int nft_message_new(sd_netlink *nfnl, sd_netlink_message **ret, int famil static int sd_nfnl_message_batch(sd_netlink *nfnl, sd_netlink_message **ret, int v) { _cleanup_(sd_netlink_message_unrefp) sd_netlink_message *m = NULL; - struct nfgenmsg *nfh; int r; r = message_new(nfnl, &m, v); if (r < 0) return r; - nfh = NLMSG_DATA(m->hdr); - nfh->nfgen_family = AF_UNSPEC; - nfh->version = NFNETLINK_V0; - nfh->res_id = NFNL_SUBSYS_NFTABLES; + *(struct nfgenmsg*) NLMSG_DATA(m->hdr) = (struct nfgenmsg) { + .nfgen_family = AF_UNSPEC, + .version = NFNETLINK_V0, + .res_id = NFNL_SUBSYS_NFTABLES, + }; *ret = TAKE_PTR(m); return r; From 4fa024683c19259be356f5602124b1208b9d0f2d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 9 Mar 2021 17:24:57 +0100 Subject: [PATCH 2/3] sd-netlink: use setsockopt_int() also for NETLINK_ADD/DROP_MEMBERSHIP We use 'unsigned' as the type, but netlink(7) says the type is 'int'. It doesn't really matter, since they are both the same size. Let's use our helper to shorten the code a bit. --- src/libsystemd/sd-netlink/netlink-socket.c | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/src/libsystemd/sd-netlink/netlink-socket.c b/src/libsystemd/sd-netlink/netlink-socket.c index 9e8dff1a72..05d6d3dafa 100644 --- a/src/libsystemd/sd-netlink/netlink-socket.c +++ b/src/libsystemd/sd-netlink/netlink-socket.c @@ -130,17 +130,12 @@ static int broadcast_group_set_ref(sd_netlink *nl, unsigned group, unsigned n_re } static int broadcast_group_join(sd_netlink *nl, unsigned group) { - int r; - assert(nl); assert(nl->fd >= 0); assert(group > 0); - r = setsockopt(nl->fd, SOL_NETLINK, NETLINK_ADD_MEMBERSHIP, &group, sizeof(group)); - if (r < 0) - return -errno; - - return 0; + /* group is "unsigned", but netlink(7) says the argument for NETLINK_ADD_MEMBERSHIP is "int" */ + return setsockopt_int(nl->fd, SOL_NETLINK, NETLINK_ADD_MEMBERSHIP, group); } int socket_broadcast_group_ref(sd_netlink *nl, unsigned group) { @@ -173,8 +168,6 @@ int socket_broadcast_group_ref(sd_netlink *nl, unsigned group) { } static int broadcast_group_leave(sd_netlink *nl, unsigned group) { - int r; - assert(nl); assert(nl->fd >= 0); assert(group > 0); @@ -182,11 +175,8 @@ static int broadcast_group_leave(sd_netlink *nl, unsigned group) { if (nl->broadcast_group_dont_leave) return 0; - r = setsockopt(nl->fd, SOL_NETLINK, NETLINK_DROP_MEMBERSHIP, &group, sizeof(group)); - if (r < 0) - return -errno; - - return 0; + /* group is "unsigned", but netlink(7) says the argument for NETLINK_DROP_MEMBERSHIP is "int" */ + return setsockopt_int(nl->fd, SOL_NETLINK, NETLINK_DROP_MEMBERSHIP, group); } int socket_broadcast_group_unref(sd_netlink *nl, unsigned group) { From c7209bcfe197f55bc16a327a791a007cf64ec4e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 9 Mar 2021 17:26:01 +0100 Subject: [PATCH 3/3] sd-netlink: shorten things a bit --- src/libsystemd/sd-netlink/netlink-message.c | 9 ++-- src/libsystemd/sd-netlink/netlink-socket.c | 48 +++++++-------------- 2 files changed, 19 insertions(+), 38 deletions(-) diff --git a/src/libsystemd/sd-netlink/netlink-message.c b/src/libsystemd/sd-netlink/netlink-message.c index 34c74f99dc..90915fd155 100644 --- a/src/libsystemd/sd-netlink/netlink-message.c +++ b/src/libsystemd/sd-netlink/netlink-message.c @@ -144,7 +144,6 @@ static int add_rtattr(sd_netlink_message *m, unsigned short type, const void *da size_t message_length; struct nlmsghdr *new_hdr; struct rtattr *rta; - unsigned i; int offset; assert(m); @@ -172,7 +171,7 @@ static int add_rtattr(sd_netlink_message *m, unsigned short type, const void *da rtattr_append_attribute_internal(rta, type, data, data_length); /* if we are inside containers, extend them */ - for (i = 0; i < m->n_containers; i++) + for (unsigned i = 0; i < m->n_containers; i++) GET_CONTAINER(m, i)->rta_len += RTA_SPACE(data_length); /* update message size */ @@ -643,7 +642,6 @@ int sd_netlink_message_open_array(sd_netlink_message *m, uint16_t type) { } int sd_netlink_message_cancel_array(sd_netlink_message *m) { - unsigned i; uint32_t rta_len; assert_return(m, -EINVAL); @@ -652,7 +650,7 @@ int sd_netlink_message_cancel_array(sd_netlink_message *m) { rta_len = GET_CONTAINER(m, (m->n_containers - 1))->rta_len; - for (i = 0; i < m->n_containers; i++) + for (unsigned i = 0; i < m->n_containers; i++) GET_CONTAINER(m, i)->rta_len -= rta_len; m->hdr->nlmsg_len -= rta_len; @@ -1283,7 +1281,6 @@ int sd_netlink_message_rewind(sd_netlink_message *m, sd_netlink *genl) { const NLType *nl_type; uint16_t type; size_t size; - unsigned i; int r; assert_return(m, -EINVAL); @@ -1293,7 +1290,7 @@ int sd_netlink_message_rewind(sd_netlink_message *m, sd_netlink *genl) { if (!m->sealed) rtnl_message_seal(m); - for (i = 1; i <= m->n_containers; i++) + for (unsigned i = 1; i <= m->n_containers; i++) m->containers[i].attributes = mfree(m->containers[i].attributes); m->n_containers = 0; diff --git a/src/libsystemd/sd-netlink/netlink-socket.c b/src/libsystemd/sd-netlink/netlink-socket.c index 05d6d3dafa..7af9a94e3e 100644 --- a/src/libsystemd/sd-netlink/netlink-socket.c +++ b/src/libsystemd/sd-netlink/netlink-socket.c @@ -29,7 +29,6 @@ int socket_open(int family) { static int broadcast_groups_get(sd_netlink *nl) { _cleanup_free_ uint32_t *groups = NULL; socklen_t len = 0, old_len; - unsigned i, j; int r; assert(nl); @@ -37,11 +36,11 @@ static int broadcast_groups_get(sd_netlink *nl) { r = getsockopt(nl->fd, SOL_NETLINK, NETLINK_LIST_MEMBERSHIPS, NULL, &len); if (r < 0) { - if (errno == ENOPROTOOPT) { - nl->broadcast_group_dont_leave = true; - return 0; - } else + if (errno != ENOPROTOOPT) return -errno; + + nl->broadcast_group_dont_leave = true; + return 0; } if (len == 0) @@ -64,23 +63,15 @@ static int broadcast_groups_get(sd_netlink *nl) { if (r < 0) return r; - for (i = 0; i < len; i++) { - for (j = 0; j < sizeof(uint32_t) * 8; j++) { - uint32_t offset; - unsigned group; + for (unsigned i = 0; i < len; i++) + for (unsigned j = 0; j < sizeof(uint32_t) * 8; j++) + if (groups[i] & (1U << j)) { + unsigned group = i * sizeof(uint32_t) * 8 + j + 1; - offset = 1U << j; - - if (!(groups[i] & offset)) - continue; - - group = i * sizeof(uint32_t) * 8 + j + 1; - - r = hashmap_put(nl->broadcast_group_refs, UINT_TO_PTR(group), UINT_TO_PTR(1)); - if (r < 0) - return r; - } - } + r = hashmap_put(nl->broadcast_group_refs, UINT_TO_PTR(group), UINT_TO_PTR(1)); + if (r < 0) + return r; + } return 0; } @@ -104,11 +95,7 @@ int socket_bind(sd_netlink *nl) { if (r < 0) return -errno; - r = broadcast_groups_get(nl); - if (r < 0) - return r; - - return 0; + return broadcast_groups_get(nl); } static unsigned broadcast_group_get_ref(sd_netlink *nl, unsigned group) { @@ -230,8 +217,6 @@ int socket_write_message(sd_netlink *nl, sd_netlink_message *m) { int socket_writev_message(sd_netlink *nl, sd_netlink_message **m, size_t msgcount) { _cleanup_free_ struct iovec *iovs = NULL; - ssize_t k; - size_t i; assert(nl); assert(m); @@ -241,13 +226,13 @@ int socket_writev_message(sd_netlink *nl, sd_netlink_message **m, size_t msgcoun if (!iovs) return -ENOMEM; - for (i = 0; i < msgcount; i++) { + for (size_t i = 0; i < msgcount; i++) { assert(m[i]->hdr != NULL); assert(m[i]->hdr->nlmsg_len > 0); iovs[i] = IOVEC_MAKE(m[i]->hdr, m[i]->hdr->nlmsg_len); } - k = writev(nl->fd, iovs, msgcount); + ssize_t k = writev(nl->fd, iovs, msgcount); if (k < 0) return -errno; @@ -315,7 +300,6 @@ int socket_read_message(sd_netlink *rtnl) { struct iovec iov = {}; uint32_t group = 0; bool multi_part = false, done = false; - struct nlmsghdr *new_msg; size_t len; int r; unsigned i = 0; @@ -362,7 +346,7 @@ int socket_read_message(sd_netlink *rtnl) { } } - for (new_msg = rtnl->rbuffer; NLMSG_OK(new_msg, len) && !done; new_msg = NLMSG_NEXT(new_msg, len)) { + for (struct nlmsghdr *new_msg = rtnl->rbuffer; NLMSG_OK(new_msg, len) && !done; new_msg = NLMSG_NEXT(new_msg, len)) { _cleanup_(sd_netlink_message_unrefp) sd_netlink_message *m = NULL; const NLType *nl_type;