From 597da51bae9e46a93b310125174d48c9438d3bab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 18 Dec 2019 13:54:13 +0100 Subject: [PATCH 01/11] tree-wide: make parse_ifindex simply return the index We don't need a seperate output parameter that is of type int. glibc() says that the type is "unsigned", but the kernel thinks it's "int". And the "alternative names" interface also uses ints. So let's standarize on ints, since it's clearly not realisitic to have interface numbers in the upper half of unsigned int range. --- src/basic/in-addr-util.c | 32 +++++++++----------------- src/basic/parse-util.c | 17 ++++++-------- src/basic/parse-util.h | 4 ++-- src/libsystemd/sd-device/sd-device.c | 16 ++++++------- src/libsystemd/sd-login/sd-login.c | 3 ++- src/libsystemd/sd-network/sd-network.c | 11 ++++----- src/machine/machine.c | 14 +++++------ src/network/networkctl.c | 12 +++++----- src/network/networkd-link-bus.c | 4 ++-- src/network/networkd-route.c | 3 ++- src/nspawn/nspawn-network.c | 14 +++++------ src/resolve/resolvectl.c | 20 ++++++++-------- src/resolve/resolved-link-bus.c | 7 +++--- src/resolve/resolved-manager.c | 5 ++-- src/timedate/timedatectl.c | 7 ++++-- 15 files changed, 77 insertions(+), 92 deletions(-) diff --git a/src/basic/in-addr-util.c b/src/basic/in-addr-util.c index 06b92db579..b6ca27e3e8 100644 --- a/src/basic/in-addr-util.c +++ b/src/basic/in-addr-util.c @@ -439,48 +439,38 @@ int in_addr_from_string_auto(const char *s, int *ret_family, union in_addr_union return -EINVAL; } -int in_addr_ifindex_from_string_auto(const char *s, int *family, union in_addr_union *ret, int *ifindex) { +int in_addr_ifindex_from_string_auto(const char *s, int *family, union in_addr_union *ret_addr, int *ret_ifindex) { _cleanup_free_ char *buf = NULL; const char *suffix; - int r, ifi = 0; + int r, ifindex = 0; assert(s); assert(family); - assert(ret); + assert(ret_addr); /* Similar to in_addr_from_string_auto() but also parses an optionally appended IPv6 zone suffix ("scope id") * if one is found. */ suffix = strchr(s, '%'); if (suffix) { - - if (ifindex) { + if (ret_ifindex) { /* If we shall return the interface index, try to parse it */ - r = parse_ifindex(suffix + 1, &ifi); - if (r < 0) { - unsigned u; - - u = if_nametoindex(suffix + 1); - if (u <= 0) - return -errno; - - ifi = (int) u; - } + ifindex = parse_ifindex_or_ifname(suffix + 1); + if (ifindex < 0) + return ifindex; } - buf = strndup(s, suffix - s); + s = buf = strndup(s, suffix - s); if (!buf) return -ENOMEM; - - s = buf; } - r = in_addr_from_string_auto(s, family, ret); + r = in_addr_from_string_auto(s, family, ret_addr); if (r < 0) return r; - if (ifindex) - *ifindex = ifi; + if (ret_ifindex) + *ret_ifindex = ifindex; return r; } diff --git a/src/basic/parse-util.c b/src/basic/parse-util.c index b81db04989..d3140cc1a4 100644 --- a/src/basic/parse-util.c +++ b/src/basic/parse-util.c @@ -79,11 +79,10 @@ int parse_mode(const char *s, mode_t *ret) { return 0; } -int parse_ifindex(const char *s, int *ret) { +int parse_ifindex(const char *s) { int ifi, r; assert(s); - assert(ret); r = safe_atoi(s, &ifi); if (r < 0) @@ -91,26 +90,24 @@ int parse_ifindex(const char *s, int *ret) { if (ifi <= 0) return -EINVAL; - *ret = ifi; - return 0; + return ifi; } -int parse_ifindex_or_ifname(const char *s, int *ret) { +int parse_ifindex_or_ifname(const char *s) { int r; assert(s); - assert(ret); - r = parse_ifindex(s, ret); - if (r >= 0) + r = parse_ifindex(s); + if (r > 0) return r; + assert(r < 0); r = (int) if_nametoindex(s); if (r <= 0) return -errno; - *ret = r; - return 0; + return r; } int parse_mtu(int family, const char *s, uint32_t *ret) { diff --git a/src/basic/parse-util.h b/src/basic/parse-util.h index 3a70b79276..b20f5b52cc 100644 --- a/src/basic/parse-util.h +++ b/src/basic/parse-util.h @@ -13,8 +13,8 @@ int parse_boolean(const char *v) _pure_; int parse_dev(const char *s, dev_t *ret); int parse_pid(const char *s, pid_t* ret_pid); int parse_mode(const char *s, mode_t *ret); -int parse_ifindex(const char *s, int *ret); -int parse_ifindex_or_ifname(const char *s, int *ret); +int parse_ifindex(const char *s); +int parse_ifindex_or_ifname(const char *s); int parse_mtu(int family, const char *s, uint32_t *ret); int parse_size(const char *t, uint64_t base, uint64_t *size); diff --git a/src/libsystemd/sd-device/sd-device.c b/src/libsystemd/sd-device/sd-device.c index f35612fe12..1f2451f8e1 100644 --- a/src/libsystemd/sd-device/sd-device.c +++ b/src/libsystemd/sd-device/sd-device.c @@ -340,17 +340,17 @@ int device_set_devtype(sd_device *device, const char *_devtype) { return 0; } -int device_set_ifindex(sd_device *device, const char *_ifindex) { - int ifindex, r; +int device_set_ifindex(sd_device *device, const char *name) { + int r, ifindex; assert(device); - assert(_ifindex); + assert(name); - r = parse_ifindex(_ifindex, &ifindex); - if (r < 0) - return r; + ifindex = parse_ifindex(name); + if (ifindex < 0) + return ifindex; - r = device_add_property_internal(device, "IFINDEX", _ifindex); + r = device_add_property_internal(device, "IFINDEX", name); if (r < 0) return r; @@ -619,7 +619,7 @@ _public_ int sd_device_new_from_device_id(sd_device **ret, const char *id) { struct ifreq ifr = {}; int ifindex; - r = parse_ifindex(&id[1], &ifr.ifr_ifindex); + r = ifr.ifr_ifindex = parse_ifindex(&id[1]); if (r < 0) return r; diff --git a/src/libsystemd/sd-login/sd-login.c b/src/libsystemd/sd-login/sd-login.c index fbfd89b69b..3e96c98cd9 100644 --- a/src/libsystemd/sd-login/sd-login.c +++ b/src/libsystemd/sd-login/sd-login.c @@ -928,7 +928,8 @@ _public_ int sd_machine_get_ifindices(const char *machine, int **ifindices) { *(char*) (mempcpy(buf, word, l)) = 0; - if (parse_ifindex(buf, &ifi) < 0) + ifi = parse_ifindex(buf); + if (ifi < 0) continue; if (!GREEDY_REALLOC(ni, allocated, nr+1)) { diff --git a/src/libsystemd/sd-network/sd-network.c b/src/libsystemd/sd-network/sd-network.c index dc5ccb793b..b9b618e585 100644 --- a/src/libsystemd/sd-network/sd-network.c +++ b/src/libsystemd/sd-network/sd-network.c @@ -276,7 +276,6 @@ static int network_link_get_ifindexes(int ifindex, const char *key, int **ret) { _cleanup_free_ int *ifis = NULL; _cleanup_free_ char *s = NULL; size_t allocated = 0, c = 0; - const char *x; int r; assert_return(ifindex > 0, -EINVAL); @@ -289,7 +288,7 @@ static int network_link_get_ifindexes(int ifindex, const char *key, int **ret) { if (r < 0) return r; - for (x = s;;) { + for (const char *x = s;;) { _cleanup_free_ char *word = NULL; r = extract_first_word(&x, &word, NULL, 0); @@ -298,14 +297,12 @@ static int network_link_get_ifindexes(int ifindex, const char *key, int **ret) { if (r == 0) break; - r = parse_ifindex(word, &ifindex); - if (r < 0) - return r; - if (!GREEDY_REALLOC(ifis, allocated, c + 2)) return -ENOMEM; - ifis[c++] = ifindex; + r = ifis[c++] = parse_ifindex(word); + if (r < 0) + return r; } if (ifis) diff --git a/src/machine/machine.c b/src/machine/machine.c index efe327f381..8154c42ed7 100644 --- a/src/machine/machine.c +++ b/src/machine/machine.c @@ -294,12 +294,11 @@ int machine_load(Machine *m) { if (netif) { size_t allocated = 0, nr = 0; const char *p; - int *ni = NULL; + _cleanup_free_ int *ni = NULL; p = netif; for (;;) { _cleanup_free_ char *word = NULL; - int ifi; r = extract_first_word(&p, &word, NULL, 0); if (r == 0) @@ -311,19 +310,18 @@ int machine_load(Machine *m) { break; } - if (parse_ifindex(word, &ifi) < 0) + r = parse_ifindex(word); + if (r < 0) continue; - if (!GREEDY_REALLOC(ni, allocated, nr+1)) { - free(ni); + if (!GREEDY_REALLOC(ni, allocated, nr + 1)) return log_oom(); - } - ni[nr++] = ifi; + ni[nr++] = r; } free(m->netif); - m->netif = ni; + m->netif = TAKE_PTR(ni); m->n_netif = nr; } diff --git a/src/network/networkctl.c b/src/network/networkctl.c index e6dc70a0e2..0cc018e088 100644 --- a/src/network/networkctl.c +++ b/src/network/networkctl.c @@ -1880,8 +1880,8 @@ static int link_delete(int argc, char *argv[], void *userdata) { return log_oom(); for (i = 1; i < argc; i++) { - r = parse_ifindex_or_ifname(argv[i], &index); - if (r < 0) { + index = parse_ifindex_or_ifname(argv[i]); + if (index < 0) { r = rtnl_resolve_link_alternative_name(&rtnl, argv[i], &index); if (r < 0) return log_error_errno(r, "Failed to resolve interface %s", argv[i]); @@ -1936,8 +1936,8 @@ static int link_renew(int argc, char *argv[], void *userdata) { return log_error_errno(r, "Failed to connect system bus: %m"); for (i = 1; i < argc; i++) { - r = parse_ifindex_or_ifname(argv[i], &index); - if (r < 0) { + index = parse_ifindex_or_ifname(argv[i]); + if (index < 0) { r = rtnl_resolve_link_alternative_name(&rtnl, argv[i], &index); if (r < 0) return log_error_errno(r, "Failed to resolve interface %s", argv[i]); @@ -1991,8 +1991,8 @@ static int verb_reconfigure(int argc, char *argv[], void *userdata) { return log_oom(); for (i = 1; i < argc; i++) { - r = parse_ifindex_or_ifname(argv[i], &index); - if (r < 0) { + index = parse_ifindex_or_ifname(argv[i]); + if (index < 0) { r = rtnl_resolve_link_alternative_name(&rtnl, argv[i], &index); if (r < 0) return log_error_errno(r, "Failed to resolve interface %s", argv[i]); diff --git a/src/network/networkd-link-bus.c b/src/network/networkd-link-bus.c index 356ceaf063..8f3b2e92f8 100644 --- a/src/network/networkd-link-bus.c +++ b/src/network/networkd-link-bus.c @@ -715,8 +715,8 @@ int link_object_find(sd_bus *bus, const char *path, const char *interface, void if (r <= 0) return 0; - r = parse_ifindex(identifier, &ifindex); - if (r < 0) + ifindex = parse_ifindex(identifier); + if (ifindex < 0) return 0; r = link_get(m, ifindex, &link); diff --git a/src/network/networkd-route.c b/src/network/networkd-route.c index f94bee6d8b..2223a356da 100644 --- a/src/network/networkd-route.c +++ b/src/network/networkd-route.c @@ -1646,12 +1646,13 @@ int config_parse_multipath_route( m->gateway.family = family; if (dev) { - r = parse_ifindex_or_ifname(dev, &m->ifindex); + r = parse_ifindex_or_ifname(dev); if (r < 0) { log_syntax(unit, LOG_ERR, filename, line, r, "Invalid interface name or index, ignoring assignment: %s", dev); return 0; } + m->ifindex = r; } if (!isempty(p)) { diff --git a/src/nspawn/nspawn-network.c b/src/nspawn/nspawn-network.c index 9c9cf4812e..b2592b023d 100644 --- a/src/nspawn/nspawn-network.c +++ b/src/nspawn/nspawn-network.c @@ -337,9 +337,9 @@ static int join_bridge(sd_netlink *rtnl, const char *veth_name, const char *brid assert(veth_name); assert(bridge_name); - r = parse_ifindex_or_ifname(bridge_name, &bridge_ifi); - if (r < 0) - return r; + bridge_ifi = parse_ifindex_or_ifname(bridge_name); + if (bridge_ifi < 0) + return bridge_ifi; r = sd_rtnl_message_new_link(rtnl, &m, RTM_SETLINK, 0); if (r < 0) @@ -473,11 +473,11 @@ int remove_bridge(const char *bridge_name) { } static int parse_interface(const char *name) { - int ifi, r; + int ifi; - r = parse_ifindex_or_ifname(name, &ifi); - if (r < 0) - return log_error_errno(r, "Failed to resolve interface %s: %m", name); + ifi = parse_ifindex_or_ifname(name); + if (ifi < 0) + return log_error_errno(ifi, "Failed to resolve interface %s: %m", name); return ifi; } diff --git a/src/resolve/resolvectl.c b/src/resolve/resolvectl.c index 0a96a18b38..87ecd519d5 100644 --- a/src/resolve/resolvectl.c +++ b/src/resolve/resolvectl.c @@ -80,7 +80,7 @@ typedef enum StatusMode { int ifname_mangle(const char *s) { _cleanup_free_ char *iface = NULL; const char *dot; - int ifi, r; + int ifi; assert(s); @@ -94,14 +94,14 @@ int ifname_mangle(const char *s) { if (!iface) return log_oom(); - r = parse_ifindex_or_ifname(iface, &ifi); - if (r < 0) { - if (r == -ENODEV && arg_ifindex_permissive) { + ifi = parse_ifindex_or_ifname(iface); + if (ifi < 0) { + if (ifi == -ENODEV && arg_ifindex_permissive) { log_debug("Interface '%s' not found, but -f specified, ignoring.", iface); return 0; /* done */ } - return log_error_errno(r, "Unknown interface '%s': %m", iface); + return log_error_errno(ifi, "Unknown interface '%s': %m", iface); } if (arg_ifindex > 0 && arg_ifindex != ifi) @@ -1819,18 +1819,18 @@ static int status_all(sd_bus *bus, StatusMode mode) { static int verb_status(int argc, char **argv, void *userdata) { sd_bus *bus = userdata; - int q, r = 0; + int r = 0; if (argc > 1) { char **ifname; bool empty_line = false; STRV_FOREACH(ifname, argv + 1) { - int ifindex; + int ifindex, q; - q = parse_ifindex_or_ifname(*ifname, &ifindex); - if (q < 0) { - log_error_errno(q, "Unknown interface '%s', ignoring: %m", *ifname); + ifindex = parse_ifindex_or_ifname(*ifname); + if (ifindex < 0) { + log_warning_errno(ifindex, "Unknown interface '%s', ignoring: %m", *ifname); continue; } diff --git a/src/resolve/resolved-link-bus.c b/src/resolve/resolved-link-bus.c index dae8435b45..a8480f190a 100644 --- a/src/resolve/resolved-link-bus.c +++ b/src/resolve/resolved-link-bus.c @@ -712,9 +712,8 @@ const sd_bus_vtable link_vtable[] = { int link_object_find(sd_bus *bus, const char *path, const char *interface, void *userdata, void **found, sd_bus_error *error) { _cleanup_free_ char *e = NULL; Manager *m = userdata; - int ifindex; Link *link; - int r; + int ifindex, r; assert(bus); assert(path); @@ -726,8 +725,8 @@ int link_object_find(sd_bus *bus, const char *path, const char *interface, void if (r <= 0) return 0; - r = parse_ifindex(e, &ifindex); - if (r < 0) + ifindex = parse_ifindex(e); + if (ifindex < 0) return 0; link = hashmap_get(m->links, INT_TO_PTR(ifindex)); diff --git a/src/resolve/resolved-manager.c b/src/resolve/resolved-manager.c index 95f698ab06..af91a8ec1a 100644 --- a/src/resolve/resolved-manager.c +++ b/src/resolve/resolved-manager.c @@ -1466,7 +1466,6 @@ void manager_reset_server_features(Manager *m) { void manager_cleanup_saved_user(Manager *m) { _cleanup_closedir_ DIR *d = NULL; struct dirent *de; - int r; assert(m); @@ -1494,8 +1493,8 @@ void manager_cleanup_saved_user(Manager *m) { if (dot_or_dot_dot(de->d_name)) continue; - r = parse_ifindex(de->d_name, &ifindex); - if (r < 0) /* Probably some temporary file from a previous run. Delete it */ + ifindex = parse_ifindex(de->d_name); + if (ifindex < 0) /* Probably some temporary file from a previous run. Delete it */ goto rm; l = hashmap_get(m->links, INT_TO_PTR(ifindex)); diff --git a/src/timedate/timedatectl.c b/src/timedate/timedatectl.c index 17297ab228..d51e29de1e 100644 --- a/src/timedate/timedatectl.c +++ b/src/timedate/timedatectl.c @@ -839,9 +839,12 @@ static int parse_ifindex_bus(sd_bus *bus, const char *str, int *ret) { assert(str); assert(ret); - r = parse_ifindex(str, ret); - if (r >= 0) + r = parse_ifindex(str); + if (r >= 0) { + assert(r != 0); + *ret = r; return 0; + } r = sd_bus_call_method( bus, From 9030b50a7bfe081f17d03f51c615def8485e937a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 18 Dec 2019 13:57:01 +0100 Subject: [PATCH 02/11] timedatectl: drop ifindex output parameter too --- src/timedate/timedatectl.c | 26 +++++++++++--------------- 1 file changed, 11 insertions(+), 15 deletions(-) diff --git a/src/timedate/timedatectl.c b/src/timedate/timedatectl.c index d51e29de1e..7d0201e6ff 100644 --- a/src/timedate/timedatectl.c +++ b/src/timedate/timedatectl.c @@ -829,7 +829,7 @@ static int show_timesync(int argc, char **argv, void *userdata) { return 0; } -static int parse_ifindex_bus(sd_bus *bus, const char *str, int *ret) { +static int parse_ifindex_bus(sd_bus *bus, const char *str) { _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; int32_t i; @@ -837,14 +837,11 @@ static int parse_ifindex_bus(sd_bus *bus, const char *str, int *ret) { assert(bus); assert(str); - assert(ret); r = parse_ifindex(str); - if (r >= 0) { - assert(r != 0); - *ret = r; - return 0; - } + if (r > 0) + return r; + assert(r < 0); r = sd_bus_call_method( bus, @@ -862,8 +859,7 @@ static int parse_ifindex_bus(sd_bus *bus, const char *str, int *ret) { if (r < 0) return bus_log_create_error(r); - *ret = i; - return 0; + return i; } static int verb_ntp_servers(int argc, char **argv, void *userdata) { @@ -874,9 +870,9 @@ static int verb_ntp_servers(int argc, char **argv, void *userdata) { assert(bus); - r = parse_ifindex_bus(bus, argv[1], &ifindex); - if (r < 0) - return r; + ifindex = parse_ifindex_bus(bus, argv[1]); + if (ifindex < 0) + return ifindex; polkit_agent_open_if_enabled(arg_transport, arg_ask_password); @@ -912,9 +908,9 @@ static int verb_revert(int argc, char **argv, void *userdata) { assert(bus); - r = parse_ifindex_bus(bus, argv[1], &ifindex); - if (r < 0) - return r; + ifindex = parse_ifindex_bus(bus, argv[1]); + if (ifindex < 0) + return ifindex; polkit_agent_open_if_enabled(arg_transport, arg_ask_password); From 231d9de1e3e92dc65ee3b7f98294367c0d291a19 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 18 Dec 2019 14:18:34 +0100 Subject: [PATCH 03/11] networkctl: define a helper for interface name resolution --- src/libsystemd/sd-netlink/netlink-util.c | 11 ++++--- src/libsystemd/sd-netlink/netlink-util.h | 2 +- src/network/networkctl.c | 42 ++++++++++++++---------- src/network/networkd-manager-bus.c | 4 +-- 4 files changed, 34 insertions(+), 25 deletions(-) diff --git a/src/libsystemd/sd-netlink/netlink-util.c b/src/libsystemd/sd-netlink/netlink-util.c index a91db5df5f..c9d4e3c607 100644 --- a/src/libsystemd/sd-netlink/netlink-util.c +++ b/src/libsystemd/sd-netlink/netlink-util.c @@ -167,13 +167,12 @@ int rtnl_set_link_alternative_names_by_ifname(sd_netlink **rtnl, const char *ifn return 0; } -int rtnl_resolve_link_alternative_name(sd_netlink **rtnl, const char *name, int *ret) { +int rtnl_resolve_link_alternative_name(sd_netlink **rtnl, const char *name) { _cleanup_(sd_netlink_message_unrefp) sd_netlink_message *message = NULL, *reply = NULL; - int r; + int r, ret; assert(rtnl); assert(name); - assert(ret); if (!*rtnl) { r = sd_netlink_open(rtnl); @@ -193,7 +192,11 @@ int rtnl_resolve_link_alternative_name(sd_netlink **rtnl, const char *name, int if (r < 0) return r; - return sd_rtnl_message_link_get_ifindex(reply, ret); + r = sd_rtnl_message_link_get_ifindex(reply, &ret); + if (r < 0) + return r; + assert(ret > 0); + return ret; } int rtnl_message_new_synthetic_error(sd_netlink *rtnl, int error, uint32_t serial, sd_netlink_message **ret) { diff --git a/src/libsystemd/sd-netlink/netlink-util.h b/src/libsystemd/sd-netlink/netlink-util.h index 7b007a61c2..55bc12712a 100644 --- a/src/libsystemd/sd-netlink/netlink-util.h +++ b/src/libsystemd/sd-netlink/netlink-util.h @@ -51,7 +51,7 @@ int rtnl_set_link_name(sd_netlink **rtnl, int ifindex, const char *name); int rtnl_set_link_properties(sd_netlink **rtnl, int ifindex, const char *alias, const struct ether_addr *mac, uint32_t mtu); int rtnl_set_link_alternative_names(sd_netlink **rtnl, int ifindex, char * const *alternative_names); int rtnl_set_link_alternative_names_by_ifname(sd_netlink **rtnl, const char *ifname, char * const *alternative_names); -int rtnl_resolve_link_alternative_name(sd_netlink **rtnl, const char *name, int *ret); +int rtnl_resolve_link_alternative_name(sd_netlink **rtnl, const char *name); int rtnl_log_parse_error(int r); int rtnl_log_create_error(int r); diff --git a/src/network/networkctl.c b/src/network/networkctl.c index 0cc018e088..5cc62841ec 100644 --- a/src/network/networkctl.c +++ b/src/network/networkctl.c @@ -64,6 +64,21 @@ static bool arg_stats = false; static bool arg_full = false; static unsigned arg_lines = 10; +static int resolve_ifname(sd_netlink **rtnl, const char *name) { + int r; + + r = parse_ifindex_or_ifname(name); + if (r > 0) + return r; + assert(r < 0); + + r = rtnl_resolve_link_alternative_name(rtnl, name); + if (r > 0) + return r; + assert(r < 0); + return log_error_errno(r, "Failed to resolve interface \"%s\": %m", name); +} + static char *link_get_type_string(unsigned short iftype, sd_device *d) { const char *t, *devtype; char *p; @@ -1880,12 +1895,9 @@ static int link_delete(int argc, char *argv[], void *userdata) { return log_oom(); for (i = 1; i < argc; i++) { - index = parse_ifindex_or_ifname(argv[i]); - if (index < 0) { - r = rtnl_resolve_link_alternative_name(&rtnl, argv[i], &index); - if (r < 0) - return log_error_errno(r, "Failed to resolve interface %s", argv[i]); - } + index = resolve_ifname(&rtnl, argv[i]); + if (index < 0) + return index; r = set_put(indexes, INT_TO_PTR(index)); if (r < 0) @@ -1936,12 +1948,9 @@ static int link_renew(int argc, char *argv[], void *userdata) { return log_error_errno(r, "Failed to connect system bus: %m"); for (i = 1; i < argc; i++) { - index = parse_ifindex_or_ifname(argv[i]); - if (index < 0) { - r = rtnl_resolve_link_alternative_name(&rtnl, argv[i], &index); - if (r < 0) - return log_error_errno(r, "Failed to resolve interface %s", argv[i]); - } + index = resolve_ifname(&rtnl, argv[i]); + if (index < 0) + return index; r = link_renew_one(bus, index, argv[i]); if (r < 0 && k >= 0) @@ -1991,12 +2000,9 @@ static int verb_reconfigure(int argc, char *argv[], void *userdata) { return log_oom(); for (i = 1; i < argc; i++) { - index = parse_ifindex_or_ifname(argv[i]); - if (index < 0) { - r = rtnl_resolve_link_alternative_name(&rtnl, argv[i], &index); - if (r < 0) - return log_error_errno(r, "Failed to resolve interface %s", argv[i]); - } + index = resolve_ifname(&rtnl, argv[i]); + if (index < 0) + return index; r = set_put(indexes, INT_TO_PTR(index)); if (r < 0) diff --git a/src/network/networkd-manager-bus.c b/src/network/networkd-manager-bus.c index 8bcce06b85..4db72b3302 100644 --- a/src/network/networkd-manager-bus.c +++ b/src/network/networkd-manager-bus.c @@ -68,8 +68,8 @@ static int method_get_link_by_name(sd_bus_message *message, void *userdata, sd_b index = if_nametoindex(name); if (index <= 0) { - r = rtnl_resolve_link_alternative_name(&manager->rtnl, name, &index); - if (r < 0) + index = rtnl_resolve_link_alternative_name(&manager->rtnl, name); + if (index < 0) return sd_bus_error_setf(error, BUS_ERROR_NO_SUCH_LINK, "Link %s not known", name); } From bad7cecc0aa8eafef1cf594c7eea45cbaadd194a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 10 Jan 2020 16:32:27 +0100 Subject: [PATCH 04/11] sd-netlink: do not require rtnl pointer to be passed --- src/libsystemd/sd-netlink/netlink-util.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/libsystemd/sd-netlink/netlink-util.c b/src/libsystemd/sd-netlink/netlink-util.c index c9d4e3c607..0b2d171b2d 100644 --- a/src/libsystemd/sd-netlink/netlink-util.c +++ b/src/libsystemd/sd-netlink/netlink-util.c @@ -168,12 +168,14 @@ int rtnl_set_link_alternative_names_by_ifname(sd_netlink **rtnl, const char *ifn } int rtnl_resolve_link_alternative_name(sd_netlink **rtnl, const char *name) { + _cleanup_(sd_netlink_unrefp) sd_netlink *our_rtnl = NULL; _cleanup_(sd_netlink_message_unrefp) sd_netlink_message *message = NULL, *reply = NULL; int r, ret; - assert(rtnl); assert(name); + if (!rtnl) + rtnl = &our_rtnl; if (!*rtnl) { r = sd_netlink_open(rtnl); if (r < 0) From 955bb7fac3bc4704b98b87206ec44e2ae464861b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 10 Jan 2020 16:51:29 +0100 Subject: [PATCH 05/11] basic/socket-util: indent for clarity --- src/basic/socket-util.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/basic/socket-util.c b/src/basic/socket-util.c index 64bc79712f..b482ca988c 100644 --- a/src/basic/socket-util.c +++ b/src/basic/socket-util.c @@ -42,12 +42,12 @@ #endif static const char* const socket_address_type_table[] = { - [SOCK_STREAM] = "Stream", - [SOCK_DGRAM] = "Datagram", - [SOCK_RAW] = "Raw", - [SOCK_RDM] = "ReliableDatagram", + [SOCK_STREAM] = "Stream", + [SOCK_DGRAM] = "Datagram", + [SOCK_RAW] = "Raw", + [SOCK_RDM] = "ReliableDatagram", [SOCK_SEQPACKET] = "SequentialPacket", - [SOCK_DCCP] = "DatagramCongestionControl", + [SOCK_DCCP] = "DatagramCongestionControl", }; DEFINE_STRING_TABLE_LOOKUP(socket_address_type, int); From 5c3fa98db68fbcf1f559b16238a5aba429a56448 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 10 Jan 2020 17:22:37 +0100 Subject: [PATCH 06/11] util-lib: move things that parse ifnames to shared/ In subsequent commits, calls to if_nametoindex() will be replaced by a wrapper that falls back to alternative name resolution over netlink. netlink support requires libsystemd (for sd-netlink), and we don't want to add any functions that require netlink in basic/. So stuff that calls if_nametoindex() for user supplied interface names, and everything that depends on that, needs to be moved. --- src/activate/activate.c | 1 + src/basic/in-addr-util.c | 36 --- src/basic/in-addr-util.h | 2 +- src/basic/socket-label.c | 28 -- src/basic/socket-util.c | 247 ----------------- src/basic/socket-util.h | 14 +- src/core/dbus-socket.c | 1 + src/core/load-fragment.c | 1 + src/core/socket.c | 1 + src/journal-remote/journal-remote-main.c | 1 + src/resolve/resolvectl.c | 2 +- src/resolve/resolved-bus.c | 1 + src/resolve/resolved-etc-hosts.c | 1 + src/resolve/resolved-util.c | 2 +- src/shared/meson.build | 2 + src/shared/socket-netlink.c | 327 +++++++++++++++++++++++ src/shared/socket-netlink.h | 17 ++ src/test/meson.build | 4 + src/test/test-socket-netlink.c | 227 ++++++++++++++++ src/test/test-socket-util.c | 220 +-------------- 20 files changed, 594 insertions(+), 541 deletions(-) create mode 100644 src/shared/socket-netlink.c create mode 100644 src/shared/socket-netlink.h create mode 100644 src/test/test-socket-netlink.c diff --git a/src/activate/activate.c b/src/activate/activate.c index 77b9a055ca..5d796ab38d 100644 --- a/src/activate/activate.c +++ b/src/activate/activate.c @@ -17,6 +17,7 @@ #include "pretty-print.h" #include "process-util.h" #include "signal-util.h" +#include "socket-netlink.h" #include "socket-util.h" #include "string-util.h" #include "strv.h" diff --git a/src/basic/in-addr-util.c b/src/basic/in-addr-util.c index b6ca27e3e8..bfe855fb4b 100644 --- a/src/basic/in-addr-util.c +++ b/src/basic/in-addr-util.c @@ -439,42 +439,6 @@ int in_addr_from_string_auto(const char *s, int *ret_family, union in_addr_union return -EINVAL; } -int in_addr_ifindex_from_string_auto(const char *s, int *family, union in_addr_union *ret_addr, int *ret_ifindex) { - _cleanup_free_ char *buf = NULL; - const char *suffix; - int r, ifindex = 0; - - assert(s); - assert(family); - assert(ret_addr); - - /* Similar to in_addr_from_string_auto() but also parses an optionally appended IPv6 zone suffix ("scope id") - * if one is found. */ - - suffix = strchr(s, '%'); - if (suffix) { - if (ret_ifindex) { - /* If we shall return the interface index, try to parse it */ - ifindex = parse_ifindex_or_ifname(suffix + 1); - if (ifindex < 0) - return ifindex; - } - - s = buf = strndup(s, suffix - s); - if (!buf) - return -ENOMEM; - } - - r = in_addr_from_string_auto(s, family, ret_addr); - if (r < 0) - return r; - - if (ret_ifindex) - *ret_ifindex = ifindex; - - return r; -} - unsigned char in4_addr_netmask_to_prefixlen(const struct in_addr *addr) { assert(addr); diff --git a/src/basic/in-addr-util.h b/src/basic/in-addr-util.h index 28afc7d86c..ae2dad0bb1 100644 --- a/src/basic/in-addr-util.h +++ b/src/basic/in-addr-util.h @@ -42,7 +42,7 @@ int in_addr_prefix_to_string(int family, const union in_addr_union *u, unsigned int in_addr_ifindex_to_string(int family, const union in_addr_union *u, int ifindex, char **ret); int in_addr_from_string(int family, const char *s, union in_addr_union *ret); int in_addr_from_string_auto(const char *s, int *ret_family, union in_addr_union *ret); -int in_addr_ifindex_from_string_auto(const char *s, int *family, union in_addr_union *ret, int *ifindex); + unsigned char in4_addr_netmask_to_prefixlen(const struct in_addr *addr); struct in_addr* in4_addr_prefixlen_to_netmask(struct in_addr *addr, unsigned char prefixlen); int in4_addr_default_prefixlen(const struct in_addr *addr, unsigned char *prefixlen); diff --git a/src/basic/socket-label.c b/src/basic/socket-label.c index 8c087268e4..ec52d81653 100644 --- a/src/basic/socket-label.c +++ b/src/basic/socket-label.c @@ -134,31 +134,3 @@ int socket_address_listen( return r; } - -int make_socket_fd(int log_level, const char* address, int type, int flags) { - SocketAddress a; - int fd, r; - - r = socket_address_parse(&a, address); - if (r < 0) - return log_error_errno(r, "Failed to parse socket address \"%s\": %m", address); - - a.type = type; - - fd = socket_address_listen(&a, type | flags, SOMAXCONN, SOCKET_ADDRESS_DEFAULT, - NULL, false, false, false, 0755, 0644, NULL); - if (fd < 0 || log_get_max_level() >= log_level) { - _cleanup_free_ char *p = NULL; - - r = socket_address_print(&a, &p); - if (r < 0) - return log_error_errno(r, "socket_address_print(): %m"); - - if (fd < 0) - log_error_errno(fd, "Failed to listen on %s: %m", p); - else - log_full(log_level, "Listening on %s", p); - } - - return fd; -} diff --git a/src/basic/socket-util.c b/src/basic/socket-util.c index b482ca988c..ad467ab851 100644 --- a/src/basic/socket-util.c +++ b/src/basic/socket-util.c @@ -52,227 +52,6 @@ static const char* const socket_address_type_table[] = { DEFINE_STRING_TABLE_LOOKUP(socket_address_type, int); -int socket_address_parse(SocketAddress *a, const char *s) { - _cleanup_free_ char *n = NULL; - char *e; - int r; - - assert(a); - assert(s); - - *a = (SocketAddress) { - .type = SOCK_STREAM, - }; - - if (*s == '[') { - uint16_t port; - - /* IPv6 in [x:.....:z]:p notation */ - - e = strchr(s+1, ']'); - if (!e) - return -EINVAL; - - n = strndup(s+1, e-s-1); - if (!n) - return -ENOMEM; - - errno = 0; - if (inet_pton(AF_INET6, n, &a->sockaddr.in6.sin6_addr) <= 0) - return errno_or_else(EINVAL); - - e++; - if (*e != ':') - return -EINVAL; - - e++; - r = parse_ip_port(e, &port); - if (r < 0) - return r; - - a->sockaddr.in6.sin6_family = AF_INET6; - a->sockaddr.in6.sin6_port = htobe16(port); - a->size = sizeof(struct sockaddr_in6); - - } else if (*s == '/') { - /* AF_UNIX socket */ - - size_t l; - - l = strlen(s); - if (l >= sizeof(a->sockaddr.un.sun_path)) /* Note that we refuse non-NUL-terminated sockets when - * parsing (the kernel itself is less strict here in what it - * accepts) */ - return -EINVAL; - - a->sockaddr.un.sun_family = AF_UNIX; - memcpy(a->sockaddr.un.sun_path, s, l); - a->size = offsetof(struct sockaddr_un, sun_path) + l + 1; - - } else if (*s == '@') { - /* Abstract AF_UNIX socket */ - size_t l; - - l = strlen(s+1); - if (l >= sizeof(a->sockaddr.un.sun_path) - 1) /* Note that we refuse non-NUL-terminated sockets here - * when parsing, even though abstract namespace sockets - * explicitly allow embedded NUL bytes and don't consider - * them special. But it's simply annoying to debug such - * sockets. */ - return -EINVAL; - - a->sockaddr.un.sun_family = AF_UNIX; - memcpy(a->sockaddr.un.sun_path+1, s+1, l); - a->size = offsetof(struct sockaddr_un, sun_path) + 1 + l; - - } else if (startswith(s, "vsock:")) { - /* AF_VSOCK socket in vsock:cid:port notation */ - const char *cid_start = s + STRLEN("vsock:"); - unsigned port; - - e = strchr(cid_start, ':'); - if (!e) - return -EINVAL; - - r = safe_atou(e+1, &port); - if (r < 0) - return r; - - n = strndup(cid_start, e - cid_start); - if (!n) - return -ENOMEM; - - if (!isempty(n)) { - r = safe_atou(n, &a->sockaddr.vm.svm_cid); - if (r < 0) - return r; - } else - a->sockaddr.vm.svm_cid = VMADDR_CID_ANY; - - a->sockaddr.vm.svm_family = AF_VSOCK; - a->sockaddr.vm.svm_port = port; - a->size = sizeof(struct sockaddr_vm); - - } else { - uint16_t port; - - e = strchr(s, ':'); - if (e) { - r = parse_ip_port(e + 1, &port); - if (r < 0) - return r; - - n = strndup(s, e-s); - if (!n) - return -ENOMEM; - - /* IPv4 in w.x.y.z:p notation? */ - r = inet_pton(AF_INET, n, &a->sockaddr.in.sin_addr); - if (r < 0) - return -errno; - - if (r > 0) { - /* Gotcha, it's a traditional IPv4 address */ - a->sockaddr.in.sin_family = AF_INET; - a->sockaddr.in.sin_port = htobe16(port); - a->size = sizeof(struct sockaddr_in); - } else { - unsigned idx; - - if (strlen(n) > IF_NAMESIZE-1) - return -EINVAL; - - /* Uh, our last resort, an interface name */ - idx = if_nametoindex(n); - if (idx == 0) - return -EINVAL; - - a->sockaddr.in6.sin6_family = AF_INET6; - a->sockaddr.in6.sin6_port = htobe16(port); - a->sockaddr.in6.sin6_scope_id = idx; - a->sockaddr.in6.sin6_addr = in6addr_any; - a->size = sizeof(struct sockaddr_in6); - } - } else { - - /* Just a port */ - r = parse_ip_port(s, &port); - if (r < 0) - return r; - - if (socket_ipv6_is_supported()) { - a->sockaddr.in6.sin6_family = AF_INET6; - a->sockaddr.in6.sin6_port = htobe16(port); - a->sockaddr.in6.sin6_addr = in6addr_any; - a->size = sizeof(struct sockaddr_in6); - } else { - a->sockaddr.in.sin_family = AF_INET; - a->sockaddr.in.sin_port = htobe16(port); - a->sockaddr.in.sin_addr.s_addr = INADDR_ANY; - a->size = sizeof(struct sockaddr_in); - } - } - } - - return 0; -} - -int socket_address_parse_and_warn(SocketAddress *a, const char *s) { - SocketAddress b; - int r; - - /* Similar to socket_address_parse() but warns for IPv6 sockets when we don't support them. */ - - r = socket_address_parse(&b, s); - if (r < 0) - return r; - - if (!socket_ipv6_is_supported() && b.sockaddr.sa.sa_family == AF_INET6) { - log_warning("Binding to IPv6 address not available since kernel does not support IPv6."); - return -EAFNOSUPPORT; - } - - *a = b; - return 0; -} - -int socket_address_parse_netlink(SocketAddress *a, const char *s) { - _cleanup_free_ char *word = NULL; - unsigned group = 0; - int family, r; - - assert(a); - assert(s); - - zero(*a); - a->type = SOCK_RAW; - - r = extract_first_word(&s, &word, NULL, 0); - if (r < 0) - return r; - if (r == 0) - return -EINVAL; - - family = netlink_family_from_string(word); - if (family < 0) - return -EINVAL; - - if (!isempty(s)) { - r = safe_atou(s, &group); - if (r < 0) - return r; - } - - a->sockaddr.nl.nl_family = AF_NETLINK; - a->sockaddr.nl.nl_groups = group; - - a->type = SOCK_RAW; - a->size = sizeof(struct sockaddr_nl); - a->protocol = family; - - return 0; -} - int socket_address_verify(const SocketAddress *a, bool strict) { assert(a); @@ -482,32 +261,6 @@ bool socket_address_equal(const SocketAddress *a, const SocketAddress *b) { return true; } -bool socket_address_is(const SocketAddress *a, const char *s, int type) { - struct SocketAddress b; - - assert(a); - assert(s); - - if (socket_address_parse(&b, s) < 0) - return false; - - b.type = type; - - return socket_address_equal(a, &b); -} - -bool socket_address_is_netlink(const SocketAddress *a, const char *s) { - struct SocketAddress b; - - assert(a); - assert(s); - - if (socket_address_parse_netlink(&b, s) < 0) - return false; - - return socket_address_equal(a, &b); -} - const char* socket_address_get_path(const SocketAddress *a) { assert(a); diff --git a/src/basic/socket-util.h b/src/basic/socket-util.h index 17c20abc32..24e1213951 100644 --- a/src/basic/socket-util.h +++ b/src/basic/socket-util.h @@ -41,6 +41,8 @@ union sockaddr_union { uint8_t un_buffer[sizeof(struct sockaddr_un) + 1]; }; +#define SUN_PATH_LEN (sizeof(((struct sockaddr_un){}).sun_path)) + typedef struct SocketAddress { union sockaddr_union sockaddr; @@ -68,12 +70,6 @@ typedef enum SocketAddressBindIPv6Only { const char* socket_address_type_to_string(int t) _const_; int socket_address_type_from_string(const char *s) _pure_; -int socket_address_parse(SocketAddress *a, const char *s); -int socket_address_parse_and_warn(SocketAddress *a, const char *s); -int socket_address_parse_netlink(SocketAddress *a, const char *s); -int socket_address_print(const SocketAddress *a, char **p); -int socket_address_verify(const SocketAddress *a, bool strict) _pure_; - int sockaddr_un_unlink(const struct sockaddr_un *sa); static inline int socket_address_unlink(const SocketAddress *a) { @@ -94,11 +90,9 @@ int socket_address_listen( mode_t directory_mode, mode_t socket_mode, const char *label); -int make_socket_fd(int log_level, const char* address, int type, int flags); - -bool socket_address_is(const SocketAddress *a, const char *s, int type); -bool socket_address_is_netlink(const SocketAddress *a, const char *s); +int socket_address_verify(const SocketAddress *a, bool strict) _pure_; +int socket_address_print(const SocketAddress *a, char **p); bool socket_address_matches_fd(const SocketAddress *a, int fd); bool socket_address_equal(const SocketAddress *a, const SocketAddress *b) _pure_; diff --git a/src/core/dbus-socket.c b/src/core/dbus-socket.c index 25d3d71391..c8253c7940 100644 --- a/src/core/dbus-socket.c +++ b/src/core/dbus-socket.c @@ -12,6 +12,7 @@ #include "parse-util.h" #include "path-util.h" #include "socket.h" +#include "socket-netlink.h" #include "socket-util.h" #include "string-util.h" #include "unit.h" diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c index 1679e047dd..3da78b30a4 100644 --- a/src/core/load-fragment.c +++ b/src/core/load-fragment.c @@ -48,6 +48,7 @@ #endif #include "securebits-util.h" #include "signal-util.h" +#include "socket-netlink.h" #include "stat-util.h" #include "string-util.h" #include "strv.h" diff --git a/src/core/socket.c b/src/core/socket.c index c587eaebed..633be68337 100644 --- a/src/core/socket.c +++ b/src/core/socket.c @@ -36,6 +36,7 @@ #include "signal-util.h" #include "smack-util.h" #include "socket.h" +#include "socket-netlink.h" #include "special.h" #include "string-table.h" #include "string-util.h" diff --git a/src/journal-remote/journal-remote-main.c b/src/journal-remote/journal-remote-main.c index ac2bf648d2..88e42d3a98 100644 --- a/src/journal-remote/journal-remote-main.c +++ b/src/journal-remote/journal-remote-main.c @@ -17,6 +17,7 @@ #include "process-util.h" #include "rlimit-util.h" #include "signal-util.h" +#include "socket-netlink.h" #include "socket-util.h" #include "stat-util.h" #include "string-table.h" diff --git a/src/resolve/resolvectl.c b/src/resolve/resolvectl.c index 87ecd519d5..cabb3f6c69 100644 --- a/src/resolve/resolvectl.c +++ b/src/resolve/resolvectl.c @@ -16,7 +16,6 @@ #include "escape.h" #include "format-util.h" #include "gcrypt-util.h" -#include "in-addr-util.h" #include "main-func.h" #include "missing_network.h" #include "netlink-util.h" @@ -27,6 +26,7 @@ #include "resolvectl.h" #include "resolved-def.h" #include "resolved-dns-packet.h" +#include "socket-netlink.h" #include "string-table.h" #include "strv.h" #include "terminal-util.h" diff --git a/src/resolve/resolved-bus.c b/src/resolve/resolved-bus.c index 2c1aba911a..ff6acb2eac 100644 --- a/src/resolve/resolved-bus.c +++ b/src/resolve/resolved-bus.c @@ -12,6 +12,7 @@ #include "resolved-dnssd-bus.h" #include "resolved-dnssd.h" #include "resolved-link-bus.h" +#include "socket-netlink.h" #include "stdio-util.h" #include "strv.h" #include "user-util.h" diff --git a/src/resolve/resolved-etc-hosts.c b/src/resolve/resolved-etc-hosts.c index 6a23d2bb8e..e6bf20db54 100644 --- a/src/resolve/resolved-etc-hosts.c +++ b/src/resolve/resolved-etc-hosts.c @@ -9,6 +9,7 @@ #include "hostname-util.h" #include "resolved-dns-synthesize.h" #include "resolved-etc-hosts.h" +#include "socket-netlink.h" #include "string-util.h" #include "strv.h" #include "time-util.h" diff --git a/src/resolve/resolved-util.c b/src/resolve/resolved-util.c index 2f18f8c19d..ac8df63513 100644 --- a/src/resolve/resolved-util.c +++ b/src/resolve/resolved-util.c @@ -1,9 +1,9 @@ /* SPDX-License-Identifier: LGPL-2.1+ */ #include "alloc-util.h" -#include "in-addr-util.h" #include "macro.h" #include "resolved-util.h" +#include "socket-netlink.h" int in_addr_ifindex_name_from_string_auto(const char *s, int *family, union in_addr_union *ret, int *ifindex, char **server_name) { _cleanup_free_ char *buf = NULL, *name = NULL; diff --git a/src/shared/meson.build b/src/shared/meson.build index 3b53ce1a75..08aa480b6c 100644 --- a/src/shared/meson.build +++ b/src/shared/meson.build @@ -165,6 +165,8 @@ shared_sources = files(''' serialize.h sleep-config.c sleep-config.h + socket-netlink.c + socket-netlink.h spawn-ask-password-agent.c spawn-ask-password-agent.h spawn-polkit-agent.c diff --git a/src/shared/socket-netlink.c b/src/shared/socket-netlink.c new file mode 100644 index 0000000000..9f992d3e2f --- /dev/null +++ b/src/shared/socket-netlink.c @@ -0,0 +1,327 @@ +/* SPDX-License-Identifier: LGPL-2.1+ */ + +#include +#include +#include +#include + +#include "alloc-util.h" +#include "errno-util.h" +#include "extract-word.h" +#include "log.h" +#include "memory-util.h" +#include "parse-util.h" +#include "socket-netlink.h" +#include "socket-util.h" +#include "string-util.h" + +int socket_address_parse(SocketAddress *a, const char *s) { + _cleanup_free_ char *n = NULL; + char *e; + int r; + + assert(a); + assert(s); + + *a = (SocketAddress) { + .type = SOCK_STREAM, + }; + + if (*s == '[') { + uint16_t port; + + /* IPv6 in [x:.....:z]:p notation */ + + e = strchr(s+1, ']'); + if (!e) + return -EINVAL; + + n = strndup(s+1, e-s-1); + if (!n) + return -ENOMEM; + + errno = 0; + if (inet_pton(AF_INET6, n, &a->sockaddr.in6.sin6_addr) <= 0) + return errno_or_else(EINVAL); + + e++; + if (*e != ':') + return -EINVAL; + + e++; + r = parse_ip_port(e, &port); + if (r < 0) + return r; + + a->sockaddr.in6.sin6_family = AF_INET6; + a->sockaddr.in6.sin6_port = htobe16(port); + a->size = sizeof(struct sockaddr_in6); + + } else if (*s == '/') { + /* AF_UNIX socket */ + + size_t l; + + l = strlen(s); + if (l >= sizeof(a->sockaddr.un.sun_path)) /* Note that we refuse non-NUL-terminated sockets when + * parsing (the kernel itself is less strict here in what it + * accepts) */ + return -EINVAL; + + a->sockaddr.un.sun_family = AF_UNIX; + memcpy(a->sockaddr.un.sun_path, s, l); + a->size = offsetof(struct sockaddr_un, sun_path) + l + 1; + + } else if (*s == '@') { + /* Abstract AF_UNIX socket */ + size_t l; + + l = strlen(s+1); + if (l >= sizeof(a->sockaddr.un.sun_path) - 1) /* Note that we refuse non-NUL-terminated sockets here + * when parsing, even though abstract namespace sockets + * explicitly allow embedded NUL bytes and don't consider + * them special. But it's simply annoying to debug such + * sockets. */ + return -EINVAL; + + a->sockaddr.un.sun_family = AF_UNIX; + memcpy(a->sockaddr.un.sun_path+1, s+1, l); + a->size = offsetof(struct sockaddr_un, sun_path) + 1 + l; + + } else if (startswith(s, "vsock:")) { + /* AF_VSOCK socket in vsock:cid:port notation */ + const char *cid_start = s + STRLEN("vsock:"); + unsigned port; + + e = strchr(cid_start, ':'); + if (!e) + return -EINVAL; + + r = safe_atou(e+1, &port); + if (r < 0) + return r; + + n = strndup(cid_start, e - cid_start); + if (!n) + return -ENOMEM; + + if (!isempty(n)) { + r = safe_atou(n, &a->sockaddr.vm.svm_cid); + if (r < 0) + return r; + } else + a->sockaddr.vm.svm_cid = VMADDR_CID_ANY; + + a->sockaddr.vm.svm_family = AF_VSOCK; + a->sockaddr.vm.svm_port = port; + a->size = sizeof(struct sockaddr_vm); + + } else { + uint16_t port; + + e = strchr(s, ':'); + if (e) { + r = parse_ip_port(e + 1, &port); + if (r < 0) + return r; + + n = strndup(s, e-s); + if (!n) + return -ENOMEM; + + /* IPv4 in w.x.y.z:p notation? */ + r = inet_pton(AF_INET, n, &a->sockaddr.in.sin_addr); + if (r < 0) + return -errno; + + if (r > 0) { + /* Gotcha, it's a traditional IPv4 address */ + a->sockaddr.in.sin_family = AF_INET; + a->sockaddr.in.sin_port = htobe16(port); + a->size = sizeof(struct sockaddr_in); + } else { + unsigned idx; + + if (strlen(n) > IF_NAMESIZE-1) + return -EINVAL; + + /* Uh, our last resort, an interface name */ + idx = if_nametoindex(n); + if (idx == 0) + return -EINVAL; + + a->sockaddr.in6.sin6_family = AF_INET6; + a->sockaddr.in6.sin6_port = htobe16(port); + a->sockaddr.in6.sin6_scope_id = idx; + a->sockaddr.in6.sin6_addr = in6addr_any; + a->size = sizeof(struct sockaddr_in6); + } + } else { + + /* Just a port */ + r = parse_ip_port(s, &port); + if (r < 0) + return r; + + if (socket_ipv6_is_supported()) { + a->sockaddr.in6.sin6_family = AF_INET6; + a->sockaddr.in6.sin6_port = htobe16(port); + a->sockaddr.in6.sin6_addr = in6addr_any; + a->size = sizeof(struct sockaddr_in6); + } else { + a->sockaddr.in.sin_family = AF_INET; + a->sockaddr.in.sin_port = htobe16(port); + a->sockaddr.in.sin_addr.s_addr = INADDR_ANY; + a->size = sizeof(struct sockaddr_in); + } + } + } + + return 0; +} + +int socket_address_parse_and_warn(SocketAddress *a, const char *s) { + SocketAddress b; + int r; + + /* Similar to socket_address_parse() but warns for IPv6 sockets when we don't support them. */ + + r = socket_address_parse(&b, s); + if (r < 0) + return r; + + if (!socket_ipv6_is_supported() && b.sockaddr.sa.sa_family == AF_INET6) { + log_warning("Binding to IPv6 address not available since kernel does not support IPv6."); + return -EAFNOSUPPORT; + } + + *a = b; + return 0; +} + +int socket_address_parse_netlink(SocketAddress *a, const char *s) { + _cleanup_free_ char *word = NULL; + unsigned group = 0; + int family, r; + + assert(a); + assert(s); + + zero(*a); + a->type = SOCK_RAW; + + r = extract_first_word(&s, &word, NULL, 0); + if (r < 0) + return r; + if (r == 0) + return -EINVAL; + + family = netlink_family_from_string(word); + if (family < 0) + return -EINVAL; + + if (!isempty(s)) { + r = safe_atou(s, &group); + if (r < 0) + return r; + } + + a->sockaddr.nl.nl_family = AF_NETLINK; + a->sockaddr.nl.nl_groups = group; + + a->type = SOCK_RAW; + a->size = sizeof(struct sockaddr_nl); + a->protocol = family; + + return 0; +} + +bool socket_address_is(const SocketAddress *a, const char *s, int type) { + struct SocketAddress b; + + assert(a); + assert(s); + + if (socket_address_parse(&b, s) < 0) + return false; + + b.type = type; + + return socket_address_equal(a, &b); +} + +bool socket_address_is_netlink(const SocketAddress *a, const char *s) { + struct SocketAddress b; + + assert(a); + assert(s); + + if (socket_address_parse_netlink(&b, s) < 0) + return false; + + return socket_address_equal(a, &b); +} + +int make_socket_fd(int log_level, const char* address, int type, int flags) { + SocketAddress a; + int fd, r; + + r = socket_address_parse(&a, address); + if (r < 0) + return log_error_errno(r, "Failed to parse socket address \"%s\": %m", address); + + a.type = type; + + fd = socket_address_listen(&a, type | flags, SOMAXCONN, SOCKET_ADDRESS_DEFAULT, + NULL, false, false, false, 0755, 0644, NULL); + if (fd < 0 || log_get_max_level() >= log_level) { + _cleanup_free_ char *p = NULL; + + r = socket_address_print(&a, &p); + if (r < 0) + return log_error_errno(r, "socket_address_print(): %m"); + + if (fd < 0) + log_error_errno(fd, "Failed to listen on %s: %m", p); + else + log_full(log_level, "Listening on %s", p); + } + + return fd; +} + +int in_addr_ifindex_from_string_auto(const char *s, int *family, union in_addr_union *ret_addr, int *ret_ifindex) { + _cleanup_free_ char *buf = NULL; + const char *suffix; + int r, ifindex = 0; + + assert(s); + assert(family); + assert(ret_addr); + + /* Similar to in_addr_from_string_auto() but also parses an optionally appended IPv6 zone suffix ("scope id") + * if one is found. */ + + suffix = strchr(s, '%'); + if (suffix) { + if (ret_ifindex) { + /* If we shall return the interface index, try to parse it */ + ifindex = parse_ifindex_or_ifname(suffix + 1); + if (ifindex < 0) + return ifindex; + } + + s = buf = strndup(s, suffix - s); + if (!buf) + return -ENOMEM; + } + + r = in_addr_from_string_auto(s, family, ret_addr); + if (r < 0) + return r; + + if (ret_ifindex) + *ret_ifindex = ifindex; + + return r; +} diff --git a/src/shared/socket-netlink.h b/src/shared/socket-netlink.h new file mode 100644 index 0000000000..8808c589b7 --- /dev/null +++ b/src/shared/socket-netlink.h @@ -0,0 +1,17 @@ +/* SPDX-License-Identifier: LGPL-2.1+ */ +#pragma once + +#include "in-addr-util.h" +#include "macro.h" +#include "socket-util.h" + +int make_socket_fd(int log_level, const char* address, int type, int flags); + +int socket_address_parse(SocketAddress *a, const char *s); +int socket_address_parse_and_warn(SocketAddress *a, const char *s); +int socket_address_parse_netlink(SocketAddress *a, const char *s); + +bool socket_address_is(const SocketAddress *a, const char *s, int type); +bool socket_address_is_netlink(const SocketAddress *a, const char *s); + +int in_addr_ifindex_from_string_auto(const char *s, int *family, union in_addr_union *ret, int *ifindex); diff --git a/src/test/meson.build b/src/test/meson.build index 7e5a61a11e..5df1366561 100644 --- a/src/test/meson.build +++ b/src/test/meson.build @@ -362,6 +362,10 @@ tests += [ [], []], + [['src/test/test-socket-netlink.c'], + [], + []], + [['src/test/test-in-addr-util.c'], [], []], diff --git a/src/test/test-socket-netlink.c b/src/test/test-socket-netlink.c new file mode 100644 index 0000000000..0a2007f0b7 --- /dev/null +++ b/src/test/test-socket-netlink.c @@ -0,0 +1,227 @@ +/* SPDX-License-Identifier: LGPL-2.1+ */ + +#include "alloc-util.h" +#include "tests.h" +#include "socket-netlink.h" +#include "string-util.h" + +static void test_socket_address_parse_one(const char *in, int ret, int family, const char *expected) { + SocketAddress a; + _cleanup_free_ char *out = NULL; + int r; + + r = socket_address_parse(&a, in); + if (r >= 0) + assert_se(socket_address_print(&a, &out) >= 0); + + log_info("\"%s\" → %s → \"%s\" (expect \"%s\")", in, + r >= 0 ? "✓" : "✗", empty_to_dash(out), r >= 0 ? expected ?: in : "-"); + assert_se(r == ret); + if (r >= 0) { + assert_se(a.sockaddr.sa.sa_family == family); + assert_se(streq(out, expected ?: in)); + } +} + +static void test_socket_address_parse(void) { + log_info("/* %s */", __func__); + + test_socket_address_parse_one("junk", -EINVAL, 0, NULL); + test_socket_address_parse_one("192.168.1.1", -EINVAL, 0, NULL); + test_socket_address_parse_one(".168.1.1", -EINVAL, 0, NULL); + test_socket_address_parse_one("989.168.1.1", -EINVAL, 0, NULL); + test_socket_address_parse_one("192.168.1.1:65536", -ERANGE, 0, NULL); + test_socket_address_parse_one("192.168.1.1:0", -EINVAL, 0, NULL); + test_socket_address_parse_one("0", -EINVAL, 0, NULL); + test_socket_address_parse_one("65536", -ERANGE, 0, NULL); + + const int default_family = socket_ipv6_is_supported() ? AF_INET6 : AF_INET; + + test_socket_address_parse_one("65535", 0, default_family, "[::]:65535"); + + /* The checks below will pass even if ipv6 is disabled in + * kernel. The underlying glibc's inet_pton() is just a string + * parser and doesn't make any syscalls. */ + + test_socket_address_parse_one("[::1]", -EINVAL, 0, NULL); + test_socket_address_parse_one("[::1]8888", -EINVAL, 0, NULL); + test_socket_address_parse_one("::1", -EINVAL, 0, NULL); + test_socket_address_parse_one("[::1]:0", -EINVAL, 0, NULL); + test_socket_address_parse_one("[::1]:65536", -ERANGE, 0, NULL); + test_socket_address_parse_one("[a:b:1]:8888", -EINVAL, 0, NULL); + + test_socket_address_parse_one("8888", 0, default_family, "[::]:8888"); + test_socket_address_parse_one("[2001:0db8:0000:85a3:0000:0000:ac1f:8001]:8888", 0, AF_INET6, + "[2001:db8:0:85a3::ac1f:8001]:8888"); + test_socket_address_parse_one("[::1]:8888", 0, AF_INET6, NULL); + test_socket_address_parse_one("192.168.1.254:8888", 0, AF_INET, NULL); + test_socket_address_parse_one("/foo/bar", 0, AF_UNIX, NULL); + test_socket_address_parse_one("/", 0, AF_UNIX, NULL); + test_socket_address_parse_one("@abstract", 0, AF_UNIX, NULL); + + { + char aaa[SUN_PATH_LEN + 1] = "@"; + + memset(aaa + 1, 'a', SUN_PATH_LEN - 1); + char_array_0(aaa); + + test_socket_address_parse_one(aaa, -EINVAL, 0, NULL); + + aaa[SUN_PATH_LEN - 1] = '\0'; + test_socket_address_parse_one(aaa, 0, AF_UNIX, NULL); + } + + test_socket_address_parse_one("vsock:2:1234", 0, AF_VSOCK, NULL); + test_socket_address_parse_one("vsock::1234", 0, AF_VSOCK, NULL); + test_socket_address_parse_one("vsock:2:1234x", -EINVAL, 0, NULL); + test_socket_address_parse_one("vsock:2x:1234", -EINVAL, 0, NULL); + test_socket_address_parse_one("vsock:2", -EINVAL, 0, NULL); +} + +static void test_socket_address_parse_netlink(void) { + SocketAddress a; + + log_info("/* %s */", __func__); + + assert_se(socket_address_parse_netlink(&a, "junk") < 0); + assert_se(socket_address_parse_netlink(&a, "") < 0); + + assert_se(socket_address_parse_netlink(&a, "route") >= 0); + assert_se(a.sockaddr.nl.nl_family == AF_NETLINK); + assert_se(a.sockaddr.nl.nl_groups == 0); + assert_se(a.protocol == NETLINK_ROUTE); + assert_se(socket_address_parse_netlink(&a, "route") >= 0); + assert_se(socket_address_parse_netlink(&a, "route 10") >= 0); + assert_se(a.sockaddr.nl.nl_family == AF_NETLINK); + assert_se(a.sockaddr.nl.nl_groups == 10); + assert_se(a.protocol == NETLINK_ROUTE); + + /* With spaces and tabs */ + assert_se(socket_address_parse_netlink(&a, " kobject-uevent ") >= 0); + assert_se(a.sockaddr.nl.nl_family == AF_NETLINK); + assert_se(a.sockaddr.nl.nl_groups == 0); + assert_se(a.protocol == NETLINK_KOBJECT_UEVENT); + assert_se(socket_address_parse_netlink(&a, " \t kobject-uevent \t 10") >= 0); + assert_se(a.sockaddr.nl.nl_family == AF_NETLINK); + assert_se(a.sockaddr.nl.nl_groups == 10); + assert_se(a.protocol == NETLINK_KOBJECT_UEVENT); + assert_se(socket_address_parse_netlink(&a, "kobject-uevent\t10") >= 0); + assert_se(a.sockaddr.nl.nl_family == AF_NETLINK); + assert_se(a.sockaddr.nl.nl_groups == 10); + assert_se(a.protocol == NETLINK_KOBJECT_UEVENT); + + /* trailing space is not supported */ + assert_se(socket_address_parse_netlink(&a, "kobject-uevent\t10 ") < 0); + + /* Group must be unsigned */ + assert_se(socket_address_parse_netlink(&a, "kobject-uevent -1") < 0); + + /* oss-fuzz #6884 */ + assert_se(socket_address_parse_netlink(&a, "\xff") < 0); +} + +static void test_socket_address_equal(void) { + SocketAddress a, b; + + log_info("/* %s */", __func__); + + assert_se(socket_address_parse(&a, "192.168.1.1:8888") >= 0); + assert_se(socket_address_parse(&b, "192.168.1.1:888") >= 0); + assert_se(!socket_address_equal(&a, &b)); + + assert_se(socket_address_parse(&a, "192.168.1.1:8888") >= 0); + assert_se(socket_address_parse(&b, "192.16.1.1:8888") >= 0); + assert_se(!socket_address_equal(&a, &b)); + + assert_se(socket_address_parse(&a, "192.168.1.1:8888") >= 0); + assert_se(socket_address_parse(&b, "8888") >= 0); + assert_se(!socket_address_equal(&a, &b)); + + assert_se(socket_address_parse(&a, "192.168.1.1:8888") >= 0); + assert_se(socket_address_parse(&b, "/foo/bar/") >= 0); + assert_se(!socket_address_equal(&a, &b)); + + assert_se(socket_address_parse(&a, "192.168.1.1:8888") >= 0); + assert_se(socket_address_parse(&b, "192.168.1.1:8888") >= 0); + assert_se(socket_address_equal(&a, &b)); + + assert_se(socket_address_parse(&a, "/foo/bar") >= 0); + assert_se(socket_address_parse(&b, "/foo/bar") >= 0); + assert_se(socket_address_equal(&a, &b)); + + assert_se(socket_address_parse(&a, "[::1]:8888") >= 0); + assert_se(socket_address_parse(&b, "[::1]:8888") >= 0); + assert_se(socket_address_equal(&a, &b)); + + assert_se(socket_address_parse(&a, "@abstract") >= 0); + assert_se(socket_address_parse(&b, "@abstract") >= 0); + assert_se(socket_address_equal(&a, &b)); + + assert_se(socket_address_parse_netlink(&a, "firewall") >= 0); + assert_se(socket_address_parse_netlink(&b, "firewall") >= 0); + assert_se(socket_address_equal(&a, &b)); + + assert_se(socket_address_parse(&a, "vsock:2:1234") >= 0); + assert_se(socket_address_parse(&b, "vsock:2:1234") >= 0); + assert_se(socket_address_equal(&a, &b)); + assert_se(socket_address_parse(&b, "vsock:2:1235") >= 0); + assert_se(!socket_address_equal(&a, &b)); + assert_se(socket_address_parse(&b, "vsock:3:1234") >= 0); + assert_se(!socket_address_equal(&a, &b)); +} + +static void test_socket_address_get_path(void) { + SocketAddress a; + + log_info("/* %s */", __func__); + + assert_se(socket_address_parse(&a, "192.168.1.1:8888") >= 0); + assert_se(!socket_address_get_path(&a)); + + assert_se(socket_address_parse(&a, "@abstract") >= 0); + assert_se(!socket_address_get_path(&a)); + + assert_se(socket_address_parse(&a, "[::1]:8888") >= 0); + assert_se(!socket_address_get_path(&a)); + + assert_se(socket_address_parse(&a, "/foo/bar") >= 0); + assert_se(streq(socket_address_get_path(&a), "/foo/bar")); + + assert_se(socket_address_parse(&a, "vsock:2:1234") >= 0); + assert_se(!socket_address_get_path(&a)); +} + +static void test_socket_address_is(void) { + SocketAddress a; + + log_info("/* %s */", __func__); + + assert_se(socket_address_parse(&a, "192.168.1.1:8888") >= 0); + assert_se(socket_address_is(&a, "192.168.1.1:8888", SOCK_STREAM)); + assert_se(!socket_address_is(&a, "route", SOCK_STREAM)); + assert_se(!socket_address_is(&a, "192.168.1.1:8888", SOCK_RAW)); +} + +static void test_socket_address_is_netlink(void) { + SocketAddress a; + + log_info("/* %s */", __func__); + + assert_se(socket_address_parse_netlink(&a, "route 10") >= 0); + assert_se(socket_address_is_netlink(&a, "route 10")); + assert_se(!socket_address_is_netlink(&a, "192.168.1.1:8888")); + assert_se(!socket_address_is_netlink(&a, "route 1")); +} + +int main(int argc, char *argv[]) { + test_setup_logging(LOG_DEBUG); + + test_socket_address_parse(); + test_socket_address_parse_netlink(); + test_socket_address_equal(); + test_socket_address_get_path(); + test_socket_address_is(); + test_socket_address_is_netlink(); + + return 0; +} diff --git a/src/test/test-socket-util.c b/src/test/test-socket-util.c index 2489d00e26..d36caaa71e 100644 --- a/src/test/test-socket-util.c +++ b/src/test/test-socket-util.c @@ -17,11 +17,14 @@ #include "macro.h" #include "missing_network.h" #include "process-util.h" +#include "socket-netlink.h" #include "socket-util.h" #include "string-util.h" #include "tests.h" #include "tmpfile-util.h" +assert_cc(SUN_PATH_LEN == 108); + static void test_ifname_valid(void) { log_info("/* %s */", __func__); @@ -48,82 +51,6 @@ static void test_ifname_valid(void) { assert(ifname_valid_full("xxxxxxxxxxxxxxxx", true)); } -static void test_socket_address_parse_one(const char *in, int ret, int family, const char *expected) { - SocketAddress a; - _cleanup_free_ char *out = NULL; - int r; - - r = socket_address_parse(&a, in); - if (r >= 0) - assert_se(socket_address_print(&a, &out) >= 0); - - log_info("\"%s\" → %s → \"%s\" (expect \"%s\")", in, - r >= 0 ? "✓" : "✗", empty_to_dash(out), r >= 0 ? expected ?: in : "-"); - assert_se(r == ret); - if (r >= 0) { - assert_se(a.sockaddr.sa.sa_family == family); - assert_se(streq(out, expected ?: in)); - } -} - -#define SUN_PATH_LEN (sizeof(((struct sockaddr_un){}).sun_path)) -assert_cc(SUN_PATH_LEN == 108); - -static void test_socket_address_parse(void) { - log_info("/* %s */", __func__); - - test_socket_address_parse_one("junk", -EINVAL, 0, NULL); - test_socket_address_parse_one("192.168.1.1", -EINVAL, 0, NULL); - test_socket_address_parse_one(".168.1.1", -EINVAL, 0, NULL); - test_socket_address_parse_one("989.168.1.1", -EINVAL, 0, NULL); - test_socket_address_parse_one("192.168.1.1:65536", -ERANGE, 0, NULL); - test_socket_address_parse_one("192.168.1.1:0", -EINVAL, 0, NULL); - test_socket_address_parse_one("0", -EINVAL, 0, NULL); - test_socket_address_parse_one("65536", -ERANGE, 0, NULL); - - const int default_family = socket_ipv6_is_supported() ? AF_INET6 : AF_INET; - - test_socket_address_parse_one("65535", 0, default_family, "[::]:65535"); - - /* The checks below will pass even if ipv6 is disabled in - * kernel. The underlying glibc's inet_pton() is just a string - * parser and doesn't make any syscalls. */ - - test_socket_address_parse_one("[::1]", -EINVAL, 0, NULL); - test_socket_address_parse_one("[::1]8888", -EINVAL, 0, NULL); - test_socket_address_parse_one("::1", -EINVAL, 0, NULL); - test_socket_address_parse_one("[::1]:0", -EINVAL, 0, NULL); - test_socket_address_parse_one("[::1]:65536", -ERANGE, 0, NULL); - test_socket_address_parse_one("[a:b:1]:8888", -EINVAL, 0, NULL); - - test_socket_address_parse_one("8888", 0, default_family, "[::]:8888"); - test_socket_address_parse_one("[2001:0db8:0000:85a3:0000:0000:ac1f:8001]:8888", 0, AF_INET6, - "[2001:db8:0:85a3::ac1f:8001]:8888"); - test_socket_address_parse_one("[::1]:8888", 0, AF_INET6, NULL); - test_socket_address_parse_one("192.168.1.254:8888", 0, AF_INET, NULL); - test_socket_address_parse_one("/foo/bar", 0, AF_UNIX, NULL); - test_socket_address_parse_one("/", 0, AF_UNIX, NULL); - test_socket_address_parse_one("@abstract", 0, AF_UNIX, NULL); - - { - char aaa[SUN_PATH_LEN + 1] = "@"; - - memset(aaa + 1, 'a', SUN_PATH_LEN - 1); - char_array_0(aaa); - - test_socket_address_parse_one(aaa, -EINVAL, 0, NULL); - - aaa[SUN_PATH_LEN - 1] = '\0'; - test_socket_address_parse_one(aaa, 0, AF_UNIX, NULL); - } - - test_socket_address_parse_one("vsock:2:1234", 0, AF_VSOCK, NULL); - test_socket_address_parse_one("vsock::1234", 0, AF_VSOCK, NULL); - test_socket_address_parse_one("vsock:2:1234x", -EINVAL, 0, NULL); - test_socket_address_parse_one("vsock:2x:1234", -EINVAL, 0, NULL); - test_socket_address_parse_one("vsock:2", -EINVAL, 0, NULL); -} - static void test_socket_print_unix_one(const char *in, size_t len_in, const char *expected) { _cleanup_free_ char *out = NULL, *c = NULL; @@ -157,141 +84,6 @@ static void test_socket_print_unix(void) { test_socket_print_unix_one("\0\a\b\n\255", 6, "@\\a\\b\\n\\255\\000"); } -static void test_socket_address_parse_netlink(void) { - SocketAddress a; - - log_info("/* %s */", __func__); - - assert_se(socket_address_parse_netlink(&a, "junk") < 0); - assert_se(socket_address_parse_netlink(&a, "") < 0); - - assert_se(socket_address_parse_netlink(&a, "route") >= 0); - assert_se(a.sockaddr.nl.nl_family == AF_NETLINK); - assert_se(a.sockaddr.nl.nl_groups == 0); - assert_se(a.protocol == NETLINK_ROUTE); - assert_se(socket_address_parse_netlink(&a, "route") >= 0); - assert_se(socket_address_parse_netlink(&a, "route 10") >= 0); - assert_se(a.sockaddr.nl.nl_family == AF_NETLINK); - assert_se(a.sockaddr.nl.nl_groups == 10); - assert_se(a.protocol == NETLINK_ROUTE); - - /* With spaces and tabs */ - assert_se(socket_address_parse_netlink(&a, " kobject-uevent ") >= 0); - assert_se(a.sockaddr.nl.nl_family == AF_NETLINK); - assert_se(a.sockaddr.nl.nl_groups == 0); - assert_se(a.protocol == NETLINK_KOBJECT_UEVENT); - assert_se(socket_address_parse_netlink(&a, " \t kobject-uevent \t 10") >= 0); - assert_se(a.sockaddr.nl.nl_family == AF_NETLINK); - assert_se(a.sockaddr.nl.nl_groups == 10); - assert_se(a.protocol == NETLINK_KOBJECT_UEVENT); - assert_se(socket_address_parse_netlink(&a, "kobject-uevent\t10") >= 0); - assert_se(a.sockaddr.nl.nl_family == AF_NETLINK); - assert_se(a.sockaddr.nl.nl_groups == 10); - assert_se(a.protocol == NETLINK_KOBJECT_UEVENT); - - /* trailing space is not supported */ - assert_se(socket_address_parse_netlink(&a, "kobject-uevent\t10 ") < 0); - - /* Group must be unsigned */ - assert_se(socket_address_parse_netlink(&a, "kobject-uevent -1") < 0); - - /* oss-fuzz #6884 */ - assert_se(socket_address_parse_netlink(&a, "\xff") < 0); -} - -static void test_socket_address_equal(void) { - SocketAddress a, b; - - log_info("/* %s */", __func__); - - assert_se(socket_address_parse(&a, "192.168.1.1:8888") >= 0); - assert_se(socket_address_parse(&b, "192.168.1.1:888") >= 0); - assert_se(!socket_address_equal(&a, &b)); - - assert_se(socket_address_parse(&a, "192.168.1.1:8888") >= 0); - assert_se(socket_address_parse(&b, "192.16.1.1:8888") >= 0); - assert_se(!socket_address_equal(&a, &b)); - - assert_se(socket_address_parse(&a, "192.168.1.1:8888") >= 0); - assert_se(socket_address_parse(&b, "8888") >= 0); - assert_se(!socket_address_equal(&a, &b)); - - assert_se(socket_address_parse(&a, "192.168.1.1:8888") >= 0); - assert_se(socket_address_parse(&b, "/foo/bar/") >= 0); - assert_se(!socket_address_equal(&a, &b)); - - assert_se(socket_address_parse(&a, "192.168.1.1:8888") >= 0); - assert_se(socket_address_parse(&b, "192.168.1.1:8888") >= 0); - assert_se(socket_address_equal(&a, &b)); - - assert_se(socket_address_parse(&a, "/foo/bar") >= 0); - assert_se(socket_address_parse(&b, "/foo/bar") >= 0); - assert_se(socket_address_equal(&a, &b)); - - assert_se(socket_address_parse(&a, "[::1]:8888") >= 0); - assert_se(socket_address_parse(&b, "[::1]:8888") >= 0); - assert_se(socket_address_equal(&a, &b)); - - assert_se(socket_address_parse(&a, "@abstract") >= 0); - assert_se(socket_address_parse(&b, "@abstract") >= 0); - assert_se(socket_address_equal(&a, &b)); - - assert_se(socket_address_parse_netlink(&a, "firewall") >= 0); - assert_se(socket_address_parse_netlink(&b, "firewall") >= 0); - assert_se(socket_address_equal(&a, &b)); - - assert_se(socket_address_parse(&a, "vsock:2:1234") >= 0); - assert_se(socket_address_parse(&b, "vsock:2:1234") >= 0); - assert_se(socket_address_equal(&a, &b)); - assert_se(socket_address_parse(&b, "vsock:2:1235") >= 0); - assert_se(!socket_address_equal(&a, &b)); - assert_se(socket_address_parse(&b, "vsock:3:1234") >= 0); - assert_se(!socket_address_equal(&a, &b)); -} - -static void test_socket_address_get_path(void) { - SocketAddress a; - - log_info("/* %s */", __func__); - - assert_se(socket_address_parse(&a, "192.168.1.1:8888") >= 0); - assert_se(!socket_address_get_path(&a)); - - assert_se(socket_address_parse(&a, "@abstract") >= 0); - assert_se(!socket_address_get_path(&a)); - - assert_se(socket_address_parse(&a, "[::1]:8888") >= 0); - assert_se(!socket_address_get_path(&a)); - - assert_se(socket_address_parse(&a, "/foo/bar") >= 0); - assert_se(streq(socket_address_get_path(&a), "/foo/bar")); - - assert_se(socket_address_parse(&a, "vsock:2:1234") >= 0); - assert_se(!socket_address_get_path(&a)); -} - -static void test_socket_address_is(void) { - SocketAddress a; - - log_info("/* %s */", __func__); - - assert_se(socket_address_parse(&a, "192.168.1.1:8888") >= 0); - assert_se(socket_address_is(&a, "192.168.1.1:8888", SOCK_STREAM)); - assert_se(!socket_address_is(&a, "route", SOCK_STREAM)); - assert_se(!socket_address_is(&a, "192.168.1.1:8888", SOCK_RAW)); -} - -static void test_socket_address_is_netlink(void) { - SocketAddress a; - - log_info("/* %s */", __func__); - - assert_se(socket_address_parse_netlink(&a, "route 10") >= 0); - assert_se(socket_address_is_netlink(&a, "route 10")); - assert_se(!socket_address_is_netlink(&a, "192.168.1.1:8888")); - assert_se(!socket_address_is_netlink(&a, "route 1")); -} - static void test_in_addr_is_null(void) { union in_addr_union i = {}; @@ -876,13 +668,7 @@ int main(int argc, char *argv[]) { test_ifname_valid(); - test_socket_address_parse(); test_socket_print_unix(); - test_socket_address_parse_netlink(); - test_socket_address_equal(); - test_socket_address_get_path(); - test_socket_address_is(); - test_socket_address_is_netlink(); test_in_addr_is_null(); test_in_addr_prefix_intersect(); From fc2ea97ad03baaa480253034b2f709e82bbc0fe1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 10 Jan 2020 21:18:31 +0100 Subject: [PATCH 07/11] util-lib: add function to resolve "alternative" names Calls to if_nametoindex() are expected to use resolve_ifname() instead. --- src/network/networkd-manager-bus.c | 11 ++++------- src/shared/socket-netlink.c | 26 +++++++++++++++++++------- src/shared/socket-netlink.h | 4 ++++ 3 files changed, 27 insertions(+), 14 deletions(-) diff --git a/src/network/networkd-manager-bus.c b/src/network/networkd-manager-bus.c index 4db72b3302..660c2847eb 100644 --- a/src/network/networkd-manager-bus.c +++ b/src/network/networkd-manager-bus.c @@ -7,12 +7,12 @@ #include "alloc-util.h" #include "bus-common-errors.h" #include "bus-util.h" -#include "netlink-util.h" #include "networkd-link-bus.h" #include "networkd-link.h" #include "networkd-manager-bus.h" #include "networkd-manager.h" #include "path-util.h" +#include "socket-netlink.h" #include "strv.h" #include "user-util.h" @@ -66,12 +66,9 @@ static int method_get_link_by_name(sd_bus_message *message, void *userdata, sd_b if (r < 0) return r; - index = if_nametoindex(name); - if (index <= 0) { - index = rtnl_resolve_link_alternative_name(&manager->rtnl, name); - if (index < 0) - return sd_bus_error_setf(error, BUS_ERROR_NO_SUCH_LINK, "Link %s not known", name); - } + index = resolve_ifname(&manager->rtnl, name); + if (index < 0) + return sd_bus_error_setf(error, BUS_ERROR_NO_SUCH_LINK, "Link %s cannot be resolved", name); link = hashmap_get(manager->links, INT_TO_PTR(index)); if (!link) diff --git a/src/shared/socket-netlink.c b/src/shared/socket-netlink.c index 9f992d3e2f..3fb7356317 100644 --- a/src/shared/socket-netlink.c +++ b/src/shared/socket-netlink.c @@ -10,11 +10,26 @@ #include "extract-word.h" #include "log.h" #include "memory-util.h" +#include "netlink-util.h" #include "parse-util.h" #include "socket-netlink.h" #include "socket-util.h" #include "string-util.h" +int resolve_ifname(sd_netlink **rtnl, const char *name) { + int r; + + /* Like if_nametoindex, but resolves "alternative names" too. */ + + assert(name); + + r = if_nametoindex(name); + if (r > 0) + return r; + + return rtnl_resolve_link_alternative_name(rtnl, name); +} + int socket_address_parse(SocketAddress *a, const char *s) { _cleanup_free_ char *n = NULL; char *e; @@ -140,15 +155,12 @@ int socket_address_parse(SocketAddress *a, const char *s) { a->sockaddr.in.sin_port = htobe16(port); a->size = sizeof(struct sockaddr_in); } else { - unsigned idx; - - if (strlen(n) > IF_NAMESIZE-1) - return -EINVAL; + int idx; /* Uh, our last resort, an interface name */ - idx = if_nametoindex(n); - if (idx == 0) - return -EINVAL; + idx = resolve_ifname(NULL, n); + if (idx < 0) + return idx; a->sockaddr.in6.sin6_family = AF_INET6; a->sockaddr.in6.sin6_port = htobe16(port); diff --git a/src/shared/socket-netlink.h b/src/shared/socket-netlink.h index 8808c589b7..d389857242 100644 --- a/src/shared/socket-netlink.h +++ b/src/shared/socket-netlink.h @@ -1,10 +1,14 @@ /* SPDX-License-Identifier: LGPL-2.1+ */ #pragma once +#include "sd-netlink.h" + #include "in-addr-util.h" #include "macro.h" #include "socket-util.h" +int resolve_ifname(sd_netlink **rtnl, const char *name); + int make_socket_fd(int log_level, const char* address, int type, int flags); int socket_address_parse(SocketAddress *a, const char *s); From d308bb99d20b45fed1923ed6b893a59dbac02eca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sat, 11 Jan 2020 11:21:01 +0100 Subject: [PATCH 08/11] Resolve alternative ifnames wherever we would resolve an interface name To keep the names manageable, "ifname_or_ifindex" is replaced by "interface". --- src/basic/parse-util.c | 17 ---------------- src/basic/parse-util.h | 1 - src/libsystemd/sd-netlink/netlink-util.c | 2 ++ src/network/networkctl.c | 22 ++++---------------- src/network/networkd-route.c | 3 ++- src/nspawn/nspawn-network.c | 24 ++++++++-------------- src/resolve/resolvectl.c | 8 ++++---- src/shared/socket-netlink.c | 26 +++++++++++++++++++++++- src/shared/socket-netlink.h | 2 ++ 9 files changed, 47 insertions(+), 58 deletions(-) diff --git a/src/basic/parse-util.c b/src/basic/parse-util.c index d3140cc1a4..b644c4a4d6 100644 --- a/src/basic/parse-util.c +++ b/src/basic/parse-util.c @@ -93,23 +93,6 @@ int parse_ifindex(const char *s) { return ifi; } -int parse_ifindex_or_ifname(const char *s) { - int r; - - assert(s); - - r = parse_ifindex(s); - if (r > 0) - return r; - assert(r < 0); - - r = (int) if_nametoindex(s); - if (r <= 0) - return -errno; - - return r; -} - int parse_mtu(int family, const char *s, uint32_t *ret) { uint64_t u; size_t m; diff --git a/src/basic/parse-util.h b/src/basic/parse-util.h index b20f5b52cc..c6d1d24967 100644 --- a/src/basic/parse-util.h +++ b/src/basic/parse-util.h @@ -14,7 +14,6 @@ int parse_dev(const char *s, dev_t *ret); int parse_pid(const char *s, pid_t* ret_pid); int parse_mode(const char *s, mode_t *ret); int parse_ifindex(const char *s); -int parse_ifindex_or_ifname(const char *s); int parse_mtu(int family, const char *s, uint32_t *ret); int parse_size(const char *t, uint64_t base, uint64_t *size); diff --git a/src/libsystemd/sd-netlink/netlink-util.c b/src/libsystemd/sd-netlink/netlink-util.c index 0b2d171b2d..9c6a5d29f6 100644 --- a/src/libsystemd/sd-netlink/netlink-util.c +++ b/src/libsystemd/sd-netlink/netlink-util.c @@ -191,6 +191,8 @@ int rtnl_resolve_link_alternative_name(sd_netlink **rtnl, const char *name) { return r; r = sd_netlink_call(*rtnl, message, 0, &reply); + if (r == -EINVAL) + return -ENODEV; /* The device doesn't exist */ if (r < 0) return r; diff --git a/src/network/networkctl.c b/src/network/networkctl.c index 5cc62841ec..e186515205 100644 --- a/src/network/networkctl.c +++ b/src/network/networkctl.c @@ -38,6 +38,7 @@ #include "parse-util.h" #include "pretty-print.h" #include "set.h" +#include "socket-netlink.h" #include "socket-util.h" #include "sort-util.h" #include "sparse-endian.h" @@ -64,21 +65,6 @@ static bool arg_stats = false; static bool arg_full = false; static unsigned arg_lines = 10; -static int resolve_ifname(sd_netlink **rtnl, const char *name) { - int r; - - r = parse_ifindex_or_ifname(name); - if (r > 0) - return r; - assert(r < 0); - - r = rtnl_resolve_link_alternative_name(rtnl, name); - if (r > 0) - return r; - assert(r < 0); - return log_error_errno(r, "Failed to resolve interface \"%s\": %m", name); -} - static char *link_get_type_string(unsigned short iftype, sd_device *d) { const char *t, *devtype; char *p; @@ -1895,7 +1881,7 @@ static int link_delete(int argc, char *argv[], void *userdata) { return log_oom(); for (i = 1; i < argc; i++) { - index = resolve_ifname(&rtnl, argv[i]); + index = resolve_interface_or_warn(&rtnl, argv[i]); if (index < 0) return index; @@ -1948,7 +1934,7 @@ static int link_renew(int argc, char *argv[], void *userdata) { return log_error_errno(r, "Failed to connect system bus: %m"); for (i = 1; i < argc; i++) { - index = resolve_ifname(&rtnl, argv[i]); + index = resolve_interface_or_warn(&rtnl, argv[i]); if (index < 0) return index; @@ -2000,7 +1986,7 @@ static int verb_reconfigure(int argc, char *argv[], void *userdata) { return log_oom(); for (i = 1; i < argc; i++) { - index = resolve_ifname(&rtnl, argv[i]); + index = resolve_interface_or_warn(&rtnl, argv[i]); if (index < 0) return index; diff --git a/src/network/networkd-route.c b/src/network/networkd-route.c index 2223a356da..2df8b864e8 100644 --- a/src/network/networkd-route.c +++ b/src/network/networkd-route.c @@ -12,6 +12,7 @@ #include "networkd-route.h" #include "parse-util.h" #include "set.h" +#include "socket-netlink.h" #include "string-table.h" #include "string-util.h" #include "strxcpyx.h" @@ -1646,7 +1647,7 @@ int config_parse_multipath_route( m->gateway.family = family; if (dev) { - r = parse_ifindex_or_ifname(dev); + r = resolve_interface(NULL, dev); if (r < 0) { log_syntax(unit, LOG_ERR, filename, line, r, "Invalid interface name or index, ignoring assignment: %s", dev); diff --git a/src/nspawn/nspawn-network.c b/src/nspawn/nspawn-network.c index b2592b023d..da074d77c6 100644 --- a/src/nspawn/nspawn-network.c +++ b/src/nspawn/nspawn-network.c @@ -18,6 +18,7 @@ #include "nspawn-network.h" #include "parse-util.h" #include "siphash24.h" +#include "socket-netlink.h" #include "socket-util.h" #include "stat-util.h" #include "string-util.h" @@ -280,7 +281,8 @@ int setup_veth(const char *machine_name, if (r < 0) return r; - u = if_nametoindex(n); + u = if_nametoindex(n); /* We don't need to use resolve_ifname() here because the + * name we assigned is always the main name. */ if (u == 0) return log_error_errno(errno, "Failed to resolve interface %s: %m", n); @@ -337,7 +339,7 @@ static int join_bridge(sd_netlink *rtnl, const char *veth_name, const char *brid assert(veth_name); assert(bridge_name); - bridge_ifi = parse_ifindex_or_ifname(bridge_name); + bridge_ifi = resolve_interface(&rtnl, bridge_name); if (bridge_ifi < 0) return bridge_ifi; @@ -472,16 +474,6 @@ int remove_bridge(const char *bridge_name) { return remove_one_link(rtnl, bridge_name); } -static int parse_interface(const char *name) { - int ifi; - - ifi = parse_ifindex_or_ifname(name); - if (ifi < 0) - return log_error_errno(ifi, "Failed to resolve interface %s: %m", name); - - return ifi; -} - int test_network_interface_initialized(const char *name) { _cleanup_(sd_device_unrefp) sd_device *d = NULL; int ifi, r; @@ -492,7 +484,7 @@ int test_network_interface_initialized(const char *name) { /* udev should be around. */ - ifi = parse_interface(name); + ifi = resolve_interface_or_warn(NULL, name); if (ifi < 0) return ifi; @@ -532,7 +524,7 @@ int move_network_interfaces(int netns_fd, char **ifaces) { _cleanup_(sd_netlink_message_unrefp) sd_netlink_message *m = NULL; int ifi; - ifi = parse_interface(*i); + ifi = resolve_interface_or_warn(&rtnl, *i); if (ifi < 0) return ifi; @@ -571,7 +563,7 @@ int setup_macvlan(const char *machine_name, pid_t pid, char **ifaces) { struct ether_addr mac; int ifi; - ifi = parse_interface(*i); + ifi = resolve_interface_or_warn(&rtnl, *i); if (ifi < 0) return ifi; @@ -657,7 +649,7 @@ int setup_ipvlan(const char *machine_name, pid_t pid, char **ifaces) { _cleanup_free_ char *n = NULL, *a = NULL; int ifi; - ifi = parse_interface(*i); + ifi = resolve_interface_or_warn(&rtnl, *i); if (ifi < 0) return ifi; diff --git a/src/resolve/resolvectl.c b/src/resolve/resolvectl.c index cabb3f6c69..2dceaa5d21 100644 --- a/src/resolve/resolvectl.c +++ b/src/resolve/resolvectl.c @@ -94,14 +94,14 @@ int ifname_mangle(const char *s) { if (!iface) return log_oom(); - ifi = parse_ifindex_or_ifname(iface); + ifi = resolve_interface(NULL, iface); if (ifi < 0) { if (ifi == -ENODEV && arg_ifindex_permissive) { log_debug("Interface '%s' not found, but -f specified, ignoring.", iface); return 0; /* done */ } - return log_error_errno(ifi, "Unknown interface '%s': %m", iface); + return log_error_errno(ifi, "Failed to resolve interface \"%s\": %m", iface); } if (arg_ifindex > 0 && arg_ifindex != ifi) @@ -1828,9 +1828,9 @@ static int verb_status(int argc, char **argv, void *userdata) { STRV_FOREACH(ifname, argv + 1) { int ifindex, q; - ifindex = parse_ifindex_or_ifname(*ifname); + ifindex = resolve_interface(NULL, *ifname); if (ifindex < 0) { - log_warning_errno(ifindex, "Unknown interface '%s', ignoring: %m", *ifname); + log_warning_errno(ifindex, "Failed to resolve interface \"%s\", ignoring: %m", *ifname); continue; } diff --git a/src/shared/socket-netlink.c b/src/shared/socket-netlink.c index 3fb7356317..5177137b99 100644 --- a/src/shared/socket-netlink.c +++ b/src/shared/socket-netlink.c @@ -30,6 +30,30 @@ int resolve_ifname(sd_netlink **rtnl, const char *name) { return rtnl_resolve_link_alternative_name(rtnl, name); } +int resolve_interface(sd_netlink **rtnl, const char *name) { + int r; + + /* Like resolve_ifname, but resolves interface numbers too. */ + + assert(name); + + r = parse_ifindex(name); + if (r > 0) + return r; + assert(r < 0); + + return resolve_ifname(rtnl, name); +} + +int resolve_interface_or_warn(sd_netlink **rtnl, const char *name) { + int r; + + r = resolve_interface(rtnl, name); + if (r < 0) + return log_error_errno(r, "Failed to resolve interface \"%s\": %m", name); + return r; +} + int socket_address_parse(SocketAddress *a, const char *s) { _cleanup_free_ char *n = NULL; char *e; @@ -318,7 +342,7 @@ int in_addr_ifindex_from_string_auto(const char *s, int *family, union in_addr_u if (suffix) { if (ret_ifindex) { /* If we shall return the interface index, try to parse it */ - ifindex = parse_ifindex_or_ifname(suffix + 1); + ifindex = resolve_interface(NULL, suffix + 1); if (ifindex < 0) return ifindex; } diff --git a/src/shared/socket-netlink.h b/src/shared/socket-netlink.h index d389857242..fa58409d61 100644 --- a/src/shared/socket-netlink.h +++ b/src/shared/socket-netlink.h @@ -8,6 +8,8 @@ #include "socket-util.h" int resolve_ifname(sd_netlink **rtnl, const char *name); +int resolve_interface(sd_netlink **rtnl, const char *name); +int resolve_interface_or_warn(sd_netlink **rtnl, const char *name); int make_socket_fd(int log_level, const char* address, int type, int flags); From 957d9df388222850225dd8f9ba0a3e8457f864df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sat, 11 Jan 2020 11:37:26 +0100 Subject: [PATCH 09/11] resolvectl: minor optimizations to allocate less --- src/resolve/resolvectl.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/resolve/resolvectl.c b/src/resolve/resolvectl.c index 2dceaa5d21..df40349e66 100644 --- a/src/resolve/resolvectl.c +++ b/src/resolve/resolvectl.c @@ -27,6 +27,7 @@ #include "resolved-def.h" #include "resolved-dns-packet.h" #include "socket-netlink.h" +#include "stdio-util.h" #include "string-table.h" #include "strv.h" #include "terminal-util.h" @@ -1382,8 +1383,8 @@ static int status_ifindex(sd_bus *bus, int ifindex, const char *name, StatusMode _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; _cleanup_(sd_bus_message_unrefp) sd_bus_message *m = NULL; _cleanup_(link_info_clear) struct link_info link_info = {}; - _cleanup_free_ char *ifi = NULL, *p = NULL; - char ifname[IF_NAMESIZE + 1] = ""; + _cleanup_free_ char *p = NULL; + char ifi[DECIMAL_STR_MAX(int)], ifname[IF_NAMESIZE + 1] = ""; char **i; int r; @@ -1397,9 +1398,7 @@ static int status_ifindex(sd_bus *bus, int ifindex, const char *name, StatusMode name = ifname; } - if (asprintf(&ifi, "%i", ifindex) < 0) - return log_oom(); - + xsprintf(ifi, "%i", ifindex); r = sd_bus_path_encode("/org/freedesktop/resolve1/link", ifi, &p); if (r < 0) return log_oom(); @@ -1819,6 +1818,7 @@ static int status_all(sd_bus *bus, StatusMode mode) { static int verb_status(int argc, char **argv, void *userdata) { sd_bus *bus = userdata; + _cleanup_(sd_netlink_unrefp) sd_netlink *rtnl = NULL; int r = 0; if (argc > 1) { @@ -1828,7 +1828,7 @@ static int verb_status(int argc, char **argv, void *userdata) { STRV_FOREACH(ifname, argv + 1) { int ifindex, q; - ifindex = resolve_interface(NULL, *ifname); + ifindex = resolve_interface(&rtnl, *ifname); if (ifindex < 0) { log_warning_errno(ifindex, "Failed to resolve interface \"%s\", ignoring: %m", *ifname); continue; From 8571210a21d7a5299b5ba5c7da7acfa64af404ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sat, 11 Jan 2020 12:33:24 +0100 Subject: [PATCH 10/11] machinectl: reduce scope of iterator variables --- src/machine/machinectl.c | 43 +++++++++++++++++++--------------------- 1 file changed, 20 insertions(+), 23 deletions(-) diff --git a/src/machine/machinectl.c b/src/machine/machinectl.c index 4b02b0812d..191a897370 100644 --- a/src/machine/machinectl.c +++ b/src/machine/machinectl.c @@ -600,11 +600,9 @@ static void print_machine_status_info(sd_bus *bus, MachineStatusInfo *i) { printf("\t Root: %s\n", i->root_directory); if (i->n_netif > 0) { - size_t c; - fputs("\t Iface:", stdout); - for (c = 0; c < i->n_netif; c++) { + for (size_t c = 0; c < i->n_netif; c++) { char name[IF_NAMESIZE+1]; if (format_ifname(i->netif[c], name)) { @@ -746,7 +744,7 @@ static int show_machine(int argc, char *argv[], void *userdata) { _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; bool properties, new_line = false; sd_bus *bus = userdata; - int r = 0, i; + int r = 0; assert(bus); @@ -763,7 +761,7 @@ static int show_machine(int argc, char *argv[], void *userdata) { return r; } - for (i = 1; i < argc; i++) { + for (int i = 1; i < argc; i++) { const char *path = NULL; r = sd_bus_call_method(bus, @@ -1084,7 +1082,7 @@ static int show_image(int argc, char *argv[], void *userdata) { _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; bool properties, new_line = false; sd_bus *bus = userdata; - int r = 0, i; + int r = 0; assert(bus); @@ -1105,7 +1103,7 @@ static int show_image(int argc, char *argv[], void *userdata) { return r; } - for (i = 1; i < argc; i++) { + for (int i = 1; i < argc; i++) { const char *path = NULL; r = sd_bus_call_method( @@ -1136,7 +1134,7 @@ static int show_image(int argc, char *argv[], void *userdata) { static int kill_machine(int argc, char *argv[], void *userdata) { _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; sd_bus *bus = userdata; - int r, i; + int r; assert(bus); @@ -1145,7 +1143,7 @@ static int kill_machine(int argc, char *argv[], void *userdata) { if (!arg_kill_who) arg_kill_who = "all"; - for (i = 1; i < argc; i++) { + for (int i = 1; i < argc; i++) { r = sd_bus_call_method( bus, "org.freedesktop.machine1", @@ -1179,13 +1177,13 @@ static int poweroff_machine(int argc, char *argv[], void *userdata) { static int terminate_machine(int argc, char *argv[], void *userdata) { _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; sd_bus *bus = userdata; - int r, i; + int r; assert(bus); polkit_agent_open_if_enabled(arg_transport, arg_ask_password); - for (i = 1; i < argc; i++) { + for (int i = 1; i < argc; i++) { r = sd_bus_call_method( bus, "org.freedesktop.machine1", @@ -1550,13 +1548,13 @@ static int shell_machine(int argc, char *argv[], void *userdata) { static int remove_image(int argc, char *argv[], void *userdata) { sd_bus *bus = userdata; - int r, i; + int r; assert(bus); polkit_agent_open_if_enabled(arg_transport, arg_ask_password); - for (i = 1; i < argc; i++) { + for (int i = 1; i < argc; i++) { _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; _cleanup_(sd_bus_message_unrefp) sd_bus_message *m = NULL; @@ -1718,7 +1716,7 @@ static int start_machine(int argc, char *argv[], void *userdata) { _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; _cleanup_(bus_wait_for_jobs_freep) BusWaitForJobs *w = NULL; sd_bus *bus = userdata; - int r, i; + int r; assert(bus); @@ -1729,7 +1727,7 @@ static int start_machine(int argc, char *argv[], void *userdata) { if (r < 0) return log_oom(); - for (i = 1; i < argc; i++) { + for (int i = 1; i < argc; i++) { _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; _cleanup_free_ char *unit = NULL; const char *object; @@ -1781,7 +1779,7 @@ static int enable_machine(int argc, char *argv[], void *userdata) { size_t n_changes = 0; const char *method = NULL; sd_bus *bus = userdata; - int r, i; + int r; assert(bus); @@ -1803,7 +1801,7 @@ static int enable_machine(int argc, char *argv[], void *userdata) { if (r < 0) return bus_log_create_error(r); - for (i = 1; i < argc; i++) { + for (int i = 1; i < argc; i++) { _cleanup_free_ char *unit = NULL; r = make_service_name(argv[i], &unit); @@ -2444,7 +2442,7 @@ static int list_transfers(int argc, char *argv[], void *userdata) { _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; _cleanup_free_ TransferInfo *transfers = NULL; - size_t n_transfers = 0, n_allocated = 0, j; + size_t n_transfers = 0, n_allocated = 0; const char *type, *remote, *local; sd_bus *bus = userdata; uint32_t id, max_id = 0; @@ -2514,7 +2512,7 @@ static int list_transfers(int argc, char *argv[], void *userdata) { (int) max_local, "LOCAL", (int) max_remote, "REMOTE"); - for (j = 0; j < n_transfers; j++) + for (size_t j = 0; j < n_transfers; j++) if (transfers[j].progress < 0) printf("%*" PRIu32 " %*s %-*s %-*s %-*s\n", @@ -2544,13 +2542,13 @@ static int list_transfers(int argc, char *argv[], void *userdata) { static int cancel_transfer(int argc, char *argv[], void *userdata) { _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; sd_bus *bus = userdata; - int r, i; + int r; assert(bus); polkit_agent_open_if_enabled(arg_transport, arg_ask_password); - for (i = 1; i < argc; i++) { + for (int i = 1; i < argc; i++) { uint32_t id; r = safe_atou32(argv[i], &id); @@ -3040,7 +3038,6 @@ static int parse_argv(int argc, char *argv[]) { done: if (shell >= 0) { char *t; - int i; /* We found the "shell" verb while processing the argument list. Since we turned off reordering of the * argument list initially let's readjust it now, and move the "shell" verb to the back. */ @@ -3048,7 +3045,7 @@ done: optind -= 1; /* place the option index where the "shell" verb will be placed */ t = argv[shell]; - for (i = shell; i < optind; i++) + for (int i = shell; i < optind; i++) argv[i] = argv[i+1]; argv[optind] = t; } From 117caf37655727febe1a477bf4f37850a49a8568 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sat, 11 Jan 2020 12:36:21 +0100 Subject: [PATCH 11/11] networkctl: break long line --- src/network/networkctl.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/network/networkctl.c b/src/network/networkctl.c index e186515205..d0515dbe85 100644 --- a/src/network/networkctl.c +++ b/src/network/networkctl.c @@ -2007,7 +2007,8 @@ static int verb_reconfigure(int argc, char *argv[], void *userdata) { if (r < 0) { char ifname[IF_NAMESIZE + 1]; - return log_error_errno(r, "Failed to reconfigure network interface %s: %m", format_ifname_full(index, ifname, FORMAT_IFNAME_IFINDEX)); + return log_error_errno(r, "Failed to reconfigure network interface %s: %m", + format_ifname_full(index, ifname, FORMAT_IFNAME_IFINDEX)); } }