From 2206aa5c35a20f923b6b80294725085833b86ce3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sun, 24 May 2020 19:06:12 +0200 Subject: [PATCH 1/5] sd-network: fix inverted error message We get -ENOMSG when there is no lease. --- src/network/networkd-link.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c index 32fe86045d..fbabe67868 100644 --- a/src/network/networkd-link.c +++ b/src/network/networkd-link.c @@ -4069,7 +4069,7 @@ int link_save(Link *link) { if (link->dhcp6_client) { r = sd_dhcp6_client_get_lease(link->dhcp6_client, &dhcp6_lease); if (r < 0 && r != -ENOMSG) - log_link_debug(link, "No DHCPv6 lease"); + log_link_debug_errno(link, r, "Failed to get DHCPv6 lease: %m"); } fprintf(f, "NETWORK_FILE=%s\n", link->network->filename); From dddc8d1e1e69b5a941cffcaa03ab2cf60310ece4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sun, 24 May 2020 19:18:39 +0200 Subject: [PATCH 2/5] sd-network: reduce scope of some variables --- src/libsystemd-network/network-internal.c | 19 ++++++------------- src/network/networkd-link.c | 6 ++---- 2 files changed, 8 insertions(+), 17 deletions(-) diff --git a/src/libsystemd-network/network-internal.c b/src/libsystemd-network/network-internal.c index 13a0a5d929..c23e439250 100644 --- a/src/libsystemd-network/network-internal.c +++ b/src/libsystemd-network/network-internal.c @@ -658,15 +658,12 @@ size_t serialize_in_addrs(FILE *f, size_t size, bool with_leading_space, bool (*predicate)(const struct in_addr *addr)) { - size_t count; - size_t i; - assert(f); assert(addresses); - count = 0; + size_t count = 0; - for (i = 0; i < size; i++) { + for (size_t i = 0; i < size; i++) { char sbuf[INET_ADDRSTRLEN]; if (predicate && !predicate(&addresses[i])) @@ -719,13 +716,11 @@ int deserialize_in_addrs(struct in_addr **ret, const char *string) { } void serialize_in6_addrs(FILE *f, const struct in6_addr *addresses, size_t size) { - unsigned i; - assert(f); assert(addresses); assert(size); - for (i = 0; i < size; i++) { + for (size_t i = 0; i < size; i++) { char buffer[INET6_ADDRSTRLEN]; fputs(inet_ntop(AF_INET6, addresses+i, buffer, sizeof(buffer)), f); @@ -772,8 +767,6 @@ int deserialize_in6_addrs(struct in6_addr **ret, const char *string) { } void serialize_dhcp_routes(FILE *f, const char *key, sd_dhcp_route **routes, size_t size) { - unsigned i; - assert(f); assert(key); assert(routes); @@ -781,7 +774,7 @@ void serialize_dhcp_routes(FILE *f, const char *key, sd_dhcp_route **routes, siz fprintf(f, "%s=", key); - for (i = 0; i < size; i++) { + for (size_t i = 0; i < size; i++) { char sbuf[INET_ADDRSTRLEN]; struct in_addr dest, gw; uint8_t length; @@ -790,8 +783,8 @@ void serialize_dhcp_routes(FILE *f, const char *key, sd_dhcp_route **routes, siz assert_se(sd_dhcp_route_get_gateway(routes[i], &gw) >= 0); assert_se(sd_dhcp_route_get_destination_prefix_length(routes[i], &length) >= 0); - fprintf(f, "%s/%" PRIu8, inet_ntop(AF_INET, &dest, sbuf, sizeof(sbuf)), length); - fprintf(f, ",%s%s", inet_ntop(AF_INET, &gw, sbuf, sizeof(sbuf)), (i < (size - 1)) ? " ": ""); + fprintf(f, "%s/%" PRIu8, inet_ntop(AF_INET, &dest, sbuf, sizeof sbuf), length); + fprintf(f, ",%s%s", inet_ntop(AF_INET, &gw, sbuf, sizeof sbuf), i < size - 1 ? " ": ""); } fputs("\n", f); diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c index fbabe67868..43b5d8e3c9 100644 --- a/src/network/networkd-link.c +++ b/src/network/networkd-link.c @@ -3982,11 +3982,9 @@ static void print_link_hashmap(FILE *f, const char *prefix, Hashmap* h) { } static void link_save_dns(FILE *f, struct in_addr_data *dns, unsigned n_dns, bool *space) { - unsigned j; - int r; - - for (j = 0; j < n_dns; j++) { + for (unsigned j = 0; j < n_dns; j++) { _cleanup_free_ char *b = NULL; + int r; r = in_addr_to_string(dns[j].family, &dns[j].address, &b); if (r < 0) { From 00813316b0e96963d6c975a32a2a825a3d3bad96 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sun, 24 May 2020 21:51:39 +0200 Subject: [PATCH 3/5] sd-dhcp6: constify output arguments in get_{ntp,nds}_addr This matches what we do for ipv4 and is in general better. --- src/libsystemd-network/sd-dhcp6-lease.c | 4 ++-- src/libsystemd-network/test-dhcp6-client.c | 6 +++--- src/network/networkd-link.c | 4 ++-- src/systemd/sd-dhcp6-lease.h | 5 ++--- 4 files changed, 9 insertions(+), 10 deletions(-) diff --git a/src/libsystemd-network/sd-dhcp6-lease.c b/src/libsystemd-network/sd-dhcp6-lease.c index 8aebb53c87..4eee10ea89 100644 --- a/src/libsystemd-network/sd-dhcp6-lease.c +++ b/src/libsystemd-network/sd-dhcp6-lease.c @@ -213,7 +213,7 @@ int dhcp6_lease_set_dns(sd_dhcp6_lease *lease, uint8_t *optval, size_t optlen) { return 0; } -int sd_dhcp6_lease_get_dns(sd_dhcp6_lease *lease, struct in6_addr **addrs) { +int sd_dhcp6_lease_get_dns(sd_dhcp6_lease *lease, const struct in6_addr **addrs) { assert_return(lease, -EINVAL); assert_return(addrs, -EINVAL); @@ -341,7 +341,7 @@ int dhcp6_lease_set_sntp(sd_dhcp6_lease *lease, uint8_t *optval, size_t optlen) } int sd_dhcp6_lease_get_ntp_addrs(sd_dhcp6_lease *lease, - struct in6_addr **addrs) { + const struct in6_addr **addrs) { assert_return(lease, -EINVAL); assert_return(addrs, -EINVAL); diff --git a/src/libsystemd-network/test-dhcp6-client.c b/src/libsystemd-network/test-dhcp6-client.c index 3a68f1fe71..4b40e31c12 100644 --- a/src/libsystemd-network/test-dhcp6-client.c +++ b/src/libsystemd-network/test-dhcp6-client.c @@ -371,7 +371,7 @@ static int test_advertise_option(sd_event *e) { int r; uint8_t *opt; bool opt_clientid = false; - struct in6_addr *addrs; + const struct in6_addr *addrs; char **domains; log_debug("/* %s */", __func__); @@ -518,7 +518,7 @@ static void test_client_solicit_cb(sd_dhcp6_client *client, int event, void *userdata) { sd_event *e = userdata; sd_dhcp6_lease *lease; - struct in6_addr *addrs; + const struct in6_addr *addrs; char **domains; log_debug("/* %s */", __func__); @@ -744,7 +744,7 @@ static void test_client_information_cb(sd_dhcp6_client *client, int event, void *userdata) { sd_event *e = userdata; sd_dhcp6_lease *lease; - struct in6_addr *addrs; + const struct in6_addr *addrs; struct in6_addr address = { { { 0xfe, 0x80, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0x01 } } }; char **domains; diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c index 43b5d8e3c9..daa9683a78 100644 --- a/src/network/networkd-link.c +++ b/src/network/networkd-link.c @@ -4091,7 +4091,7 @@ int link_save(Link *link) { } if (link->network->dhcp6_use_dns && dhcp6_lease) { - struct in6_addr *in6_addrs; + const struct in6_addr *in6_addrs; r = sd_dhcp6_lease_get_dns(dhcp6_lease, &in6_addrs); if (r > 0) { @@ -4195,7 +4195,7 @@ int link_save(Link *link) { } if (link->network->dhcp6_use_ntp && dhcp6_lease) { - struct in6_addr *in6_addrs; + const struct in6_addr *in6_addrs; char **hosts; r = sd_dhcp6_lease_get_ntp_addrs(dhcp6_lease, diff --git a/src/systemd/sd-dhcp6-lease.h b/src/systemd/sd-dhcp6-lease.h index 33a32a6dc5..4301c6db87 100644 --- a/src/systemd/sd-dhcp6-lease.h +++ b/src/systemd/sd-dhcp6-lease.h @@ -39,10 +39,9 @@ int sd_dhcp6_lease_get_pd(sd_dhcp6_lease *lease, struct in6_addr *prefix, uint32_t *lifetime_preferred, uint32_t *lifetime_valid); -int sd_dhcp6_lease_get_dns(sd_dhcp6_lease *lease, struct in6_addr **addrs); +int sd_dhcp6_lease_get_dns(sd_dhcp6_lease *lease, const struct in6_addr **addrs); int sd_dhcp6_lease_get_domains(sd_dhcp6_lease *lease, char ***domains); -int sd_dhcp6_lease_get_ntp_addrs(sd_dhcp6_lease *lease, - struct in6_addr **addrs); +int sd_dhcp6_lease_get_ntp_addrs(sd_dhcp6_lease *lease, const struct in6_addr **addrs); int sd_dhcp6_lease_get_ntp_fqdn(sd_dhcp6_lease *lease, char ***ntp_fqdn); sd_dhcp6_lease *sd_dhcp6_lease_ref(sd_dhcp6_lease *lease); From d5e172d2fb2d80b4dc4542089b30d1d19032ff74 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sun, 24 May 2020 22:02:47 +0200 Subject: [PATCH 4/5] networkd: unfoobar serialization of links We'd start writing an entry line, then another one, then another one, and then output the rest of the first one, and then some other random stuff, and the rest of some other lines... Results were ...eh... random. Let's define a helper to avoid some of the copy&paste madness, and separate blocks that output a single line with /**********************************/. This rework doesn't change what data is written, it only tries to fix the format of the output. The fact that some entries only write data from link->network, and some from either link->network or link, some stuff only for dhpc4 leases while some for both dhpc4 and dhcp6, etc, looks rather suspicious too, but I didn't touch this. --- TODO | 2 + src/network/networkd-link.c | 291 ++++++++++++++++++------------------ 2 files changed, 149 insertions(+), 144 deletions(-) diff --git a/TODO b/TODO index e4182588fe..d80845b616 100644 --- a/TODO +++ b/TODO @@ -1227,6 +1227,8 @@ Features: - duplicate address check for static IPs (like ARPCHECK in network-scripts) - whenever uplink info changes, make DHCP server send out FORCERENEW +* Figure out how to do unittests of networkd's state serialization + * dhcp: - figure out how much we can increase Maximum Message Size diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c index daa9683a78..926bde7d48 100644 --- a/src/network/networkd-link.c +++ b/src/network/networkd-link.c @@ -3999,6 +3999,61 @@ static void link_save_dns(FILE *f, struct in_addr_data *dns, unsigned n_dns, boo } } +static void serialize_addresses( + FILE *f, + const char *lvalue, + bool *space, + char **addresses, + sd_dhcp_lease *lease, + bool conditional, + sd_dhcp_lease_info what, + sd_dhcp6_lease *lease6, + bool conditional6, + int (*lease6_get_addr)(sd_dhcp6_lease*, const struct in6_addr**), + int (*lease6_get_fqdn)(sd_dhcp6_lease*, char ***)) { + int r; + + bool _space = false; + if (!space) + space = &_space; + + if (lvalue) + fprintf(f, "%s=", lvalue); + fputstrv(f, addresses, NULL, space); + + if (lease && conditional) { + const struct in_addr *lease_addresses; + + r = sd_dhcp_lease_get_servers(lease, what, &lease_addresses); + if (r > 0) + if (serialize_in_addrs(f, lease_addresses, r, space, in4_addr_is_non_local) > 0) + *space = true; + } + + if (lease6 && conditional6 && lease6_get_addr) { + const struct in6_addr *in6_addrs; + + r = lease6_get_addr(lease6, &in6_addrs); + if (r > 0) { + if (*space) + fputc(' ', f); + serialize_in6_addrs(f, in6_addrs, r); + *space = true; + } + } + + if (lease6 && conditional6 && lease6_get_fqdn) { + char **in6_hosts; + + r = lease6_get_fqdn(lease6, &in6_hosts); + if (r > 0) + fputstrv(f, in6_hosts, NULL, space); + } + + if (lvalue) + fputc('\n', f); +} + int link_save(Link *link) { const char *admin_state, *oper_state, *carrier_state, *address_state; _cleanup_free_ char *temp_path = NULL; @@ -4055,14 +4110,11 @@ int link_save(Link *link) { fprintf(f, "REQUIRED_FOR_ONLINE=%s\n", yes_no(link->network->required_for_online)); - fprintf(f, "REQUIRED_OPER_STATE_FOR_ONLINE=%s", - strempty(link_operstate_to_string(link->network->required_operstate_for_online.min))); - - if (link->network->required_operstate_for_online.max != LINK_OPERSTATE_RANGE_DEFAULT.max) - fprintf(f, ":%s", - strempty(link_operstate_to_string(link->network->required_operstate_for_online.max))); - - fprintf(f, "\n"); + LinkOperationalStateRange st = link->network->required_operstate_for_online; + fprintf(f, "REQUIRED_OPER_STATE_FOR_ONLINE=%s%s%s\n", + strempty(link_operstate_to_string(st.min)), + st.max != LINK_OPERSTATE_RANGE_DEFAULT.max ? ":" : "", + st.max != LINK_OPERSTATE_RANGE_DEFAULT.max ? strempty(link_operstate_to_string(st.max)) : ""); if (link->dhcp6_client) { r = sd_dhcp6_client_get_lease(link->dhcp6_client, &dhcp6_lease); @@ -4072,35 +4124,24 @@ int link_save(Link *link) { fprintf(f, "NETWORK_FILE=%s\n", link->network->filename); + /************************************************************/ + fputs("DNS=", f); space = false; - if (link->n_dns != (unsigned) -1) link_save_dns(f, link->dns, link->n_dns, &space); else link_save_dns(f, link->network->dns, link->network->n_dns, &space); - if (link->network->dhcp_use_dns && - link->dhcp_lease) { - const struct in_addr *addresses; - - r = sd_dhcp_lease_get_dns(link->dhcp_lease, &addresses); - if (r > 0) - if (serialize_in_addrs(f, addresses, r, space, in4_addr_is_non_local) > 0) - space = true; - } - - if (link->network->dhcp6_use_dns && dhcp6_lease) { - const struct in6_addr *in6_addrs; - - r = sd_dhcp6_lease_get_dns(dhcp6_lease, &in6_addrs); - if (r > 0) { - if (space) - fputc(' ', f); - serialize_in6_addrs(f, in6_addrs, r); - space = true; - } - } + serialize_addresses(f, NULL, &space, + NULL, + link->dhcp_lease, + link->network->dhcp_use_dns, + SD_DHCP_LEASE_DNS_SERVERS, + dhcp6_lease, + link->network->dhcp6_use_dns, + sd_dhcp6_lease_get_dns, + NULL); /* Make sure to flush out old entries before we use the NDISC data */ ndisc_vacuum(link); @@ -4119,100 +4160,47 @@ int link_save(Link *link) { fputc('\n', f); - fputs("NTP=", f); - space = false; - fputstrv(f, link->ntp ?: link->network->ntp, NULL, &space); + /************************************************************/ - if (link->network->dhcp_use_ntp && - link->dhcp_lease) { - const struct in_addr *addresses; + serialize_addresses(f, "NTP", NULL, + link->ntp ?: link->network->ntp, + link->dhcp_lease, + link->network->dhcp_use_ntp, + SD_DHCP_LEASE_NTP_SERVERS, + dhcp6_lease, + link->network->dhcp6_use_ntp, + sd_dhcp6_lease_get_ntp_addrs, + sd_dhcp6_lease_get_ntp_fqdn); - r = sd_dhcp_lease_get_ntp(link->dhcp_lease, &addresses); - if (r > 0) - if (serialize_in_addrs(f, addresses, r, space, in4_addr_is_non_local) > 0) - space = true; - } + serialize_addresses(f, "SIP", NULL, + link->network->sip, + link->dhcp_lease, + link->network->dhcp_use_sip, + SD_DHCP_LEASE_SIP_SERVERS, + false, NULL, NULL, NULL); - fputc('\n', f); + serialize_addresses(f, "POP3_SERVERS", NULL, + link->network->pop3, + link->dhcp_lease, + true, + SD_DHCP_LEASE_POP3_SERVERS, + false, NULL, NULL, NULL); - fputs("SIP=", f); - space = false; - fputstrv(f, link->network->sip, NULL, &space); + serialize_addresses(f, "SMTP_SERVERS", NULL, + link->network->smtp, + link->dhcp_lease, + true, + SD_DHCP_LEASE_SMTP_SERVERS, + false, NULL, NULL, NULL); - if (link->network->dhcp_use_sip && - link->dhcp_lease) { - const struct in_addr *addresses; + serialize_addresses(f, "LPR_SERVERS", NULL, + link->network->lpr, + link->dhcp_lease, + true, + SD_DHCP_LEASE_LPR_SERVERS, + false, NULL, NULL, NULL); - r = sd_dhcp_lease_get_sip(link->dhcp_lease, &addresses); - if (r > 0) - if (serialize_in_addrs(f, addresses, r, space, in4_addr_is_non_local) > 0) - space = true; - } - - fputc('\n', f); - - fputs("POP3_SERVERS=", f); - space = false; - fputstrv(f, link->network->pop3, NULL, &space); - - fputc('\n', f); - - fputs("SMTP_SERVERS=", f); - space = false; - fputstrv(f, link->network->smtp, NULL, &space); - - fputc('\n', f); - - fputs("LPR_SERVERS=", f); - space = false; - fputstrv(f, link->network->lpr, NULL, &space); - - if (link->dhcp_lease) { - const struct in_addr *addresses; - - r = sd_dhcp_lease_get_pop3_server(link->dhcp_lease, &addresses); - if (r > 0) - if (serialize_in_addrs(f, addresses, r, space, in4_addr_is_non_local) > 0) - space = true; - } - - if (link->dhcp_lease) { - const struct in_addr *addresses; - - r = sd_dhcp_lease_get_smtp_server(link->dhcp_lease, &addresses); - if (r > 0) - if (serialize_in_addrs(f, addresses, r, space, in4_addr_is_non_local) > 0) - space = true; - } - - if (link->dhcp_lease) { - const struct in_addr *addresses; - - r = sd_dhcp_lease_get_lpr_servers(link->dhcp_lease, &addresses); - if (r > 0) - if (serialize_in_addrs(f, addresses, r, space, in4_addr_is_non_local) > 0) - space = true; - } - - if (link->network->dhcp6_use_ntp && dhcp6_lease) { - const struct in6_addr *in6_addrs; - char **hosts; - - r = sd_dhcp6_lease_get_ntp_addrs(dhcp6_lease, - &in6_addrs); - if (r > 0) { - if (space) - fputc(' ', f); - serialize_in6_addrs(f, in6_addrs, r); - space = true; - } - - r = sd_dhcp6_lease_get_ntp_fqdn(dhcp6_lease, &hosts); - if (r > 0) - fputstrv(f, hosts, NULL, &space); - } - - fputc('\n', f); + /************************************************************/ if (link->network->dhcp_use_domains != DHCP_USE_DOMAINS_NO) { if (link->dhcp_lease) { @@ -4246,6 +4234,8 @@ int link_save(Link *link) { fputc('\n', f); + /************************************************************/ + fputs("ROUTE_DOMAINS=", f); space = false; ORDERED_SET_FOREACH(p, link->route_domains ?: link->network->route_domains, i) @@ -4269,47 +4259,58 @@ int link_save(Link *link) { fputc('\n', f); + /************************************************************/ + fprintf(f, "LLMNR=%s\n", resolve_support_to_string(link->llmnr >= 0 ? link->llmnr : link->network->llmnr)); + + /************************************************************/ + fprintf(f, "MDNS=%s\n", resolve_support_to_string(link->mdns >= 0 ? link->mdns : link->network->mdns)); - if (link->dns_default_route >= 0) - fprintf(f, "DNS_DEFAULT_ROUTE=%s\n", yes_no(link->dns_default_route)); - else if (link->network->dns_default_route >= 0) - fprintf(f, "DNS_DEFAULT_ROUTE=%s\n", yes_no(link->network->dns_default_route)); - if (link->dns_over_tls_mode != _DNS_OVER_TLS_MODE_INVALID) - fprintf(f, "DNS_OVER_TLS=%s\n", - dns_over_tls_mode_to_string(link->dns_over_tls_mode)); - else if (link->network->dns_over_tls_mode != _DNS_OVER_TLS_MODE_INVALID) - fprintf(f, "DNS_OVER_TLS=%s\n", - dns_over_tls_mode_to_string(link->network->dns_over_tls_mode)); + /************************************************************/ - if (link->dnssec_mode != _DNSSEC_MODE_INVALID) - fprintf(f, "DNSSEC=%s\n", - dnssec_mode_to_string(link->dnssec_mode)); - else if (link->network->dnssec_mode != _DNSSEC_MODE_INVALID) - fprintf(f, "DNSSEC=%s\n", - dnssec_mode_to_string(link->network->dnssec_mode)); + int dns_default_route = + link->dns_default_route >= 0 ? link->dns_default_route : + link->network->dns_default_route; + if (dns_default_route >= 0) + fprintf(f, "DNS_DEFAULT_ROUTE=%s\n", yes_no(dns_default_route)); - if (!set_isempty(link->dnssec_negative_trust_anchors)) { + /************************************************************/ + + DnsOverTlsMode dns_over_tls_mode = + link->dns_over_tls_mode != _DNS_OVER_TLS_MODE_INVALID ? link->dns_over_tls_mode : + link->network->dns_over_tls_mode; + if (dns_over_tls_mode != _DNS_OVER_TLS_MODE_INVALID) + fprintf(f, "DNS_OVER_TLS=%s\n", dns_over_tls_mode_to_string(dns_over_tls_mode)); + + /************************************************************/ + + DnssecMode dnssec_mode = + link->dnssec_mode != _DNSSEC_MODE_INVALID ? link->dnssec_mode : + link->network->dnssec_mode; + if (dnssec_mode != _DNSSEC_MODE_INVALID) + fprintf(f, "DNSSEC=%s\n", dnssec_mode_to_string(dnssec_mode)); + + /************************************************************/ + + Set *nta_anchors = link->dnssec_negative_trust_anchors; + if (set_isempty(nta_anchors)) + nta_anchors = link->network->dnssec_negative_trust_anchors; + + if (!set_isempty(nta_anchors)) { const char *n; fputs("DNSSEC_NTA=", f); space = false; - SET_FOREACH(n, link->dnssec_negative_trust_anchors, i) - fputs_with_space(f, n, NULL, &space); - fputc('\n', f); - } else if (!set_isempty(link->network->dnssec_negative_trust_anchors)) { - const char *n; - - fputs("DNSSEC_NTA=", f); - space = false; - SET_FOREACH(n, link->network->dnssec_negative_trust_anchors, i) + SET_FOREACH(n, nta_anchors, i) fputs_with_space(f, n, NULL, &space); fputc('\n', f); } + /************************************************************/ + fputs("ADDRESSES=", f); space = false; SET_FOREACH(a, link->addresses, i) { @@ -4324,6 +4325,8 @@ int link_save(Link *link) { } fputc('\n', f); + /************************************************************/ + fputs("ROUTES=", f); space = false; SET_FOREACH(route, link->routes, i) { From d8bff5cc37bf34517691ecb8856603250eceb607 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 26 May 2020 10:19:31 +0200 Subject: [PATCH 5/5] network: simplify how initial space is handled --- src/libsystemd-network/network-internal.c | 23 +++++++++++++++-------- src/libsystemd-network/network-internal.h | 5 +++-- src/network/networkd-link.c | 22 ++++++---------------- 3 files changed, 24 insertions(+), 26 deletions(-) diff --git a/src/libsystemd-network/network-internal.c b/src/libsystemd-network/network-internal.c index c23e439250..3bdabd99fb 100644 --- a/src/libsystemd-network/network-internal.c +++ b/src/libsystemd-network/network-internal.c @@ -656,24 +656,27 @@ int config_parse_bridge_port_priority( size_t serialize_in_addrs(FILE *f, const struct in_addr *addresses, size_t size, - bool with_leading_space, + bool *with_leading_space, bool (*predicate)(const struct in_addr *addr)) { assert(f); assert(addresses); size_t count = 0; + bool _space = false; + if (!with_leading_space) + with_leading_space = &_space; for (size_t i = 0; i < size; i++) { char sbuf[INET_ADDRSTRLEN]; if (predicate && !predicate(&addresses[i])) continue; - if (with_leading_space) + + if (*with_leading_space) fputc(' ', f); - else - with_leading_space = true; fputs(inet_ntop(AF_INET, &addresses[i], sbuf, sizeof(sbuf)), f); count++; + *with_leading_space = true; } return count; @@ -715,18 +718,22 @@ int deserialize_in_addrs(struct in_addr **ret, const char *string) { return size; } -void serialize_in6_addrs(FILE *f, const struct in6_addr *addresses, size_t size) { +void serialize_in6_addrs(FILE *f, const struct in6_addr *addresses, size_t size, bool *with_leading_space) { assert(f); assert(addresses); assert(size); + bool _space = false; + if (!with_leading_space) + with_leading_space = &_space; + for (size_t i = 0; i < size; i++) { char buffer[INET6_ADDRSTRLEN]; - fputs(inet_ntop(AF_INET6, addresses+i, buffer, sizeof(buffer)), f); - - if (i < size - 1) + if (*with_leading_space) fputc(' ', f); + fputs(inet_ntop(AF_INET6, addresses+i, buffer, sizeof(buffer)), f); + *with_leading_space = true; } } diff --git a/src/libsystemd-network/network-internal.h b/src/libsystemd-network/network-internal.h index 16ff173ac6..c413afc7d5 100644 --- a/src/libsystemd-network/network-internal.h +++ b/src/libsystemd-network/network-internal.h @@ -50,11 +50,12 @@ const char *net_get_name_persistent(sd_device *device); size_t serialize_in_addrs(FILE *f, const struct in_addr *addresses, size_t size, - bool with_leading_space, + bool *with_leading_space, bool (*predicate)(const struct in_addr *addr)); int deserialize_in_addrs(struct in_addr **addresses, const char *string); void serialize_in6_addrs(FILE *f, const struct in6_addr *addresses, - size_t size); + size_t size, + bool *with_leading_space); int deserialize_in6_addrs(struct in6_addr **addresses, const char *string); /* don't include "dhcp-lease-internal.h" as it causes conflicts between netinet/ip.h and linux/ip.h */ diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c index 926bde7d48..482cc2a44c 100644 --- a/src/network/networkd-link.c +++ b/src/network/networkd-link.c @@ -4026,20 +4026,15 @@ static void serialize_addresses( r = sd_dhcp_lease_get_servers(lease, what, &lease_addresses); if (r > 0) - if (serialize_in_addrs(f, lease_addresses, r, space, in4_addr_is_non_local) > 0) - *space = true; + serialize_in_addrs(f, lease_addresses, r, space, in4_addr_is_non_local); } if (lease6 && conditional6 && lease6_get_addr) { const struct in6_addr *in6_addrs; r = lease6_get_addr(lease6, &in6_addrs); - if (r > 0) { - if (*space) - fputc(' ', f); - serialize_in6_addrs(f, in6_addrs, r); - *space = true; - } + if (r > 0) + serialize_in6_addrs(f, in6_addrs, r, space); } if (lease6 && conditional6 && lease6_get_fqdn) { @@ -4149,13 +4144,8 @@ int link_save(Link *link) { if (link->network->ipv6_accept_ra_use_dns && link->ndisc_rdnss) { NDiscRDNSS *dd; - SET_FOREACH(dd, link->ndisc_rdnss, i) { - if (space) - fputc(' ', f); - - serialize_in6_addrs(f, &dd->address, 1); - space = true; - } + SET_FOREACH(dd, link->ndisc_rdnss, i) + serialize_in6_addrs(f, &dd->address, 1, &space); } fputc('\n', f); @@ -4361,7 +4351,7 @@ int link_save(Link *link) { r = sd_dhcp_lease_get_address(link->dhcp_lease, &address); if (r >= 0) { fputs("DHCP4_ADDRESS=", f); - serialize_in_addrs(f, &address, 1, false, NULL); + serialize_in_addrs(f, &address, 1, NULL, NULL); fputc('\n', f); }