From 7c5023037815228280dcf461bf9b9f2b3575f600 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sun, 11 Oct 2020 11:54:18 +0200 Subject: [PATCH 1/9] resolvectl: break nta/domain/dns listings with newlines We would print the whole string as a single super-long line. Let's nicely break the text into lines that fit on the screen. $ COLUMNS=70 build/resolvectl --no-pager nta Global: home local intranet 23.172.in-addr.arpa lan 18.172.in-addr.arpa 16.172.in-addr.arpa 19.172.in-addr.arpa 25.172.in-addr.arpa 21.172.in-addr.arpa d.f.ip6.arpa 20.172.in-addr.arpa 30.172.in-addr.arpa 17.172.in-addr.arpa internal 168.192.in-addr.arpa 28.172.in-addr.arpa 22.172.in-addr.arpa 24.172.in-addr.arpa 26.172.in-addr.arpa corp 10.in-addr.arpa private 29.172.in-addr.arpa test 27.172.in-addr.arpa 31.172.in-addr.arpa Link 2 (hub0): Link 4 (enp0s31f6): Link 5 (wlp4s0): Link 7 (virbr0): adsfasdfasdfasd.com 21.172.in-addr.arpa lan j b a.com home d.f.ip6.arpa b.com local 16.172.in-addr.arpa 19.172.in-addr.arpa 18.172.in-addr.arpa 25.172.in-addr.arpa 20.172.in-addr.arpa k i h 23.172.in-addr.arpa 168.192.in-addr.arpa d g intranet 17.172.in-addr.arpa c e.com 30.172.in-addr.arpa a f d.com e internal Link 8 (virbr0-nic): Link 9 (vnet0): Link 10 (vb-rawhide): Link 15 (wwp0s20f0u2i12): --- src/basic/macro.h | 2 ++ src/resolve/resolvectl.c | 42 ++++++++++++++++++++++++---------------- 2 files changed, 27 insertions(+), 17 deletions(-) diff --git a/src/basic/macro.h b/src/basic/macro.h index 41c2c3289e..954bb2de2a 100644 --- a/src/basic/macro.h +++ b/src/basic/macro.h @@ -634,4 +634,6 @@ static inline int __coverity_check_and_return__(int condition) { _copy; \ }) +#define SIZE_ADD(x, y) ((x) >= SIZE_MAX - (y) ? SIZE_MAX : (x) + (y)) + #include "log.h" diff --git a/src/resolve/resolvectl.c b/src/resolve/resolvectl.c index 4581c2e3c8..f199df655d 100644 --- a/src/resolve/resolvectl.c +++ b/src/resolve/resolvectl.c @@ -35,6 +35,7 @@ #include "string-table.h" #include "strv.h" #include "terminal-util.h" +#include "utf8.h" #include "verbs.h" static int arg_family = AF_UNSPEC; @@ -1302,19 +1303,39 @@ static int map_link_domains(sd_bus *bus, const char *member, sd_bus_message *m, } static int status_print_strv_ifindex(int ifindex, const char *ifname, char **p) { + const unsigned indent = strlen("Global: "); /* Use the same indentation everywhere to make things nice */ + int pos1, pos2; + + if (ifname) + printf("%s%nLink %i (%s)%n%s:", ansi_highlight(), &pos1, ifindex, ifname, &pos2, ansi_normal()); + else + printf("%s%nGlobal%n%s:", ansi_highlight(), &pos1, &pos2, ansi_normal()); + + size_t cols = columns(), position = pos2 - pos1 + 2; char **i; - printf("%sLink %i (%s)%s:", - ansi_highlight(), ifindex, ifname, ansi_normal()); + STRV_FOREACH(i, p) { + size_t our_len = utf8_console_width(*i); /* This returns -1 on invalid utf-8 (which shouldn't happen). + * If that happens, we'll just print one item per line. */ - STRV_FOREACH(i, p) - printf(" %s", *i); + if (position <= indent || SIZE_ADD(SIZE_ADD(position, 1), our_len) < cols) { + printf(" %s", *i); + position = SIZE_ADD(SIZE_ADD(position, 1), our_len); + } else { + printf("\n%*s%s", indent, "", *i); + position = SIZE_ADD(our_len, indent); + } + } printf("\n"); return 0; } +static int status_print_strv_global(char **p) { + return status_print_strv_ifindex(0, NULL, p); +} + struct link_info { uint64_t scopes_mask; const char *llmnr; @@ -1636,19 +1657,6 @@ static int map_global_domains(sd_bus *bus, const char *member, sd_bus_message *m return 0; } -static int status_print_strv_global(char **p) { - char **i; - - printf("%sGlobal%s:", ansi_highlight(), ansi_normal()); - - STRV_FOREACH(i, p) - printf(" %s", *i); - - printf("\n"); - - return 0; -} - struct global_info { char *current_dns; char *current_dns_ex; From 2c91906e25ab0a4caa30f0bfaa1bdff6994cb9d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sun, 11 Oct 2020 12:55:10 +0200 Subject: [PATCH 2/9] man: add example of negative trust anchor file Fixes #17226. --- man/dnssec-trust-anchors.d.xml | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/man/dnssec-trust-anchors.d.xml b/man/dnssec-trust-anchors.d.xml index 8b6394e927..f14ebbce7c 100644 --- a/man/dnssec-trust-anchors.d.xml +++ b/man/dnssec-trust-anchors.d.xml @@ -138,7 +138,17 @@ and follow the same overriding rules. They are text files with the .negative suffix. Empty lines and lines whose first character is ; are ignored. Each line specifies one domain name which is the root of a DNS - subtree where validation shall be disabled. + subtree where validation shall be disabled. For example: + + # Reverse IPv4 mappings +10.in-addr.arpa +16.172.in-addr.arpa +168.192.in-addr.arpa +... +# Some custom domains +prod +stag + Negative trust anchors are useful to support private DNS subtrees that are not referenced from the Internet DNS hierarchy, From 80b8c3d7fd90e1e1943873c54dafc9e3b88fca94 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sun, 11 Oct 2020 13:46:53 +0200 Subject: [PATCH 3/9] resolvectl: add the usual typedef for struct link_info/global_info Also move the struct defintions up in preparation for further changes. --- src/resolve/resolvectl.c | 120 +++++++++++++++++++-------------------- 1 file changed, 60 insertions(+), 60 deletions(-) diff --git a/src/resolve/resolvectl.c b/src/resolve/resolvectl.c index f199df655d..523b0be1c3 100644 --- a/src/resolve/resolvectl.c +++ b/src/resolve/resolvectl.c @@ -1336,7 +1336,7 @@ static int status_print_strv_global(char **p) { return status_print_strv_ifindex(0, NULL, p); } -struct link_info { +typedef struct LinkInfo { uint64_t scopes_mask; const char *llmnr; const char *mdns; @@ -1350,9 +1350,26 @@ struct link_info { char **ntas; bool dnssec_supported; bool default_route; -}; +} LinkInfo; -static void link_info_clear(struct link_info *p) { +typedef struct GlobalInfo { + char *current_dns; + char *current_dns_ex; + char **dns; + char **dns_ex; + char **fallback_dns; + char **fallback_dns_ex; + char **domains; + char **ntas; + const char *llmnr; + const char *mdns; + const char *dns_over_tls; + const char *dnssec; + const char *resolv_conf_mode; + bool dnssec_supported; +} GlobalInfo; + +static void link_info_clear(LinkInfo *p) { free(p->current_dns); free(p->current_dns_ex); strv_free(p->dns); @@ -1361,6 +1378,17 @@ static void link_info_clear(struct link_info *p) { strv_free(p->ntas); } +static void global_info_clear(GlobalInfo *p) { + free(p->current_dns); + free(p->current_dns_ex); + strv_free(p->dns); + strv_free(p->dns_ex); + strv_free(p->fallback_dns); + strv_free(p->fallback_dns_ex); + strv_free(p->domains); + strv_free(p->ntas); +} + static int dump_list(Table *table, const char *prefix, char * const *l) { int r; @@ -1378,24 +1406,24 @@ static int dump_list(Table *table, const char *prefix, char * const *l) { static int status_ifindex(sd_bus *bus, int ifindex, const char *name, StatusMode mode, bool *empty_line) { static const struct bus_properties_map property_map[] = { - { "ScopesMask", "t", NULL, offsetof(struct link_info, scopes_mask) }, - { "DNS", "a(iay)", map_link_dns_servers, offsetof(struct link_info, dns) }, - { "DNSEx", "a(iayqs)", map_link_dns_servers_ex, offsetof(struct link_info, dns_ex) }, - { "CurrentDNSServer", "(iay)", map_link_current_dns_server, offsetof(struct link_info, current_dns) }, - { "CurrentDNSServerEx", "(iayqs)", map_link_current_dns_server_ex, offsetof(struct link_info, current_dns_ex) }, - { "Domains", "a(sb)", map_link_domains, offsetof(struct link_info, domains) }, - { "DefaultRoute", "b", NULL, offsetof(struct link_info, default_route) }, - { "LLMNR", "s", NULL, offsetof(struct link_info, llmnr) }, - { "MulticastDNS", "s", NULL, offsetof(struct link_info, mdns) }, - { "DNSOverTLS", "s", NULL, offsetof(struct link_info, dns_over_tls) }, - { "DNSSEC", "s", NULL, offsetof(struct link_info, dnssec) }, - { "DNSSECNegativeTrustAnchors", "as", NULL, offsetof(struct link_info, ntas) }, - { "DNSSECSupported", "b", NULL, offsetof(struct link_info, dnssec_supported) }, + { "ScopesMask", "t", NULL, offsetof(LinkInfo, scopes_mask) }, + { "DNS", "a(iay)", map_link_dns_servers, offsetof(LinkInfo, dns) }, + { "DNSEx", "a(iayqs)", map_link_dns_servers_ex, offsetof(LinkInfo, dns_ex) }, + { "CurrentDNSServer", "(iay)", map_link_current_dns_server, offsetof(LinkInfo, current_dns) }, + { "CurrentDNSServerEx", "(iayqs)", map_link_current_dns_server_ex, offsetof(LinkInfo, current_dns_ex) }, + { "Domains", "a(sb)", map_link_domains, offsetof(LinkInfo, domains) }, + { "DefaultRoute", "b", NULL, offsetof(LinkInfo, default_route) }, + { "LLMNR", "s", NULL, offsetof(LinkInfo, llmnr) }, + { "MulticastDNS", "s", NULL, offsetof(LinkInfo, mdns) }, + { "DNSOverTLS", "s", NULL, offsetof(LinkInfo, dns_over_tls) }, + { "DNSSEC", "s", NULL, offsetof(LinkInfo, dnssec) }, + { "DNSSECNegativeTrustAnchors", "as", NULL, offsetof(LinkInfo, ntas) }, + { "DNSSECSupported", "b", NULL, offsetof(LinkInfo, dnssec_supported) }, {} }; _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_(link_info_clear) LinkInfo link_info = {}; _cleanup_(table_unrefp) Table *table = NULL; _cleanup_free_ char *p = NULL; char ifi[DECIMAL_STR_MAX(int)], ifname[IF_NAMESIZE + 1] = ""; @@ -1657,55 +1685,27 @@ static int map_global_domains(sd_bus *bus, const char *member, sd_bus_message *m return 0; } -struct global_info { - char *current_dns; - char *current_dns_ex; - char **dns; - char **dns_ex; - char **fallback_dns; - char **fallback_dns_ex; - char **domains; - char **ntas; - const char *llmnr; - const char *mdns; - const char *dns_over_tls; - const char *dnssec; - const char *resolv_conf_mode; - bool dnssec_supported; -}; - -static void global_info_clear(struct global_info *p) { - free(p->current_dns); - free(p->current_dns_ex); - strv_free(p->dns); - strv_free(p->dns_ex); - strv_free(p->fallback_dns); - strv_free(p->fallback_dns_ex); - strv_free(p->domains); - strv_free(p->ntas); -} - static int status_global(sd_bus *bus, StatusMode mode, bool *empty_line) { static const struct bus_properties_map property_map[] = { - { "DNS", "a(iiay)", map_global_dns_servers, offsetof(struct global_info, dns) }, - { "DNSEx", "a(iiayqs)", map_global_dns_servers_ex, offsetof(struct global_info, dns_ex) }, - { "FallbackDNS", "a(iiay)", map_global_dns_servers, offsetof(struct global_info, fallback_dns) }, - { "FallbackDNSEx", "a(iiayqs)", map_global_dns_servers_ex, offsetof(struct global_info, fallback_dns_ex) }, - { "CurrentDNSServer", "(iiay)", map_global_current_dns_server, offsetof(struct global_info, current_dns) }, - { "CurrentDNSServerEx", "(iiayqs)", map_global_current_dns_server_ex, offsetof(struct global_info, current_dns_ex) }, - { "Domains", "a(isb)", map_global_domains, offsetof(struct global_info, domains) }, - { "DNSSECNegativeTrustAnchors", "as", NULL, offsetof(struct global_info, ntas) }, - { "LLMNR", "s", NULL, offsetof(struct global_info, llmnr) }, - { "MulticastDNS", "s", NULL, offsetof(struct global_info, mdns) }, - { "DNSOverTLS", "s", NULL, offsetof(struct global_info, dns_over_tls) }, - { "DNSSEC", "s", NULL, offsetof(struct global_info, dnssec) }, - { "DNSSECSupported", "b", NULL, offsetof(struct global_info, dnssec_supported) }, - { "ResolvConfMode", "s", NULL, offsetof(struct global_info, resolv_conf_mode) }, + { "DNS", "a(iiay)", map_global_dns_servers, offsetof(GlobalInfo, dns) }, + { "DNSEx", "a(iiayqs)", map_global_dns_servers_ex, offsetof(GlobalInfo, dns_ex) }, + { "FallbackDNS", "a(iiay)", map_global_dns_servers, offsetof(GlobalInfo, fallback_dns) }, + { "FallbackDNSEx", "a(iiayqs)", map_global_dns_servers_ex, offsetof(GlobalInfo, fallback_dns_ex) }, + { "CurrentDNSServer", "(iiay)", map_global_current_dns_server, offsetof(GlobalInfo, current_dns) }, + { "CurrentDNSServerEx", "(iiayqs)", map_global_current_dns_server_ex, offsetof(GlobalInfo, current_dns_ex) }, + { "Domains", "a(isb)", map_global_domains, offsetof(GlobalInfo, domains) }, + { "DNSSECNegativeTrustAnchors", "as", NULL, offsetof(GlobalInfo, ntas) }, + { "LLMNR", "s", NULL, offsetof(GlobalInfo, llmnr) }, + { "MulticastDNS", "s", NULL, offsetof(GlobalInfo, mdns) }, + { "DNSOverTLS", "s", NULL, offsetof(GlobalInfo, dns_over_tls) }, + { "DNSSEC", "s", NULL, offsetof(GlobalInfo, dnssec) }, + { "DNSSECSupported", "b", NULL, offsetof(GlobalInfo, dnssec_supported) }, + { "ResolvConfMode", "s", NULL, offsetof(GlobalInfo, resolv_conf_mode) }, {} }; _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; _cleanup_(sd_bus_message_unrefp) sd_bus_message *m = NULL; - _cleanup_(global_info_clear) struct global_info global_info = {}; + _cleanup_(global_info_clear) GlobalInfo global_info = {}; _cleanup_(table_unrefp) Table *table = NULL; int r; From af781878d5986127ca00831c4b524c2b62649823 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sun, 11 Oct 2020 12:19:46 +0200 Subject: [PATCH 4/9] resolvectl: sort domain/nta output dns list shall not be sorted. --- src/resolve/resolvectl.c | 8 ++++++-- src/shared/bus-map-properties.c | 17 +++++++++++++++++ src/shared/bus-map-properties.h | 1 + 3 files changed, 24 insertions(+), 2 deletions(-) diff --git a/src/resolve/resolvectl.c b/src/resolve/resolvectl.c index 523b0be1c3..48532d9bde 100644 --- a/src/resolve/resolvectl.c +++ b/src/resolve/resolvectl.c @@ -1299,6 +1299,8 @@ static int map_link_domains(sd_bus *bus, const char *member, sd_bus_message *m, if (r < 0) return r; + strv_sort(*l); + return 0; } @@ -1417,7 +1419,7 @@ static int status_ifindex(sd_bus *bus, int ifindex, const char *name, StatusMode { "MulticastDNS", "s", NULL, offsetof(LinkInfo, mdns) }, { "DNSOverTLS", "s", NULL, offsetof(LinkInfo, dns_over_tls) }, { "DNSSEC", "s", NULL, offsetof(LinkInfo, dnssec) }, - { "DNSSECNegativeTrustAnchors", "as", NULL, offsetof(LinkInfo, ntas) }, + { "DNSSECNegativeTrustAnchors", "as", bus_map_strv_sort, offsetof(LinkInfo, ntas) }, { "DNSSECSupported", "b", NULL, offsetof(LinkInfo, dnssec_supported) }, {} }; @@ -1682,6 +1684,8 @@ static int map_global_domains(sd_bus *bus, const char *member, sd_bus_message *m if (r < 0) return r; + strv_sort(*l); + return 0; } @@ -1694,7 +1698,7 @@ static int status_global(sd_bus *bus, StatusMode mode, bool *empty_line) { { "CurrentDNSServer", "(iiay)", map_global_current_dns_server, offsetof(GlobalInfo, current_dns) }, { "CurrentDNSServerEx", "(iiayqs)", map_global_current_dns_server_ex, offsetof(GlobalInfo, current_dns_ex) }, { "Domains", "a(isb)", map_global_domains, offsetof(GlobalInfo, domains) }, - { "DNSSECNegativeTrustAnchors", "as", NULL, offsetof(GlobalInfo, ntas) }, + { "DNSSECNegativeTrustAnchors", "as", bus_map_strv_sort, offsetof(GlobalInfo, ntas) }, { "LLMNR", "s", NULL, offsetof(GlobalInfo, llmnr) }, { "MulticastDNS", "s", NULL, offsetof(GlobalInfo, mdns) }, { "DNSOverTLS", "s", NULL, offsetof(GlobalInfo, dns_over_tls) }, diff --git a/src/shared/bus-map-properties.c b/src/shared/bus-map-properties.c index ab393c3952..5cc8ce222d 100644 --- a/src/shared/bus-map-properties.c +++ b/src/shared/bus-map-properties.c @@ -25,6 +25,23 @@ int bus_map_id128(sd_bus *bus, const char *member, sd_bus_message *m, sd_bus_err return 0; } +int bus_map_strv_sort(sd_bus *bus, const char *member, sd_bus_message *m, sd_bus_error *error, void *userdata) { + _cleanup_strv_free_ char **l = NULL; + char ***p = userdata; + int r; + + r = bus_message_read_strv_extend(m, &l); + if (r < 0) + return r; + + r = strv_extend_strv(p, l, false); + if (r < 0) + return r; + + strv_sort(*p); + return 0; +} + static int map_basic(sd_bus *bus, const char *member, sd_bus_message *m, unsigned flags, sd_bus_error *error, void *userdata) { char type; int r; diff --git a/src/shared/bus-map-properties.h b/src/shared/bus-map-properties.h index 11b899227b..94242ecc3f 100644 --- a/src/shared/bus-map-properties.h +++ b/src/shared/bus-map-properties.h @@ -18,6 +18,7 @@ enum { }; int bus_map_id128(sd_bus *bus, const char *member, sd_bus_message *m, sd_bus_error *error, void *userdata); +int bus_map_strv_sort(sd_bus *bus, const char *member, sd_bus_message *m, sd_bus_error *error, void *userdata); int bus_message_map_all_properties(sd_bus_message *m, const struct bus_properties_map *map, unsigned flags, sd_bus_error *error, void *userdata); int bus_map_all_properties(sd_bus *bus, const char *destination, const char *path, const struct bus_properties_map *map, From fe37e5a5d192ec55f87cd57893688a865b7f72d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sun, 11 Oct 2020 16:20:27 +0200 Subject: [PATCH 5/9] resolvectl: use compat status string instead of a field-by-field table The status string is modeled after our --version output: +enabled -disabled equals=more-info For example: Protocols: -DefaultRoute +LLMNR -mDNS -DNSOverTLS DNSSEC=allow-downgrade/supported --- src/basic/string-util.h | 4 ++ src/resolve/resolvectl.c | 89 ++++++++++++++++++++++++++++++---------- 2 files changed, 71 insertions(+), 22 deletions(-) diff --git a/src/basic/string-util.h b/src/basic/string-util.h index cefbda3577..18ba2a1dcb 100644 --- a/src/basic/string-util.h +++ b/src/basic/string-util.h @@ -53,6 +53,10 @@ static inline const char* true_false(bool b) { return b ? "true" : "false"; } +static inline const char* plus_minus(bool b) { + return b ? "+" : "-"; +} + static inline const char* one_zero(bool b) { return b ? "1" : "0"; } diff --git a/src/resolve/resolvectl.c b/src/resolve/resolvectl.c index 48532d9bde..6700ea2011 100644 --- a/src/resolve/resolvectl.c +++ b/src/resolve/resolvectl.c @@ -1406,6 +1406,61 @@ static int dump_list(Table *table, const char *prefix, char * const *l) { return 0; } +static int strv_extend_extended_bool(char ***strv, const char *name, const char *value) { + int r; + + if (value) { + r = parse_boolean(value); + if (r >= 0) + return strv_extendf(strv, "%s%s", plus_minus(r), name); + } + + return strv_extendf(strv, "%s=%s", name, value ?: "???"); +} + +static char* link_protocol_status(const LinkInfo *info) { + _cleanup_strv_free_ char **s = NULL; + + if (strv_extendf(&s, "%sDefaultRoute", plus_minus(info->default_route)) < 0) + return NULL; + + if (strv_extend_extended_bool(&s, "LLMNR", info->llmnr) < 0) + return NULL; + + if (strv_extend_extended_bool(&s, "mDNS", info->mdns) < 0) + return NULL; + + if (strv_extend_extended_bool(&s, "DNSOverTLS", info->dns_over_tls) < 0) + return NULL; + + if (strv_extendf(&s, "DNSSEC=%s/%s", + info->dnssec ?: "???", + info->dnssec_supported ? "supported" : "unsupported") < 0) + return NULL; + + return strv_join(s, " "); +} + +static char* global_protocol_status(const GlobalInfo *info) { + _cleanup_strv_free_ char **s = NULL; + + if (strv_extend_extended_bool(&s, "LLMNR", info->llmnr) < 0) + return NULL; + + if (strv_extend_extended_bool(&s, "mDNS", info->mdns) < 0) + return NULL; + + if (strv_extend_extended_bool(&s, "DNSOverTLS", info->dns_over_tls) < 0) + return NULL; + + if (strv_extendf(&s, "DNSSEC=%s/%s", + info->dnssec ?: "???", + info->dnssec_supported ? "supported" : "unsupported") < 0) + return NULL; + + return strv_join(s, " "); +} + static int status_ifindex(sd_bus *bus, int ifindex, const char *name, StatusMode mode, bool *empty_line) { static const struct bus_properties_map property_map[] = { { "ScopesMask", "t", NULL, offsetof(LinkInfo, scopes_mask) }, @@ -1549,19 +1604,13 @@ static int status_ifindex(sd_bus *bus, int ifindex, const char *name, StatusMode if (r < 0) return table_log_add_error(r); + _cleanup_free_ char *pstatus = link_protocol_status(&link_info); + if (!pstatus) + return log_oom(); + r = table_add_many(table, - TABLE_STRING, "DefaultRoute setting:", - TABLE_BOOLEAN, link_info.default_route, - TABLE_STRING, "LLMNR setting:", - TABLE_STRING, strna(link_info.llmnr), - TABLE_STRING, "MulticastDNS setting:", - TABLE_STRING, strna(link_info.mdns), - TABLE_STRING, "DNSOverTLS setting:", - TABLE_STRING, strna(link_info.dns_over_tls), - TABLE_STRING, "DNSSEC setting:", - TABLE_STRING, strna(link_info.dnssec), - TABLE_STRING, "DNSSEC supported:", - TABLE_BOOLEAN, link_info.dnssec_supported); + TABLE_STRING, "Protocols:", + TABLE_STRING, pstatus); if (r < 0) return table_log_add_error(r); @@ -1774,18 +1823,14 @@ static int status_global(sd_bus *bus, StatusMode mode, bool *empty_line) { table_set_header(table, false); + _cleanup_free_ char *pstatus = global_protocol_status(&global_info); + if (!pstatus) + return log_oom(); + r = table_add_many(table, - TABLE_STRING, "LLMNR setting:", + TABLE_STRING, "Protocols:", TABLE_SET_ALIGN_PERCENT, 100, - TABLE_STRING, strna(global_info.llmnr), - TABLE_STRING, "MulticastDNS setting:", - TABLE_STRING, strna(global_info.mdns), - TABLE_STRING, "DNSOverTLS setting:", - TABLE_STRING, strna(global_info.dns_over_tls), - TABLE_STRING, "DNSSEC setting:", - TABLE_STRING, strna(global_info.dnssec), - TABLE_STRING, "DNSSEC supported:", - TABLE_BOOLEAN, global_info.dnssec_supported); + TABLE_STRING, pstatus); if (r < 0) return table_log_add_error(r); From 6f8ca84c9b64c81add286790a7ffcc2eed569b27 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sun, 11 Oct 2020 16:39:12 +0200 Subject: [PATCH 6/9] format-table: reduce scope of iterator variables --- src/shared/format-table.c | 79 +++++++++++++++------------------------ 1 file changed, 31 insertions(+), 48 deletions(-) diff --git a/src/shared/format-table.c b/src/shared/format-table.c index 7e876295ff..3d7d440e1a 100644 --- a/src/shared/format-table.c +++ b/src/shared/format-table.c @@ -164,7 +164,6 @@ Table *table_new_raw(size_t n_columns) { Table *table_new_internal(const char *first_header, ...) { _cleanup_(table_unrefp) Table *t = NULL; size_t n_columns = 1; - const char *h; va_list ap; int r; @@ -172,8 +171,7 @@ Table *table_new_internal(const char *first_header, ...) { va_start(ap, first_header); for (;;) { - h = va_arg(ap, const char*); - if (!h) + if (!va_arg(ap, const char*)) break; n_columns++; @@ -185,7 +183,7 @@ Table *table_new_internal(const char *first_header, ...) { return NULL; va_start(ap, first_header); - for (h = first_header; h; h = va_arg(ap, const char*)) { + for (const char *h = first_header; h; h = va_arg(ap, const char*)) { TableCell *cell; r = table_add_cell(t, &cell, TABLE_STRING, h); @@ -223,12 +221,10 @@ DEFINE_PRIVATE_TRIVIAL_REF_UNREF_FUNC(TableData, table_data, table_data_free); DEFINE_TRIVIAL_CLEANUP_FUNC(TableData*, table_data_unref); Table *table_unref(Table *t) { - size_t i; - if (!t) return NULL; - for (i = 0; i < t->n_cells; i++) + for (size_t i = 0; i < t->n_cells; i++) table_data_unref(t->data[i]); free(t->data); @@ -1047,11 +1043,9 @@ int table_set_empty_string(Table *t, const char *empty) { } int table_set_display_all(Table *t) { - size_t allocated; - assert(t); - allocated = t->n_display_map; + size_t allocated = t->n_display_map; if (!GREEDY_REALLOC(t->display_map, allocated, MAX(t->n_columns, allocated))) return -ENOMEM; @@ -1124,7 +1118,6 @@ int table_set_sort(Table *t, size_t first_column, ...) { } int table_hide_column_from_display(Table *t, size_t column) { - size_t allocated, cur = 0; int r; assert(t); @@ -1137,7 +1130,7 @@ int table_hide_column_from_display(Table *t, size_t column) { return r; } - allocated = t->n_display_map; + size_t allocated = t->n_display_map, cur = 0; for (size_t i = 0; i < allocated; i++) { if (t->display_map[i] == column) @@ -1247,7 +1240,6 @@ static int cell_data_compare(TableData *a, size_t index_a, TableData *b, size_t } static int table_data_compare(const size_t *a, const size_t *b, Table *t) { - size_t i; int r; assert(t); @@ -1262,7 +1254,7 @@ static int table_data_compare(const size_t *a, const size_t *b, Table *t) { return 1; /* Order other lines by the sorting map */ - for (i = 0; i < t->n_sort_map; i++) { + for (size_t i = 0; i < t->n_sort_map; i++) { TableData *d, *dd; d = t->data[*a + t->sort_map[i]]; @@ -1290,13 +1282,12 @@ static const char *table_data_format(Table *t, TableData *d, bool avoid_uppercas case TABLE_STRING: case TABLE_PATH: if (d->uppercase && !avoid_uppercasing) { - char *p, *q; - d->formatted = new(char, strlen(d->string) + 1); if (!d->formatted) return NULL; - for (p = d->string, q = d->formatted; *p; p++, q++) + char *q = d->formatted; + for (char *p = d->string; *p; p++, q++) *q = (char) toupper((unsigned char) *p); *q = 0; @@ -1305,19 +1296,14 @@ static const char *table_data_format(Table *t, TableData *d, bool avoid_uppercas return d->string; - case TABLE_STRV: { - char *p; - + case TABLE_STRV: if (strv_isempty(d->strv)) return strempty(t->empty_string); - p = strv_join(d->strv, "\n"); - if (!p) + d->formatted = strv_join(d->strv, "\n"); + if (!d->formatted) return NULL; - - d->formatted = p; break; - } case TABLE_BOOLEAN: return yes_no(d->boolean); @@ -1678,7 +1664,6 @@ static char *align_string_mem(const char *str, const char *url, size_t new_lengt _cleanup_free_ char *clickable = NULL; const char *p; char *ret; - size_t i; int r; /* As with ellipsize_mem(), 'old_length' is a byte size while 'new_length' is a width in character cells */ @@ -1723,10 +1708,10 @@ static char *align_string_mem(const char *str, const char *url, size_t new_lengt if (!ret) return NULL; - for (i = 0; i < lspace; i++) + for (size_t i = 0; i < lspace; i++) ret[i] = ' '; memcpy(ret + lspace, clickable ?: str, clickable_length); - for (i = lspace + clickable_length; i < space + clickable_length; i++) + for (size_t i = lspace + clickable_length; i < space + clickable_length; i++) ret[i] = ' '; ret[space + clickable_length] = 0; @@ -1771,7 +1756,7 @@ static const char* table_data_rgap_color(TableData *d) { int table_print(Table *t, FILE *f) { size_t n_rows, *minimum_width, *maximum_width, display_columns, *requested_width, - i, j, table_minimum_width, table_maximum_width, table_requested_width, table_effective_width, + table_minimum_width, table_maximum_width, table_requested_width, table_effective_width, *width; _cleanup_free_ size_t *sorted = NULL; uint64_t *column_weight, weight_sum; @@ -1795,7 +1780,7 @@ int table_print(Table *t, FILE *f) { if (!sorted) return -ENOMEM; - for (i = 0; i < n_rows; i++) + for (size_t i = 0; i < n_rows; i++) sorted[i] = i * t->n_columns; typesafe_qsort_r(sorted, n_rows, table_data_compare, t); @@ -1814,21 +1799,21 @@ int table_print(Table *t, FILE *f) { width = newa(size_t, display_columns); column_weight = newa0(uint64_t, display_columns); - for (j = 0; j < display_columns; j++) { + for (size_t j = 0; j < display_columns; j++) { minimum_width[j] = 1; maximum_width[j] = (size_t) -1; requested_width[j] = (size_t) -1; } /* First pass: determine column sizes */ - for (i = t->header ? 0 : 1; i < n_rows; i++) { + for (size_t i = t->header ? 0 : 1; i < n_rows; i++) { TableData **row; /* Note that we don't care about ordering at this time, as we just want to determine column sizes, * hence we don't care for sorted[] during the first pass. */ row = t->data + i * t->n_columns; - for (j = 0; j < display_columns; j++) { + for (size_t j = 0; j < display_columns; j++) { TableData *d; size_t req_width, req_height; @@ -1885,7 +1870,7 @@ int table_print(Table *t, FILE *f) { /* Calculate the total weight for all columns, plus the minimum, maximum and requested width for the table. */ weight_sum = 0; - for (j = 0; j < display_columns; j++) { + for (size_t j = 0; j < display_columns; j++) { weight_sum += column_weight[j]; table_minimum_width += minimum_width[j]; @@ -1920,7 +1905,7 @@ int table_print(Table *t, FILE *f) { extra = table_effective_width - table_requested_width; - for (j = 0; j < display_columns; j++) { + for (size_t j = 0; j < display_columns; j++) { size_t delta; if (weight_sum == 0) @@ -1955,13 +1940,13 @@ int table_print(Table *t, FILE *f) { extra = table_effective_width - table_minimum_width; - for (j = 0; j < display_columns; j++) + for (size_t j = 0; j < display_columns; j++) width[j] = (size_t) -1; for (;;) { bool restart = false; - for (j = 0; j < display_columns; j++) { + for (size_t j = 0; j < display_columns; j++) { size_t delta, w; /* Did this column already get something assigned? If so, let's skip to the next */ @@ -2009,7 +1994,7 @@ int table_print(Table *t, FILE *f) { } /* Second pass: show output */ - for (i = t->header ? 0 : 1; i < n_rows; i++) { + for (size_t i = t->header ? 0 : 1; i < n_rows; i++) { size_t n_subline = 0; bool more_sublines; TableData **row; @@ -2023,7 +2008,7 @@ int table_print(Table *t, FILE *f) { const char *gap_color = NULL; more_sublines = false; - for (j = 0; j < display_columns; j++) { + for (size_t j = 0; j < display_columns; j++) { _cleanup_free_ char *buffer = NULL, *extracted = NULL; bool lines_truncated = false; const char *field, *color = NULL; @@ -2332,17 +2317,15 @@ static int table_data_to_json(TableData *d, JsonVariant **ret) { } static char* string_to_json_field_name(const char *f) { - char *c, *x; - /* Tries to make a string more suitable as JSON field name. There are no strict rules defined what a * field name can be hence this is a bit vague and black magic. Right now we only convert spaces to * underscores and leave everything as is. */ - c = strdup(f); + char *c = strdup(f); if (!c) return NULL; - for (x = c; *x; x++) + for (char *x = c; *x; x++) if (isspace(*x)) *x = '_'; @@ -2352,7 +2335,7 @@ static char* string_to_json_field_name(const char *f) { int table_to_json(Table *t, JsonVariant **ret) { JsonVariant **rows = NULL, **elements = NULL; _cleanup_free_ size_t *sorted = NULL; - size_t n_rows, i, j, display_columns; + size_t n_rows, display_columns; int r; assert(t); @@ -2372,7 +2355,7 @@ int table_to_json(Table *t, JsonVariant **ret) { goto finish; } - for (i = 0; i < n_rows; i++) + for (size_t i = 0; i < n_rows; i++) sorted[i] = i * t->n_columns; typesafe_qsort_r(sorted, n_rows, table_data_compare, t); @@ -2390,7 +2373,7 @@ int table_to_json(Table *t, JsonVariant **ret) { goto finish; } - for (j = 0; j < display_columns; j++) { + for (size_t j = 0; j < display_columns; j++) { _cleanup_free_ char *mangled = NULL; const char *formatted; TableData *d; @@ -2422,7 +2405,7 @@ int table_to_json(Table *t, JsonVariant **ret) { goto finish; } - for (i = 1; i < n_rows; i++) { + for (size_t i = 1; i < n_rows; i++) { TableData **row; if (sorted) @@ -2430,7 +2413,7 @@ int table_to_json(Table *t, JsonVariant **ret) { else row = t->data + i * t->n_columns; - for (j = 0; j < display_columns; j++) { + for (size_t j = 0; j < display_columns; j++) { TableData *d; size_t k; From b0e3d799891c4633bd2b0d88e4ed2c741bbcd532 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 12 Oct 2020 13:29:46 +0200 Subject: [PATCH 7/9] format-table: add TABLE_STRV_WRAPPED The idea is that we have strvs like list of server names or addresses, where the majority of strings is rather short, but some are long and there can potentially be many strings. So formattting them either all on one line or all in separate lines leads to output that is either hard to read or uses way too many rows. We want to wrap them, but relying on the pager to do the wrapping is not nice. Normal text has a lot of redundancy, so when the pager wraps a line in the middle of a word the read can understand what is going on without any trouble. But for a high-density zero-redundancy text like an IP address it is much nicer to wrap between words. This also makes c&p easier. This adds a variant of TABLE_STRV which is wrapped on output (with line breaks inserted between different strv entries). The change table_print() is quite ugly. A second pass is added to re-calculate column widths. Since column size is now "soft", i.e. it can adjust based on available columns, we need to two passes: - first we figure out how much space we want - in the second pass we figure out what the actual wrapped columns widths will be. To avoid unnessary work, the second pass is only done when we actually have wrappable fields. A test is added in test-format-table. --- src/basic/macro.h | 4 +- src/resolve/resolvectl.c | 6 +- src/shared/format-table.c | 438 +++++++++++++++++++++-------------- src/shared/format-table.h | 1 + src/test/test-format-table.c | 112 ++++++++- 5 files changed, 378 insertions(+), 183 deletions(-) diff --git a/src/basic/macro.h b/src/basic/macro.h index 954bb2de2a..f92e89a3a8 100644 --- a/src/basic/macro.h +++ b/src/basic/macro.h @@ -634,6 +634,8 @@ static inline int __coverity_check_and_return__(int condition) { _copy; \ }) -#define SIZE_ADD(x, y) ((x) >= SIZE_MAX - (y) ? SIZE_MAX : (x) + (y)) +static inline size_t size_add(size_t x, size_t y) { + return y >= SIZE_MAX - x ? SIZE_MAX : x + y; +} #include "log.h" diff --git a/src/resolve/resolvectl.c b/src/resolve/resolvectl.c index 6700ea2011..6f3de91b13 100644 --- a/src/resolve/resolvectl.c +++ b/src/resolve/resolvectl.c @@ -1320,12 +1320,12 @@ static int status_print_strv_ifindex(int ifindex, const char *ifname, char **p) size_t our_len = utf8_console_width(*i); /* This returns -1 on invalid utf-8 (which shouldn't happen). * If that happens, we'll just print one item per line. */ - if (position <= indent || SIZE_ADD(SIZE_ADD(position, 1), our_len) < cols) { + if (position <= indent || size_add(size_add(position, 1), our_len) < cols) { printf(" %s", *i); - position = SIZE_ADD(SIZE_ADD(position, 1), our_len); + position = size_add(size_add(position, 1), our_len); } else { printf("\n%*s%s", indent, "", *i); - position = SIZE_ADD(our_len, indent); + position = size_add(our_len, indent); } } diff --git a/src/shared/format-table.c b/src/shared/format-table.c index 3d7d440e1a..1063baba52 100644 --- a/src/shared/format-table.c +++ b/src/shared/format-table.c @@ -66,6 +66,7 @@ typedef struct TableData { size_t minimum_width; /* minimum width for the column */ size_t maximum_width; /* maximum width for the column */ + size_t formatted_for_width; /* the width we tried to format for */ unsigned weight; /* the horizontal weight for this column, in case the table is expanded/compressed */ unsigned ellipsize_percent; /* 0 … 100, where to place the ellipsis when compression is needed */ unsigned align_percent; /* 0 … 100, where to pad with spaces when expanding is needed. 0: left-aligned, 100: right-aligned */ @@ -211,7 +212,7 @@ static TableData *table_data_free(TableData *d) { free(d->formatted); free(d->url); - if (d->type == TABLE_STRV) + if (IN_SET(d->type, TABLE_STRV, TABLE_STRV_WRAPPED)) strv_free(d->strv); return mfree(d); @@ -248,6 +249,7 @@ static size_t table_data_size(TableDataType type, const void *data) { return strlen(data) + 1; case TABLE_STRV: + case TABLE_STRV_WRAPPED: return sizeof(char **); case TABLE_BOOLEAN: @@ -372,7 +374,7 @@ static TableData *table_data_new( d->align_percent = align_percent; d->ellipsize_percent = ellipsize_percent; - if (type == TABLE_STRV) { + if (IN_SET(type, TABLE_STRV, TABLE_STRV_WRAPPED)) { d->strv = strv_copy(data); if (!d->strv) return NULL; @@ -813,6 +815,7 @@ int table_add_many_internal(Table *t, TableDataType first_type, ...) { break; case TABLE_STRV: + case TABLE_STRV_WRAPPED: data = va_arg(ap, char * const *); break; @@ -1162,6 +1165,7 @@ static int cell_data_compare(TableData *a, size_t index_a, TableData *b, size_t return path_compare(a->string, b->string); case TABLE_STRV: + case TABLE_STRV_WRAPPED: return strv_compare(a->strv, b->strv); case TABLE_BOOLEAN: @@ -1269,10 +1273,46 @@ static int table_data_compare(const size_t *a, const size_t *b, Table *t) { return CMP(*a, *b); } -static const char *table_data_format(Table *t, TableData *d, bool avoid_uppercasing) { +static char* format_strv_width(char **strv, size_t column_width) { + _cleanup_fclose_ FILE *f = NULL; + size_t sz = 0; + _cleanup_free_ char *buf = NULL; + + f = open_memstream_unlocked(&buf, &sz); + if (!f) + return NULL; + + size_t position = 0; + char **p; + STRV_FOREACH(p, strv) { + size_t our_len = utf8_console_width(*p); /* This returns -1 on invalid utf-8 (which shouldn't happen). + * If that happens, we'll just print one item per line. */ + + if (position == 0) { + fputs(*p, f); + position = our_len; + } else if (size_add(size_add(position, 1), our_len) <= column_width) { + fprintf(f, " %s", *p); + position = size_add(size_add(position, 1), our_len); + } else { + fprintf(f, "\n%s", *p); + position = our_len; + } + } + + if (fflush_and_check(f) < 0) + return NULL; + + f = safe_fclose(f); + return TAKE_PTR(buf); +} + +static const char *table_data_format(Table *t, TableData *d, bool avoid_uppercasing, size_t column_width, bool *have_soft) { assert(d); - if (d->formatted) + if (d->formatted && + /* Only TABLE_STRV_WRAPPED adjust based on column_width so far… */ + (d->type != TABLE_STRV_WRAPPED || d->formatted_for_width == column_width)) return d->formatted; switch (d->type) { @@ -1305,6 +1345,22 @@ static const char *table_data_format(Table *t, TableData *d, bool avoid_uppercas return NULL; break; + case TABLE_STRV_WRAPPED: { + if (strv_isempty(d->strv)) + return strempty(t->empty_string); + + char *buf = format_strv_width(d->strv, column_width); + if (!buf) + return NULL; + + free_and_replace(d->formatted, buf); + d->formatted_for_width = column_width; + if (have_soft) + *have_soft = true; + + break; + } + case TABLE_BOOLEAN: return yes_no(d->boolean); @@ -1618,16 +1674,19 @@ static int console_width_height( static int table_data_requested_width_height( Table *table, TableData *d, + size_t available_width, size_t *ret_width, - size_t *ret_height) { + size_t *ret_height, + bool *have_soft) { _cleanup_free_ char *truncated = NULL; bool truncation_applied = false; size_t width, height; const char *t; int r; + bool soft = false; - t = table_data_format(table, d, false); + t = table_data_format(table, d, false, available_width, &soft); if (!t) return -ENOMEM; @@ -1655,6 +1714,8 @@ static int table_data_requested_width_height( *ret_width = width; if (ret_height) *ret_height = height; + if (have_soft && soft) + *have_soft = true; return truncation_applied; } @@ -1725,7 +1786,7 @@ static bool table_data_isempty(TableData *d) { return true; /* Let's also consider an empty strv as truly empty. */ - if (d->type == TABLE_STRV) + if (IN_SET(d->type, TABLE_STRV, TABLE_STRV_WRAPPED)) return strv_isempty(d->strv); /* Note that an empty string we do not consider empty here! */ @@ -1757,7 +1818,7 @@ static const char* table_data_rgap_color(TableData *d) { int table_print(Table *t, FILE *f) { size_t n_rows, *minimum_width, *maximum_width, display_columns, *requested_width, table_minimum_width, table_maximum_width, table_requested_width, table_effective_width, - *width; + *width = NULL; _cleanup_free_ size_t *sorted = NULL; uint64_t *column_weight, weight_sum; int r; @@ -1796,200 +1857,220 @@ int table_print(Table *t, FILE *f) { minimum_width = newa(size_t, display_columns); maximum_width = newa(size_t, display_columns); requested_width = newa(size_t, display_columns); - width = newa(size_t, display_columns); column_weight = newa0(uint64_t, display_columns); for (size_t j = 0; j < display_columns; j++) { minimum_width[j] = 1; maximum_width[j] = (size_t) -1; - requested_width[j] = (size_t) -1; } - /* First pass: determine column sizes */ - for (size_t i = t->header ? 0 : 1; i < n_rows; i++) { - TableData **row; - - /* Note that we don't care about ordering at this time, as we just want to determine column sizes, - * hence we don't care for sorted[] during the first pass. */ - row = t->data + i * t->n_columns; - - for (size_t j = 0; j < display_columns; j++) { - TableData *d; - size_t req_width, req_height; - - assert_se(d = row[t->display_map ? t->display_map[j] : j]); - - r = table_data_requested_width_height(t, d, &req_width, &req_height); - if (r < 0) - return r; - if (r > 0) { /* Truncated because too many lines? */ - _cleanup_free_ char *last = NULL; - const char *field; - - /* If we are going to show only the first few lines of a cell that has - * multiple make sure that we have enough space horizontally to show an - * ellipsis. Hence, let's figure out the last line, and account for its - * length plus ellipsis. */ - - field = table_data_format(t, d, false); - if (!field) - return -ENOMEM; - - assert_se(t->cell_height_max > 0); - r = string_extract_line(field, t->cell_height_max-1, &last); - if (r < 0) - return r; - - req_width = MAX(req_width, - utf8_console_width(last) + - utf8_console_width(special_glyph(SPECIAL_GLYPH_ELLIPSIS))); - } - - /* Determine the biggest width that any cell in this column would like to have */ - if (requested_width[j] == (size_t) -1 || - requested_width[j] < req_width) - requested_width[j] = req_width; - - /* Determine the minimum width any cell in this column needs */ - if (minimum_width[j] < d->minimum_width) - minimum_width[j] = d->minimum_width; - - /* Determine the maximum width any cell in this column needs */ - if (d->maximum_width != (size_t) -1 && - (maximum_width[j] == (size_t) -1 || - maximum_width[j] > d->maximum_width)) - maximum_width[j] = d->maximum_width; - - /* Determine the full columns weight */ - column_weight[j] += d->weight; - } - } - - /* One space between each column */ - table_requested_width = table_minimum_width = table_maximum_width = display_columns - 1; - - /* Calculate the total weight for all columns, plus the minimum, maximum and requested width for the table. */ - weight_sum = 0; - for (size_t j = 0; j < display_columns; j++) { - weight_sum += column_weight[j]; - - table_minimum_width += minimum_width[j]; - - if (maximum_width[j] == (size_t) -1) - table_maximum_width = (size_t) -1; - else - table_maximum_width += maximum_width[j]; - - table_requested_width += requested_width[j]; - } - - /* Calculate effective table width */ - if (t->width != 0 && t->width != (size_t) -1) - table_effective_width = t->width; - else if (t->width == 0 || pager_have() || !isatty(STDOUT_FILENO)) - table_effective_width = table_requested_width; - else - table_effective_width = MIN(table_requested_width, columns()); - - if (table_maximum_width != (size_t) -1 && table_effective_width > table_maximum_width) - table_effective_width = table_maximum_width; - - if (table_effective_width < table_minimum_width) - table_effective_width = table_minimum_width; - - if (table_effective_width >= table_requested_width) { - size_t extra; - - /* We have extra room, let's distribute it among columns according to their weights. We first provide - * each column with what it asked for and the distribute the rest. */ - - extra = table_effective_width - table_requested_width; - - for (size_t j = 0; j < display_columns; j++) { - size_t delta; - - if (weight_sum == 0) - width[j] = requested_width[j] + extra / (display_columns - j); /* Avoid division by zero */ - else - width[j] = requested_width[j] + (extra * column_weight[j]) / weight_sum; - - if (maximum_width[j] != (size_t) -1 && width[j] > maximum_width[j]) - width[j] = maximum_width[j]; - - if (width[j] < minimum_width[j]) - width[j] = minimum_width[j]; - - assert(width[j] >= requested_width[j]); - delta = width[j] - requested_width[j]; - - /* Subtract what we just added from the rest */ - if (extra > delta) - extra -= delta; - else - extra = 0; - - assert(weight_sum >= column_weight[j]); - weight_sum -= column_weight[j]; - } - - } else { - /* We need to compress the table, columns can't get what they asked for. We first provide each column - * with the minimum they need, and then distribute anything left. */ - bool finalize = false; - size_t extra; - - extra = table_effective_width - table_minimum_width; + for (unsigned pass = 0; pass < 2; pass++) { + /* First pass: determine column sizes */ for (size_t j = 0; j < display_columns; j++) - width[j] = (size_t) -1; + requested_width[j] = (size_t) -1; - for (;;) { - bool restart = false; + bool any_soft = false; + + for (size_t i = t->header ? 0 : 1; i < n_rows; i++) { + TableData **row; + + /* Note that we don't care about ordering at this time, as we just want to determine column sizes, + * hence we don't care for sorted[] during the first pass. */ + row = t->data + i * t->n_columns; for (size_t j = 0; j < display_columns; j++) { - size_t delta, w; + TableData *d; + size_t req_width, req_height; - /* Did this column already get something assigned? If so, let's skip to the next */ - if (width[j] != (size_t) -1) - continue; + assert_se(d = row[t->display_map ? t->display_map[j] : j]); + + r = table_data_requested_width_height(t, d, + width ? width[j] : SIZE_MAX, + &req_width, &req_height, &any_soft); + if (r < 0) + return r; + if (r > 0) { /* Truncated because too many lines? */ + _cleanup_free_ char *last = NULL; + const char *field; + + /* If we are going to show only the first few lines of a cell that has + * multiple make sure that we have enough space horizontally to show an + * ellipsis. Hence, let's figure out the last line, and account for its + * length plus ellipsis. */ + + field = table_data_format(t, d, false, + width ? width[j] : SIZE_MAX, + &any_soft); + if (!field) + return -ENOMEM; + + assert_se(t->cell_height_max > 0); + r = string_extract_line(field, t->cell_height_max-1, &last); + if (r < 0) + return r; + + req_width = MAX(req_width, + utf8_console_width(last) + + utf8_console_width(special_glyph(SPECIAL_GLYPH_ELLIPSIS))); + } + + /* Determine the biggest width that any cell in this column would like to have */ + if (requested_width[j] == (size_t) -1 || + requested_width[j] < req_width) + requested_width[j] = req_width; + + /* Determine the minimum width any cell in this column needs */ + if (minimum_width[j] < d->minimum_width) + minimum_width[j] = d->minimum_width; + + /* Determine the maximum width any cell in this column needs */ + if (d->maximum_width != (size_t) -1 && + (maximum_width[j] == (size_t) -1 || + maximum_width[j] > d->maximum_width)) + maximum_width[j] = d->maximum_width; + + /* Determine the full columns weight */ + column_weight[j] += d->weight; + } + } + + /* One space between each column */ + table_requested_width = table_minimum_width = table_maximum_width = display_columns - 1; + + /* Calculate the total weight for all columns, plus the minimum, maximum and requested width for the table. */ + weight_sum = 0; + for (size_t j = 0; j < display_columns; j++) { + weight_sum += column_weight[j]; + + table_minimum_width += minimum_width[j]; + + if (maximum_width[j] == (size_t) -1) + table_maximum_width = (size_t) -1; + else + table_maximum_width += maximum_width[j]; + + table_requested_width += requested_width[j]; + } + + /* Calculate effective table width */ + if (t->width != 0 && t->width != (size_t) -1) + table_effective_width = t->width; + else if (t->width == 0 || + ((pass > 0 || !any_soft) && (pager_have() || !isatty(STDOUT_FILENO)))) + table_effective_width = table_requested_width; + else + table_effective_width = MIN(table_requested_width, columns()); + + if (table_maximum_width != (size_t) -1 && table_effective_width > table_maximum_width) + table_effective_width = table_maximum_width; + + if (table_effective_width < table_minimum_width) + table_effective_width = table_minimum_width; + + if (!width) + width = newa(size_t, display_columns); + + if (table_effective_width >= table_requested_width) { + size_t extra; + + /* We have extra room, let's distribute it among columns according to their weights. We first provide + * each column with what it asked for and the distribute the rest. */ + + extra = table_effective_width - table_requested_width; + + for (size_t j = 0; j < display_columns; j++) { + size_t delta; if (weight_sum == 0) - w = minimum_width[j] + extra / (display_columns - j); /* avoid division by zero */ + width[j] = requested_width[j] + extra / (display_columns - j); /* Avoid division by zero */ else - w = minimum_width[j] + (extra * column_weight[j]) / weight_sum; + width[j] = requested_width[j] + (extra * column_weight[j]) / weight_sum; - if (w >= requested_width[j]) { - /* Never give more than requested. If we hit a column like this, there's more - * space to allocate to other columns which means we need to restart the - * iteration. However, if we hit a column like this, let's assign it the space - * it wanted for good early.*/ + if (maximum_width[j] != (size_t) -1 && width[j] > maximum_width[j]) + width[j] = maximum_width[j]; - w = requested_width[j]; - restart = true; + if (width[j] < minimum_width[j]) + width[j] = minimum_width[j]; - } else if (!finalize) - continue; + assert(width[j] >= requested_width[j]); + delta = width[j] - requested_width[j]; - width[j] = w; - - assert(w >= minimum_width[j]); - delta = w - minimum_width[j]; - - assert(delta <= extra); - extra -= delta; + /* Subtract what we just added from the rest */ + if (extra > delta) + extra -= delta; + else + extra = 0; assert(weight_sum >= column_weight[j]); weight_sum -= column_weight[j]; - - if (restart && !finalize) - break; } - if (finalize) - break; + break; /* Every column should be happy, no need to repeat calculations. */ + } else { + /* We need to compress the table, columns can't get what they asked for. We first provide each column + * with the minimum they need, and then distribute anything left. */ + bool finalize = false; + size_t extra; - if (!restart) - finalize = true; + extra = table_effective_width - table_minimum_width; + + for (size_t j = 0; j < display_columns; j++) + width[j] = (size_t) -1; + + for (;;) { + bool restart = false; + + for (size_t j = 0; j < display_columns; j++) { + size_t delta, w; + + /* Did this column already get something assigned? If so, let's skip to the next */ + if (width[j] != (size_t) -1) + continue; + + if (weight_sum == 0) + w = minimum_width[j] + extra / (display_columns - j); /* avoid division by zero */ + else + w = minimum_width[j] + (extra * column_weight[j]) / weight_sum; + + if (w >= requested_width[j]) { + /* Never give more than requested. If we hit a column like this, there's more + * space to allocate to other columns which means we need to restart the + * iteration. However, if we hit a column like this, let's assign it the space + * it wanted for good early.*/ + + w = requested_width[j]; + restart = true; + + } else if (!finalize) + continue; + + width[j] = w; + + assert(w >= minimum_width[j]); + delta = w - minimum_width[j]; + + assert(delta <= extra); + extra -= delta; + + assert(weight_sum >= column_weight[j]); + weight_sum -= column_weight[j]; + + if (restart && !finalize) + break; + } + + if (finalize) + break; + + if (!restart) + finalize = true; + } + + if (!any_soft) /* Some columns got less than requested. If some cells were "soft", + * let's try to reformat them with the new widths. Otherwise, let's + * move on. */ + break; } } @@ -2017,7 +2098,7 @@ int table_print(Table *t, FILE *f) { assert_se(d = row[t->display_map ? t->display_map[j] : j]); - field = table_data_format(t, d, false); + field = table_data_format(t, d, false, width[j], NULL); if (!field) return -ENOMEM; @@ -2232,6 +2313,7 @@ static int table_data_to_json(TableData *d, JsonVariant **ret) { return json_variant_new_string(ret, d->string); case TABLE_STRV: + case TABLE_STRV_WRAPPED: return json_variant_new_array_strv(ret, d->strv); case TABLE_BOOLEAN: @@ -2381,7 +2463,7 @@ int table_to_json(Table *t, JsonVariant **ret) { assert_se(d = t->data[t->display_map ? t->display_map[j] : j]); /* Field names must be strings, hence format whatever we got here as a string first */ - formatted = table_data_format(t, d, true); + formatted = table_data_format(t, d, true, SIZE_MAX, NULL); if (!formatted) { r = -ENOMEM; goto finish; diff --git a/src/shared/format-table.h b/src/shared/format-table.h index 1851f1d14a..0d7b7c48c5 100644 --- a/src/shared/format-table.h +++ b/src/shared/format-table.h @@ -12,6 +12,7 @@ typedef enum TableDataType { TABLE_EMPTY, TABLE_STRING, TABLE_STRV, + TABLE_STRV_WRAPPED, TABLE_PATH, TABLE_BOOLEAN, TABLE_TIMESTAMP, diff --git a/src/test/test-format-table.c b/src/test/test-format-table.c index 283cf157ba..cf2e34dc9d 100644 --- a/src/test/test-format-table.c +++ b/src/test/test-format-table.c @@ -12,6 +12,8 @@ static void test_issue_9549(void) { _cleanup_(table_unrefp) Table *table = NULL; _cleanup_free_ char *formatted = NULL; + log_info("/* %s */", __func__); + assert_se(table = table_new("name", "type", "ro", "usage", "created", "modified")); assert_se(table_set_align_percent(table, TABLE_HEADER_CELL(3), 100) >= 0); assert_se(table_add_many(table, @@ -36,6 +38,8 @@ static void test_multiline(void) { _cleanup_(table_unrefp) Table *table = NULL; _cleanup_free_ char *formatted = NULL; + log_info("/* %s */", __func__); + assert_se(table = table_new("foo", "bar")); assert_se(table_set_align_percent(table, TABLE_HEADER_CELL(1), 100) >= 0); @@ -148,6 +152,8 @@ static void test_strv(void) { _cleanup_(table_unrefp) Table *table = NULL; _cleanup_free_ char *formatted = NULL; + log_info("/* %s */", __func__); + assert_se(table = table_new("foo", "bar")); assert_se(table_set_align_percent(table, TABLE_HEADER_CELL(1), 100) >= 0); @@ -256,8 +262,111 @@ static void test_strv(void) { formatted = mfree(formatted); } -int main(int argc, char *argv[]) { +static void test_strv_wrapped(void) { + _cleanup_(table_unrefp) Table *table = NULL; + _cleanup_free_ char *formatted = NULL; + log_info("/* %s */", __func__); + + assert_se(table = table_new("foo", "bar")); + + assert_se(table_set_align_percent(table, TABLE_HEADER_CELL(1), 100) >= 0); + + assert_se(table_add_many(table, + TABLE_STRV_WRAPPED, STRV_MAKE("three", "different", "lines"), + TABLE_STRV_WRAPPED, STRV_MAKE("two", "lines")) >= 0); + + table_set_cell_height_max(table, 1); + assert_se(table_format(table, &formatted) >= 0); + fputs(formatted, stdout); + assert_se(streq(formatted, + "FOO BAR\n" + "three different lines two lines\n")); + formatted = mfree(formatted); + + table_set_cell_height_max(table, 2); + assert_se(table_format(table, &formatted) >= 0); + fputs(formatted, stdout); + assert_se(streq(formatted, + "FOO BAR\n" + "three different lines two lines\n")); + formatted = mfree(formatted); + + table_set_cell_height_max(table, 3); + assert_se(table_format(table, &formatted) >= 0); + fputs(formatted, stdout); + assert_se(streq(formatted, + "FOO BAR\n" + "three different lines two lines\n")); + formatted = mfree(formatted); + + table_set_cell_height_max(table, (size_t) -1); + assert_se(table_format(table, &formatted) >= 0); + fputs(formatted, stdout); + assert_se(streq(formatted, + "FOO BAR\n" + "three different lines two lines\n")); + formatted = mfree(formatted); + + assert_se(table_add_many(table, + TABLE_STRING, "short", + TABLE_STRV_WRAPPED, STRV_MAKE("a", "pair")) >= 0); + + assert_se(table_add_many(table, + TABLE_STRV_WRAPPED, STRV_MAKE("short2"), + TABLE_STRV_WRAPPED, STRV_MAKE("a", "eight", "line", "ćęłł", + "___5___", "___6___", "___7___", "___8___")) >= 0); + + table_set_cell_height_max(table, 1); + assert_se(table_format(table, &formatted) >= 0); + fputs(formatted, stdout); + assert_se(streq(formatted, + "FOO BAR\n" + "three different… two lines\n" + "short a pair\n" + "short2 a eight line ćęłł…\n")); + formatted = mfree(formatted); + + table_set_cell_height_max(table, 2); + assert_se(table_format(table, &formatted) >= 0); + fputs(formatted, stdout); + assert_se(streq(formatted, + "FOO BAR\n" + "three different two lines\n" + "lines \n" + "short a pair\n" + "short2 a eight line ćęłł\n" + " ___5___ ___6___…\n")); + formatted = mfree(formatted); + + table_set_cell_height_max(table, 3); + assert_se(table_format(table, &formatted) >= 0); + fputs(formatted, stdout); + assert_se(streq(formatted, + "FOO BAR\n" + "three different two lines\n" + "lines \n" + "short a pair\n" + "short2 a eight line ćęłł\n" + " ___5___ ___6___\n" + " ___7___ ___8___\n")); + formatted = mfree(formatted); + + table_set_cell_height_max(table, (size_t) -1); + assert_se(table_format(table, &formatted) >= 0); + fputs(formatted, stdout); + assert_se(streq(formatted, + "FOO BAR\n" + "three different two lines\n" + "lines \n" + "short a pair\n" + "short2 a eight line ćęłł\n" + " ___5___ ___6___\n" + " ___7___ ___8___\n")); + formatted = mfree(formatted); +} + +int main(int argc, char *argv[]) { _cleanup_(table_unrefp) Table *t = NULL; _cleanup_free_ char *formatted = NULL; @@ -399,6 +508,7 @@ int main(int argc, char *argv[]) { test_issue_9549(); test_multiline(); test_strv(); + test_strv_wrapped(); return 0; } From f08a64c5e10aed0a023e85ea664cc2f916fd6a23 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 12 Oct 2020 15:54:57 +0200 Subject: [PATCH 8/9] resolvect: use wrapping for various lists dump_list() is used for DNS servers, DNS domains, fallback DNS servers. --- src/resolve/resolvectl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/resolve/resolvectl.c b/src/resolve/resolvectl.c index 6f3de91b13..6ed7c2987d 100644 --- a/src/resolve/resolvectl.c +++ b/src/resolve/resolvectl.c @@ -1399,7 +1399,7 @@ static int dump_list(Table *table, const char *prefix, char * const *l) { r = table_add_many(table, TABLE_STRING, prefix, - TABLE_STRV, l); + TABLE_STRV_WRAPPED, l); if (r < 0) return table_log_add_error(r); From 7d1e1afe28d554b2bbf95966990f8e07c361647d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 20 Oct 2020 10:50:01 +0200 Subject: [PATCH 9/9] resolvectl: wrap the extended status string too --- src/resolve/resolvectl.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/resolve/resolvectl.c b/src/resolve/resolvectl.c index 6ed7c2987d..5cfdc2aa33 100644 --- a/src/resolve/resolvectl.c +++ b/src/resolve/resolvectl.c @@ -1418,7 +1418,7 @@ static int strv_extend_extended_bool(char ***strv, const char *name, const char return strv_extendf(strv, "%s=%s", name, value ?: "???"); } -static char* link_protocol_status(const LinkInfo *info) { +static char** link_protocol_status(const LinkInfo *info) { _cleanup_strv_free_ char **s = NULL; if (strv_extendf(&s, "%sDefaultRoute", plus_minus(info->default_route)) < 0) @@ -1438,10 +1438,10 @@ static char* link_protocol_status(const LinkInfo *info) { info->dnssec_supported ? "supported" : "unsupported") < 0) return NULL; - return strv_join(s, " "); + return TAKE_PTR(s); } -static char* global_protocol_status(const GlobalInfo *info) { +static char** global_protocol_status(const GlobalInfo *info) { _cleanup_strv_free_ char **s = NULL; if (strv_extend_extended_bool(&s, "LLMNR", info->llmnr) < 0) @@ -1458,7 +1458,7 @@ static char* global_protocol_status(const GlobalInfo *info) { info->dnssec_supported ? "supported" : "unsupported") < 0) return NULL; - return strv_join(s, " "); + return TAKE_PTR(s); } static int status_ifindex(sd_bus *bus, int ifindex, const char *name, StatusMode mode, bool *empty_line) { @@ -1604,13 +1604,13 @@ static int status_ifindex(sd_bus *bus, int ifindex, const char *name, StatusMode if (r < 0) return table_log_add_error(r); - _cleanup_free_ char *pstatus = link_protocol_status(&link_info); + _cleanup_strv_free_ char **pstatus = link_protocol_status(&link_info); if (!pstatus) return log_oom(); r = table_add_many(table, - TABLE_STRING, "Protocols:", - TABLE_STRING, pstatus); + TABLE_STRING, "Protocols:", + TABLE_STRV_WRAPPED, pstatus); if (r < 0) return table_log_add_error(r); @@ -1823,14 +1823,14 @@ static int status_global(sd_bus *bus, StatusMode mode, bool *empty_line) { table_set_header(table, false); - _cleanup_free_ char *pstatus = global_protocol_status(&global_info); + _cleanup_strv_free_ char **pstatus = global_protocol_status(&global_info); if (!pstatus) return log_oom(); r = table_add_many(table, - TABLE_STRING, "Protocols:", + TABLE_STRING, "Protocols:", TABLE_SET_ALIGN_PERCENT, 100, - TABLE_STRING, pstatus); + TABLE_STRV_WRAPPED, pstatus); if (r < 0) return table_log_add_error(r);