From 0cbb768abad835e940359d54312811732507be68 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 11 Feb 2021 03:27:22 +0900 Subject: [PATCH 1/9] conf-parser: use return codes in xxx_from_string() Follow-up for #11484. --- src/shared/conf-parser.h | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/src/shared/conf-parser.h b/src/shared/conf-parser.h index 283d60a473..5a7dc7ff59 100644 --- a/src/shared/conf-parser.h +++ b/src/shared/conf-parser.h @@ -207,7 +207,7 @@ typedef enum Disabled { \ x = from_string(rvalue); \ if (x < 0) { \ - log_syntax(unit, LOG_WARNING, filename, line, 0, \ + log_syntax(unit, LOG_WARNING, filename, line, x, \ msg ", ignoring: %s", rvalue); \ return 0; \ } \ @@ -235,7 +235,7 @@ typedef enum Disabled { \ x = name##_from_string(rvalue); \ if (x < 0) { \ - log_syntax(unit, LOG_WARNING, filename, line, 0, \ + log_syntax(unit, LOG_WARNING, filename, line, x, \ msg ", ignoring: %s", rvalue); \ return 0; \ } \ @@ -269,14 +269,17 @@ typedef enum Disabled { r = extract_first_word(&p, &en, NULL, 0); \ if (r == -ENOMEM) \ return log_oom(); \ - if (r < 0) \ - return log_syntax(unit, LOG_ERR, filename, line, 0, \ - msg ": %s", en); \ + if (r < 0) { \ + log_syntax(unit, LOG_WARNING, filename, line, r, \ + msg ", ignoring: %s", en); \ + return 0; \ + } \ if (r == 0) \ break; \ \ - if ((x = name##_from_string(en)) < 0) { \ - log_syntax(unit, LOG_WARNING, filename, line, 0, \ + x = name##_from_string(en); \ + if (x < 0) { \ + log_syntax(unit, LOG_WARNING, filename, line, x, \ msg ", ignoring: %s", en); \ continue; \ } \ From b98680b2b83c441915d09e7f394bb838abe48696 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 11 Feb 2021 04:21:46 +0900 Subject: [PATCH 2/9] tree-wide: use error codes in xxx_from_string() --- src/core/load-fragment.c | 34 +++++++++++++--------- src/network/netdev/l2tp-tunnel.c | 2 +- src/network/netdev/netdev.c | 2 +- src/network/netdev/tunnel.c | 3 +- src/network/networkd-conf.c | 2 +- src/network/networkd-dhcp-common.c | 2 +- src/network/networkd-lldp-tx.c | 2 +- src/network/networkd-radv.c | 2 +- src/network/networkd-route.c | 4 +-- src/network/networkd-routing-policy-rule.c | 4 +-- src/shared/conf-parser.c | 6 ++-- src/shared/ethtool-util.c | 2 +- 12 files changed, 36 insertions(+), 29 deletions(-) diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c index 06b71aaf15..b791156097 100644 --- a/src/core/load-fragment.c +++ b/src/core/load-fragment.c @@ -660,7 +660,7 @@ int config_parse_kill_mode( m = kill_mode_from_string(rvalue); if (m < 0) { - log_syntax(unit, LOG_WARNING, filename, line, 0, + log_syntax(unit, LOG_WARNING, filename, line, m, "Failed to parse kill mode specification, ignoring: %s", rvalue); return 0; } @@ -990,7 +990,7 @@ int config_parse_exec_input( } else { ei = exec_input_from_string(rvalue); if (ei < 0) { - log_syntax(unit, LOG_WARNING, filename, line, 0, "Failed to parse input specifier, ignoring: %s", rvalue); + log_syntax(unit, LOG_WARNING, filename, line, ei, "Failed to parse input specifier, ignoring: %s", rvalue); return 0; } } @@ -1219,7 +1219,7 @@ int config_parse_exec_output( } else { eo = exec_output_from_string(rvalue); if (eo < 0) { - log_syntax(unit, LOG_WARNING, filename, line, 0, "Failed to parse output specifier, ignoring: %s", rvalue); + log_syntax(unit, LOG_WARNING, filename, line, eo, "Failed to parse output specifier, ignoring: %s", rvalue); return 0; } } @@ -1278,7 +1278,7 @@ int config_parse_exec_io_class(const char *unit, x = ioprio_class_from_string(rvalue); if (x < 0) { - log_syntax(unit, LOG_WARNING, filename, line, 0, "Failed to parse IO scheduling class, ignoring: %s", rvalue); + log_syntax(unit, LOG_WARNING, filename, line, x, "Failed to parse IO scheduling class, ignoring: %s", rvalue); return 0; } @@ -1353,7 +1353,7 @@ int config_parse_exec_cpu_sched_policy(const char *unit, x = sched_policy_from_string(rvalue); if (x < 0) { - log_syntax(unit, LOG_WARNING, filename, line, 0, "Failed to parse CPU scheduling policy, ignoring: %s", rvalue); + log_syntax(unit, LOG_WARNING, filename, line, x, "Failed to parse CPU scheduling policy, ignoring: %s", rvalue); return 0; } @@ -1527,7 +1527,8 @@ int config_parse_root_image_options( partition_designator = partition_designator_from_string(partition); if (partition_designator < 0) { - log_syntax(unit, LOG_WARNING, filename, line, 0, "Invalid partition name %s, ignoring", partition); + log_syntax(unit, LOG_WARNING, filename, line, partition_designator, + "Invalid partition name %s, ignoring", partition); continue; } r = unit_full_printf(u, mount_options, &mount_options_resolved); @@ -2015,7 +2016,7 @@ int config_parse_trigger_unit( type = unit_name_to_type(p); if (type < 0) { - log_syntax(unit, LOG_WARNING, filename, line, 0, "Unit type not valid, ignoring: %s", rvalue); + log_syntax(unit, LOG_WARNING, filename, line, type, "Unit type not valid, ignoring: %s", rvalue); return 0; } if (unit_has_name(u, p)) { @@ -2062,7 +2063,7 @@ int config_parse_path_spec(const char *unit, b = path_type_from_string(lvalue); if (b < 0) { - log_syntax(unit, LOG_WARNING, filename, line, 0, "Failed to parse path type, ignoring: %s", lvalue); + log_syntax(unit, LOG_WARNING, filename, line, b, "Failed to parse path type, ignoring: %s", lvalue); return 0; } @@ -3386,8 +3387,12 @@ int config_parse_syscall_errno( } e = parse_errno(rvalue); - if (e <= 0) { - log_syntax(unit, LOG_WARNING, filename, line, 0, "Failed to parse error number, ignoring: %s", rvalue); + if (e < 0) { + log_syntax(unit, LOG_WARNING, filename, line, e, "Failed to parse error number, ignoring: %s", rvalue); + return 0; + } + if (e == 0) { + log_syntax(unit, LOG_WARNING, filename, line, 0, "Invalid error number, ignoring: %s", rvalue); return 0; } @@ -3857,7 +3862,7 @@ int config_parse_managed_oom_mode( m = managed_oom_mode_from_string(rvalue); if (m < 0) { - log_syntax(unit, LOG_WARNING, filename, line, 0, "Invalid syntax, ignoring: %s", rvalue); + log_syntax(unit, LOG_WARNING, filename, line, m, "Invalid syntax, ignoring: %s", rvalue); return 0; } *mode = m; @@ -4668,7 +4673,7 @@ int config_parse_set_status( } else { r = signal_from_string(word); if (r < 0) { - log_syntax(unit, LOG_WARNING, filename, line, 0, + log_syntax(unit, LOG_WARNING, filename, line, r, "Failed to parse value, ignoring: %s", word); continue; } @@ -5085,7 +5090,8 @@ int config_parse_mount_images( partition_designator = partition_designator_from_string(partition); if (partition_designator < 0) { - log_syntax(unit, LOG_WARNING, filename, line, 0, "Invalid partition name %s, ignoring", partition); + log_syntax(unit, LOG_WARNING, filename, line, partition_designator, + "Invalid partition name %s, ignoring", partition); continue; } r = unit_full_printf(u, mount_options, &mount_options_resolved); @@ -5774,7 +5780,7 @@ int config_parse_output_restricted( } else { t = exec_output_from_string(rvalue); if (t < 0) { - log_syntax(unit, LOG_WARNING, filename, line, 0, "Failed to parse output type, ignoring: %s", rvalue); + log_syntax(unit, LOG_WARNING, filename, line, t, "Failed to parse output type, ignoring: %s", rvalue); return 0; } diff --git a/src/network/netdev/l2tp-tunnel.c b/src/network/netdev/l2tp-tunnel.c index da23db947f..e302a2cf3b 100644 --- a/src/network/netdev/l2tp-tunnel.c +++ b/src/network/netdev/l2tp-tunnel.c @@ -577,7 +577,7 @@ int config_parse_l2tp_session_l2spec( spec = l2tp_l2spec_type_from_string(rvalue); if (spec < 0) { - log_syntax(unit, LOG_WARNING, filename, line, 0, + log_syntax(unit, LOG_WARNING, filename, line, spec, "Failed to parse layer2 specific header type. Ignoring assignment: %s", rvalue); return 0; } diff --git a/src/network/netdev/netdev.c b/src/network/netdev/netdev.c index 81e8fd7cf1..b26064297b 100644 --- a/src/network/netdev/netdev.c +++ b/src/network/netdev/netdev.c @@ -139,7 +139,7 @@ int config_parse_netdev_kind( k = netdev_kind_from_string(rvalue); if (k < 0) { - log_syntax(unit, LOG_WARNING, filename, line, 0, "Failed to parse netdev kind, ignoring assignment: %s", rvalue); + log_syntax(unit, LOG_WARNING, filename, line, k, "Failed to parse netdev kind, ignoring assignment: %s", rvalue); return 0; } diff --git a/src/network/netdev/tunnel.c b/src/network/netdev/tunnel.c index 66e8868284..fdaec488a2 100644 --- a/src/network/netdev/tunnel.c +++ b/src/network/netdev/tunnel.c @@ -581,7 +581,8 @@ int config_parse_tunnel_key(const char *unit, if (r < 0) { r = safe_atou32(rvalue, &k); if (r < 0) { - log_syntax(unit, LOG_WARNING, filename, line, 0, "Failed to parse tunnel key ignoring assignment: %s", rvalue); + log_syntax(unit, LOG_WARNING, filename, line, r, + "Failed to parse tunnel key ignoring assignment: %s", rvalue); return 0; } } else diff --git a/src/network/networkd-conf.c b/src/network/networkd-conf.c index bf51624ec4..c413f16739 100644 --- a/src/network/networkd-conf.c +++ b/src/network/networkd-conf.c @@ -93,7 +93,7 @@ int config_parse_duid_type( type = duid_type_from_string(type_string); if (type < 0) { - log_syntax(unit, LOG_WARNING, filename, line, 0, + log_syntax(unit, LOG_WARNING, filename, line, type, "Failed to parse DUID type '%s', ignoring.", type_string); return 0; } diff --git a/src/network/networkd-dhcp-common.c b/src/network/networkd-dhcp-common.c index cec7bb6369..e50f4bb70f 100644 --- a/src/network/networkd-dhcp-common.c +++ b/src/network/networkd-dhcp-common.c @@ -671,7 +671,7 @@ int config_parse_dhcp_send_option( type = dhcp_option_data_type_from_string(word); if (type < 0) { - log_syntax(unit, LOG_WARNING, filename, line, 0, + log_syntax(unit, LOG_WARNING, filename, line, type, "Invalid DHCP option data type, ignoring assignment: %s", p); return 0; } diff --git a/src/network/networkd-lldp-tx.c b/src/network/networkd-lldp-tx.c index b03d948bd8..de61e5119b 100644 --- a/src/network/networkd-lldp-tx.c +++ b/src/network/networkd-lldp-tx.c @@ -445,7 +445,7 @@ int config_parse_lldp_emit( else { r = parse_boolean(rvalue); if (r < 0) { - log_syntax(unit, LOG_WARNING, filename, line, 0, "Failed to parse LLDP emission setting, ignoring: %s", rvalue); + log_syntax(unit, LOG_WARNING, filename, line, r, "Failed to parse LLDP emission setting, ignoring: %s", rvalue); return 0; } diff --git a/src/network/networkd-radv.c b/src/network/networkd-radv.c index 35bf68cd7e..39246498e4 100644 --- a/src/network/networkd-radv.c +++ b/src/network/networkd-radv.c @@ -948,7 +948,7 @@ int config_parse_router_prefix_delegation( /* For backward compatibility */ val = radv_prefix_delegation_from_string(rvalue); if (val < 0) { - log_syntax(unit, LOG_WARNING, filename, line, 0, + log_syntax(unit, LOG_WARNING, filename, line, val, "Invalid %s= setting, ignoring assignment: %s", lvalue, rvalue); return 0; } diff --git a/src/network/networkd-route.c b/src/network/networkd-route.c index a98394c1ca..d0ebb4c320 100644 --- a/src/network/networkd-route.c +++ b/src/network/networkd-route.c @@ -1909,7 +1909,7 @@ int config_parse_route_scope( r = route_scope_from_string(rvalue); if (r < 0) { - log_syntax(unit, LOG_WARNING, filename, line, 0, "Unknown route scope: %s", rvalue); + log_syntax(unit, LOG_WARNING, filename, line, r, "Unknown route scope: %s", rvalue); return 0; } @@ -2124,7 +2124,7 @@ int config_parse_route_type( t = route_type_from_string(rvalue); if (t < 0) { - log_syntax(unit, LOG_WARNING, filename, line, 0, + log_syntax(unit, LOG_WARNING, filename, line, r, "Could not parse route type \"%s\", ignoring assignment: %m", rvalue); return 0; } diff --git a/src/network/networkd-routing-policy-rule.c b/src/network/networkd-routing-policy-rule.c index da8778dd1e..7802a1e55a 100644 --- a/src/network/networkd-routing-policy-rule.c +++ b/src/network/networkd-routing-policy-rule.c @@ -1422,7 +1422,7 @@ int config_parse_routing_policy_rule_family( a = routing_policy_rule_address_family_from_string(rvalue); if (a < 0) { - log_syntax(unit, LOG_WARNING, filename, line, 0, + log_syntax(unit, LOG_WARNING, filename, line, a, "Invalid address family '%s', ignoring.", rvalue); return 0; } @@ -1547,7 +1547,7 @@ int config_parse_routing_policy_rule_type( t = fr_act_type_from_string(rvalue); if (t < 0) { - log_syntax(unit, LOG_WARNING, filename, line, 0, + log_syntax(unit, LOG_WARNING, filename, line, t, "Could not parse FIB rule type \"%s\", ignoring assignment: %m", rvalue); return 0; } diff --git a/src/shared/conf-parser.c b/src/shared/conf-parser.c index f838d4513d..615782862b 100644 --- a/src/shared/conf-parser.c +++ b/src/shared/conf-parser.c @@ -874,7 +874,7 @@ int config_parse_log_facility( x = log_facility_unshifted_from_string(rvalue); if (x < 0) { - log_syntax(unit, LOG_WARNING, filename, line, 0, "Failed to parse log facility, ignoring: %s", rvalue); + log_syntax(unit, LOG_WARNING, filename, line, x, "Failed to parse log facility, ignoring: %s", rvalue); return 0; } @@ -904,7 +904,7 @@ int config_parse_log_level( x = log_level_from_string(rvalue); if (x < 0) { - log_syntax(unit, LOG_WARNING, filename, line, 0, "Failed to parse log level, ignoring: %s", rvalue); + log_syntax(unit, LOG_WARNING, filename, line, x, "Failed to parse log level, ignoring: %s", rvalue); return 0; } @@ -937,7 +937,7 @@ int config_parse_signal( r = signal_from_string(rvalue); if (r <= 0) { - log_syntax(unit, LOG_WARNING, filename, line, 0, "Failed to parse signal name, ignoring: %s", rvalue); + log_syntax(unit, LOG_WARNING, filename, line, r, "Failed to parse signal name, ignoring: %s", rvalue); return 0; } diff --git a/src/shared/ethtool-util.c b/src/shared/ethtool-util.c index 3b3d37cccc..654d36a83a 100644 --- a/src/shared/ethtool-util.c +++ b/src/shared/ethtool-util.c @@ -1092,7 +1092,7 @@ int config_parse_advertise(const char *unit, /* We reuse the kernel provided enum which does not contain negative value. So, the cast * below is mandatory. Otherwise, the check below always passes and access an invalid address. */ if ((int) mode < 0) { - log_syntax(unit, LOG_WARNING, filename, line, 0, + log_syntax(unit, LOG_WARNING, filename, line, mode, "Failed to parse advertise mode, ignoring: %s", w); continue; } From 641bcbc56b51c3e2f33aeb09dac153c06adb2b47 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 11 Feb 2021 04:42:33 +0900 Subject: [PATCH 3/9] string-table: introduce DEFINE_PRIVATE_STRING_TABLE_LOOKUP_FROM_STRING_WITH_BOOLEAN() --- src/basic/string-table.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/basic/string-table.h b/src/basic/string-table.h index 5caedacb01..491fb51044 100644 --- a/src/basic/string-table.h +++ b/src/basic/string-table.h @@ -87,6 +87,8 @@ ssize_t string_table_lookup(const char * const *table, size_t len, const char *k #define DEFINE_STRING_TABLE_LOOKUP_WITH_BOOLEAN(name,type,yes) _DEFINE_STRING_TABLE_LOOKUP_WITH_BOOLEAN(name,type,yes,) #define DEFINE_PRIVATE_STRING_TABLE_LOOKUP_WITH_BOOLEAN(name,type,yes) _DEFINE_STRING_TABLE_LOOKUP_WITH_BOOLEAN(name,type,yes,static) +#define DEFINE_PRIVATE_STRING_TABLE_LOOKUP_FROM_STRING_WITH_BOOLEAN(name,type,yes) \ + _DEFINE_STRING_TABLE_LOOKUP_FROM_STRING_WITH_BOOLEAN(name,type,yes,static) /* For string conversions where numbers are also acceptable */ #define DEFINE_STRING_TABLE_LOOKUP_WITH_FALLBACK(name,type,max) \ From 0fcc4e74847c122b9e1f99dfc79d935837cf1a22 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 11 Feb 2021 04:34:14 +0900 Subject: [PATCH 4/9] networkd: lldp: use string table --- src/network/networkd-lldp-tx.c | 51 ++++++++-------------------------- src/network/networkd-lldp-tx.h | 1 + 2 files changed, 12 insertions(+), 40 deletions(-) diff --git a/src/network/networkd-lldp-tx.c b/src/network/networkd-lldp-tx.c index de61e5119b..1b97e6584e 100644 --- a/src/network/networkd-lldp-tx.c +++ b/src/network/networkd-lldp-tx.c @@ -17,6 +17,7 @@ #include "parse-util.h" #include "random-util.h" #include "socket-util.h" +#include "string-table.h" #include "string-util.h" #include "strv.h" #include "unaligned.h" @@ -415,46 +416,6 @@ void link_lldp_emit_stop(Link *link) { link->lldp_emit_event_source = sd_event_source_unref(link->lldp_emit_event_source); } -int config_parse_lldp_emit( - const char *unit, - const char *filename, - unsigned line, - const char *section, - unsigned section_line, - const char *lvalue, - int ltype, - const char *rvalue, - void *data, - void *userdata) { - - LLDPEmit *emit = data; - int r; - - assert(filename); - assert(lvalue); - assert(rvalue); - - if (isempty(rvalue)) - *emit = LLDP_EMIT_NO; - else if (streq(rvalue, "nearest-bridge")) - *emit = LLDP_EMIT_NEAREST_BRIDGE; - else if (streq(rvalue, "non-tpmr-bridge")) - *emit = LLDP_EMIT_NON_TPMR_BRIDGE; - else if (streq(rvalue, "customer-bridge")) - *emit = LLDP_EMIT_CUSTOMER_BRIDGE; - else { - r = parse_boolean(rvalue); - if (r < 0) { - log_syntax(unit, LOG_WARNING, filename, line, r, "Failed to parse LLDP emission setting, ignoring: %s", rvalue); - return 0; - } - - *emit = r ? LLDP_EMIT_NEAREST_BRIDGE : LLDP_EMIT_NO; - } - - return 0; -} - int config_parse_lldp_mud( const char *unit, const char *filename, @@ -491,3 +452,13 @@ int config_parse_lldp_mud( return free_and_replace(n->lldp_mud, unescaped); } + +static const char * const lldp_emit_table[_LLDP_EMIT_MAX] = { + [LLDP_EMIT_NO] = "no", + [LLDP_EMIT_NEAREST_BRIDGE] = "nearest-bridge", + [LLDP_EMIT_NON_TPMR_BRIDGE] = "non-tpmr-bridge", + [LLDP_EMIT_CUSTOMER_BRIDGE] = "customer-bridge", +}; + +DEFINE_PRIVATE_STRING_TABLE_LOOKUP_FROM_STRING_WITH_BOOLEAN(lldp_emit, LLDPEmit, LLDP_EMIT_NEAREST_BRIDGE); +DEFINE_CONFIG_PARSE_ENUM_WITH_DEFAULT(config_parse_lldp_emit, lldp_emit, LLDPEmit, LLDP_EMIT_NO, "Failed to parse LLDP emission setting"); diff --git a/src/network/networkd-lldp-tx.h b/src/network/networkd-lldp-tx.h index aae30cba0f..b8917f9de8 100644 --- a/src/network/networkd-lldp-tx.h +++ b/src/network/networkd-lldp-tx.h @@ -13,6 +13,7 @@ typedef enum LLDPEmit { LLDP_EMIT_NON_TPMR_BRIDGE, LLDP_EMIT_CUSTOMER_BRIDGE, _LLDP_EMIT_MAX, + _LLDP_EMIT_INVALID = -EINVAL, } LLDPEmit; bool link_lldp_emit_enabled(Link *link); From e4ffe10367d01a605ee5872245929db54264a78b Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 11 Feb 2021 04:52:42 +0900 Subject: [PATCH 5/9] network: route: use _WITH_FALLBACK macros --- src/network/networkd-route.c | 60 +++++++++--------------------------- 1 file changed, 15 insertions(+), 45 deletions(-) diff --git a/src/network/networkd-route.c b/src/network/networkd-route.c index d0ebb4c320..ef977dac5c 100644 --- a/src/network/networkd-route.c +++ b/src/network/networkd-route.c @@ -65,21 +65,8 @@ static const char * const route_scope_table[] = { [RT_SCOPE_NOWHERE] = "nowhere", }; -DEFINE_PRIVATE_STRING_TABLE_LOOKUP(route_scope, int); - -#define ROUTE_SCOPE_STR_MAX CONST_MAX(DECIMAL_STR_MAX(int), STRLEN("nowhere") + 1) -static const char *format_route_scope(int scope, char *buf, size_t size) { - const char *s; - char *p = buf; - - s = route_scope_to_string(scope); - if (s) - strpcpy(&p, size, s); - else - strpcpyf(&p, size, "%d", scope); - - return buf; -} +DEFINE_PRIVATE_STRING_TABLE_LOOKUP_FROM_STRING(route_scope, int); +DEFINE_PRIVATE_STRING_TABLE_LOOKUP_TO_STRING_FALLBACK(route_scope, int, UINT8_MAX); static const char * const route_table_table[] = { [RT_TABLE_DEFAULT] = "default", @@ -157,7 +144,7 @@ static const char * const route_protocol_table[] = { [RTPROT_STATIC] = "static", }; -DEFINE_PRIVATE_STRING_TABLE_LOOKUP_FROM_STRING(route_protocol, int); +DEFINE_PRIVATE_STRING_TABLE_LOOKUP_FROM_STRING_FALLBACK(route_protocol, int, UINT8_MAX); static const char * const route_protocol_full_table[] = { [RTPROT_REDIRECT] = "redirect", @@ -182,21 +169,7 @@ static const char * const route_protocol_full_table[] = { [RTPROT_EIGRP] = "eigrp", }; -DEFINE_PRIVATE_STRING_TABLE_LOOKUP_TO_STRING(route_protocol_full, int); - -#define ROUTE_PROTOCOL_STR_MAX CONST_MAX(DECIMAL_STR_MAX(int), STRLEN("redirect") + 1) -static const char *format_route_protocol(int protocol, char *buf, size_t size) { - const char *s; - char *p = buf; - - s = route_protocol_full_to_string(protocol); - if (s) - strpcpy(&p, size, s); - else - strpcpyf(&p, size, "%d", protocol); - - return buf; -} +DEFINE_PRIVATE_STRING_TABLE_LOOKUP_TO_STRING_FALLBACK(route_protocol_full, int, UINT8_MAX); static unsigned routes_max(void) { static thread_local unsigned cached = 0; @@ -640,8 +613,8 @@ static void log_route_debug(const Route *route, const char *str, const Link *lin /* link may be NULL. */ if (DEBUG_LOGGING) { - _cleanup_free_ char *dst = NULL, *dst_prefixlen = NULL, *src = NULL, *gw = NULL, *prefsrc = NULL, *table = NULL; - char scope[ROUTE_SCOPE_STR_MAX], protocol[ROUTE_PROTOCOL_STR_MAX]; + _cleanup_free_ char *dst = NULL, *dst_prefixlen = NULL, *src = NULL, *gw = NULL, + *prefsrc = NULL, *table = NULL, *scope = NULL, *proto = NULL; if (!in_addr_is_null(route->family, &route->dst)) { (void) in_addr_to_string(route->family, &route->dst, &dst); @@ -653,14 +626,14 @@ static void log_route_debug(const Route *route, const char *str, const Link *lin (void) in_addr_to_string(route->gw_family, &route->gw, &gw); if (!in_addr_is_null(route->family, &route->prefsrc)) (void) in_addr_to_string(route->family, &route->prefsrc, &prefsrc); + (void) route_scope_to_string_alloc(route->scope, &scope); (void) manager_get_route_table_to_string(m, route->table, &table); + (void) route_protocol_full_to_string_alloc(route->protocol, &proto); log_link_debug(link, "%s route: dst: %s%s, src: %s, gw: %s, prefsrc: %s, scope: %s, table: %s, proto: %s, type: %s", str, strna(dst), strempty(dst_prefixlen), strna(src), strna(gw), strna(prefsrc), - format_route_scope(route->scope, scope, sizeof(scope)), - strna(table), - format_route_protocol(route->protocol, protocol, sizeof(protocol)), + strna(scope), strna(table), strna(proto), strna(route_type_to_string(route->type))); } } @@ -2082,17 +2055,14 @@ int config_parse_route_protocol( } r = route_protocol_from_string(rvalue); - if (r >= 0) - n->protocol = r; - else { - r = safe_atou8(rvalue , &n->protocol); - if (r < 0) { - log_syntax(unit, LOG_WARNING, filename, line, r, - "Could not parse route protocol \"%s\", ignoring assignment: %m", rvalue); - return 0; - } + if (r < 0) { + log_syntax(unit, LOG_WARNING, filename, line, r, + "Failed to parse route protocol \"%s\", ignoring assignment: %m", rvalue); + return 0; } + n->protocol = r; + TAKE_PTR(n); return 0; } From 58f99ed8a821d8b24fa3df3bf1014dee6a95606b Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 11 Feb 2021 05:19:18 +0900 Subject: [PATCH 6/9] string-table: introduce DEFINE_STRING_TABLE_LOOKUP_FROM_STRING() --- src/basic/string-table.h | 1 + 1 file changed, 1 insertion(+) diff --git a/src/basic/string-table.h b/src/basic/string-table.h index 491fb51044..aca6a267bd 100644 --- a/src/basic/string-table.h +++ b/src/basic/string-table.h @@ -81,6 +81,7 @@ ssize_t string_table_lookup(const char * const *table, size_t len, const char *k #define DEFINE_STRING_TABLE_LOOKUP(name,type) _DEFINE_STRING_TABLE_LOOKUP(name,type,) #define DEFINE_STRING_TABLE_LOOKUP_TO_STRING(name,type) _DEFINE_STRING_TABLE_LOOKUP_TO_STRING(name,type,) +#define DEFINE_STRING_TABLE_LOOKUP_FROM_STRING(name,type) _DEFINE_STRING_TABLE_LOOKUP_FROM_STRING(name,type,) #define DEFINE_PRIVATE_STRING_TABLE_LOOKUP(name,type) _DEFINE_STRING_TABLE_LOOKUP(name,type,static) #define DEFINE_PRIVATE_STRING_TABLE_LOOKUP_TO_STRING(name,type) _DEFINE_STRING_TABLE_LOOKUP_TO_STRING(name,type,static) #define DEFINE_PRIVATE_STRING_TABLE_LOOKUP_FROM_STRING(name,type) _DEFINE_STRING_TABLE_LOOKUP_FROM_STRING(name,type,static) From bde8467a0de6fd2ec0a0738d9f3f0689ae5dd37f Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 11 Feb 2021 05:06:19 +0900 Subject: [PATCH 7/9] network: dhcp: use string table --- src/network/networkd-dhcp-common.c | 13 +++---------- src/network/networkd-util.c | 8 ++++++++ src/network/networkd-util.h | 2 ++ 3 files changed, 13 insertions(+), 10 deletions(-) diff --git a/src/network/networkd-dhcp-common.c b/src/network/networkd-dhcp-common.c index e50f4bb70f..6bac720200 100644 --- a/src/network/networkd-dhcp-common.c +++ b/src/network/networkd-dhcp-common.c @@ -283,16 +283,9 @@ int config_parse_dhcp( /* Previously, we had a slightly different enum here, * support its values for compatibility. */ - if (streq(rvalue, "none")) - s = ADDRESS_FAMILY_NO; - else if (streq(rvalue, "v4")) - s = ADDRESS_FAMILY_IPV4; - else if (streq(rvalue, "v6")) - s = ADDRESS_FAMILY_IPV6; - else if (streq(rvalue, "both")) - s = ADDRESS_FAMILY_YES; - else { - log_syntax(unit, LOG_WARNING, filename, line, 0, + s = dhcp_deprecated_address_family_from_string(rvalue); + if (s < 0) { + log_syntax(unit, LOG_WARNING, filename, line, s, "Failed to parse DHCP option, ignoring: %s", rvalue); return 0; } diff --git a/src/network/networkd-util.c b/src/network/networkd-util.c index 83b38b2b05..b41148d2c6 100644 --- a/src/network/networkd-util.c +++ b/src/network/networkd-util.c @@ -28,6 +28,13 @@ static const char* const duplicate_address_detection_address_family_table[_ADDRE [ADDRESS_FAMILY_IPV6] = "ipv6", }; +static const char* const dhcp_deprecated_address_family_table[_ADDRESS_FAMILY_MAX] = { + [ADDRESS_FAMILY_NO] = "none", + [ADDRESS_FAMILY_YES] = "both", + [ADDRESS_FAMILY_IPV4] = "v4", + [ADDRESS_FAMILY_IPV6] = "v6", +}; + static const char* const dhcp_lease_server_type_table[_SD_DHCP_LEASE_SERVER_TYPE_MAX] = { [SD_DHCP_LEASE_DNS] = "DNS servers", [SD_DHCP_LEASE_NTP] = "NTP servers", @@ -51,6 +58,7 @@ DEFINE_STRING_TABLE_LOOKUP(routing_policy_rule_address_family, AddressFamily); DEFINE_STRING_TABLE_LOOKUP(duplicate_address_detection_address_family, AddressFamily); DEFINE_CONFIG_PARSE_ENUM(config_parse_link_local_address_family, link_local_address_family, AddressFamily, "Failed to parse option"); +DEFINE_STRING_TABLE_LOOKUP_FROM_STRING(dhcp_deprecated_address_family, AddressFamily); DEFINE_STRING_TABLE_LOOKUP(dhcp_lease_server_type, sd_dhcp_lease_server_type); int config_parse_address_family_with_kernel( diff --git a/src/network/networkd-util.h b/src/network/networkd-util.h index cc5ee35558..0cfb009890 100644 --- a/src/network/networkd-util.h +++ b/src/network/networkd-util.h @@ -40,6 +40,8 @@ AddressFamily routing_policy_rule_address_family_from_string(const char *s) _pur const char *duplicate_address_detection_address_family_to_string(AddressFamily b) _const_; AddressFamily duplicate_address_detection_address_family_from_string(const char *s) _pure_; +AddressFamily dhcp_deprecated_address_family_from_string(const char *s) _pure_; + const char *dhcp_lease_server_type_to_string(sd_dhcp_lease_server_type t) _const_; sd_dhcp_lease_server_type dhcp_lease_server_type_from_string(const char *s) _pure_; From 7fb1d980aff8b035d96a34bd30efff261f3a5494 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 11 Feb 2021 05:21:42 +0900 Subject: [PATCH 8/9] tree-wide: propagate error in xxx_from-string() --- src/core/mount.c | 18 ++++++++++-------- src/firstboot/firstboot.c | 6 +++--- src/udev/udevadm-trigger.c | 11 ++++++++--- 3 files changed, 21 insertions(+), 14 deletions(-) diff --git a/src/core/mount.c b/src/core/mount.c index 1ad80886cd..23b558859c 100644 --- a/src/core/mount.c +++ b/src/core/mount.c @@ -1249,8 +1249,9 @@ static int mount_deserialize_item(Unit *u, const char *key, const char *value, F if (streq(key, "state")) { MountState state; - if ((state = mount_state_from_string(value)) < 0) - log_unit_debug(u, "Failed to parse state value: %s", value); + state = mount_state_from_string(value); + if (state < 0) + log_unit_debug_errno(u, state, "Failed to parse state value: %s", value); else m->deserialized_state = state; @@ -1259,7 +1260,7 @@ static int mount_deserialize_item(Unit *u, const char *key, const char *value, F f = mount_result_from_string(value); if (f < 0) - log_unit_debug(u, "Failed to parse result value: %s", value); + log_unit_debug_errno(u, f, "Failed to parse result value: %s", value); else if (f != MOUNT_SUCCESS) m->result = f; @@ -1268,7 +1269,7 @@ static int mount_deserialize_item(Unit *u, const char *key, const char *value, F f = mount_result_from_string(value); if (f < 0) - log_unit_debug(u, "Failed to parse reload result value: %s", value); + log_unit_debug_errno(u, f, "Failed to parse reload result value: %s", value); else if (f != MOUNT_SUCCESS) m->reload_result = f; @@ -1276,19 +1277,20 @@ static int mount_deserialize_item(Unit *u, const char *key, const char *value, F r = safe_atou(value, &m->n_retry_umount); if (r < 0) - log_unit_debug(u, "Failed to parse n-retry-umount value: %s", value); + log_unit_debug_errno(u, r, "Failed to parse n-retry-umount value: %s", value); } else if (streq(key, "control-pid")) { - if (parse_pid(value, &m->control_pid) < 0) - log_unit_debug(u, "Failed to parse control-pid value: %s", value); + r = parse_pid(value, &m->control_pid); + if (r < 0) + log_unit_debug_errno(u, r, "Failed to parse control-pid value: %s", value); } else if (streq(key, "control-command")) { MountExecCommand id; id = mount_exec_command_from_string(value); if (id < 0) - log_unit_debug(u, "Failed to parse exec-command value: %s", value); + log_unit_debug_errno(u, id, "Failed to parse exec-command value: %s", value); else { m->control_command_id = id; m->control_command = m->exec_command + id; diff --git a/src/firstboot/firstboot.c b/src/firstboot/firstboot.c index 904612ab4a..a9cc03cc3f 100644 --- a/src/firstboot/firstboot.c +++ b/src/firstboot/firstboot.c @@ -1146,9 +1146,9 @@ static int parse_argv(int argc, char *argv[]) { break; case ARG_MACHINE_ID: - if (sd_id128_from_string(optarg, &arg_machine_id) < 0) - return log_error_errno(SYNTHETIC_ERRNO(EINVAL), - "Failed to parse machine id %s.", optarg); + r = sd_id128_from_string(optarg, &arg_machine_id); + if (r < 0) + return log_error_errno(r, "Failed to parse machine id %s.", optarg); break; diff --git a/src/udev/udevadm-trigger.c b/src/udev/udevadm-trigger.c index 7a9f968128..a4588cb4e4 100644 --- a/src/udev/udevadm-trigger.c +++ b/src/udev/udevadm-trigger.c @@ -210,16 +210,21 @@ int trigger_main(int argc, char *argv[], void *userdata) { else return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Unknown type --type=%s", optarg); break; - case 'c': + case 'c': { + DeviceAction a; + if (streq(optarg, "help")) { dump_device_action_table(); return 0; } - if (device_action_from_string(optarg) < 0) - return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Unknown action '%s'", optarg); + + a = device_action_from_string(optarg); + if (a < 0) + return log_error_errno(a, "Unknown action '%s'", optarg); action = optarg; break; + } case 's': r = sd_device_enumerator_add_match_subsystem(e, optarg, true); if (r < 0) From b3f9c17a6b361bf98e79a0228cdcb09afc40d1cc Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 11 Feb 2021 05:48:10 +0900 Subject: [PATCH 9/9] tree-wide: use free_and_strdup_warn() --- src/core/load-fragment.c | 5 +---- src/cryptsetup/cryptsetup-generator.c | 9 +++------ src/debug-generator/debug-generator.c | 14 ++++---------- src/firstboot/firstboot.c | 5 +---- src/fstab-generator/fstab-generator.c | 15 +++++---------- src/mount/mount-tool.c | 15 +++++++++------ src/network/networkd-manager.c | 10 ++++++---- src/nspawn/nspawn-settings.c | 5 +---- src/run-generator/run-generator.c | 6 ++---- src/shared/conf-parser.c | 5 +---- src/udev/net/link-config.c | 5 +---- src/udev/udev-rules.c | 5 +++-- 12 files changed, 37 insertions(+), 62 deletions(-) diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c index b791156097..410168d4d6 100644 --- a/src/core/load-fragment.c +++ b/src/core/load-fragment.c @@ -920,10 +920,7 @@ int config_parse_socket_bindtodevice( return 0; } - if (free_and_strdup(&s->bind_to_device, rvalue) < 0) - return log_oom(); - - return 0; + return free_and_strdup_warn(&s->bind_to_device, rvalue); } int config_parse_exec_input( diff --git a/src/cryptsetup/cryptsetup-generator.c b/src/cryptsetup/cryptsetup-generator.c index 0ae8ab35aa..74f739b513 100644 --- a/src/cryptsetup/cryptsetup-generator.c +++ b/src/cryptsetup/cryptsetup-generator.c @@ -667,7 +667,7 @@ static int parse_proc_cmdline_item(const char *key, const char *value, void *dat r = sscanf(value, "%m[0-9a-fA-F-]=%ms", &uuid, &uuid_value); if (r != 2) - return free_and_strdup(&arg_default_options, value) < 0 ? log_oom() : 0; + return free_and_strdup_warn(&arg_default_options, value); if (warn_uuid_invalid(uuid, key)) return 0; @@ -691,11 +691,8 @@ static int parse_proc_cmdline_item(const char *key, const char *value, void *dat return 0; n = strspn(value, ALPHANUMERICAL "-"); - if (value[n] != '=') { - if (free_and_strdup(&arg_default_keyfile, value) < 0) - return log_oom(); - return 0; - } + if (value[n] != '=') + return free_and_strdup_warn(&arg_default_keyfile, value); uuid = strndup(value, n); if (!uuid) diff --git a/src/debug-generator/debug-generator.c b/src/debug-generator/debug-generator.c index c6e4d79e3a..d9fccea5c9 100644 --- a/src/debug-generator/debug-generator.c +++ b/src/debug-generator/debug-generator.c @@ -68,27 +68,21 @@ static int parse_proc_cmdline_item(const char *key, const char *value, void *dat else if (r > 0) t = skip_dev_prefix(DEBUGTTY); - if (free_and_strdup(&arg_debug_shell, t) < 0) - return log_oom(); + return free_and_strdup_warn(&arg_debug_shell, t); } else if (streq(key, "systemd.unit")) { if (proc_cmdline_value_missing(key, value)) return 0; - r = free_and_strdup(&arg_default_unit, value); - if (r < 0) - return log_error_errno(r, "Failed to set default unit %s: %m", value); + return free_and_strdup_warn(&arg_default_unit, value); } else if (!value) { const char *target; target = runlevel_to_target(key); - if (target) { - r = free_and_strdup(&arg_default_unit, target); - if (r < 0) - return log_error_errno(r, "Failed to set default unit %s: %m", target); - } + if (target) + return free_and_strdup_warn(&arg_default_unit, target); } return 0; diff --git a/src/firstboot/firstboot.c b/src/firstboot/firstboot.c index a9cc03cc3f..522d0ff1f0 100644 --- a/src/firstboot/firstboot.c +++ b/src/firstboot/firstboot.c @@ -210,10 +210,7 @@ static int prompt_loop(const char *text, char **l, unsigned percentage, bool (*i } log_info("Selected '%s'.", l[u-1]); - if (free_and_strdup(ret, l[u-1]) < 0) - return log_oom(); - - return 0; + return free_and_strdup_warn(ret, l[u-1]); } if (!is_valid(p)) { diff --git a/src/fstab-generator/fstab-generator.c b/src/fstab-generator/fstab-generator.c index b8862f0793..a526d6e8fa 100644 --- a/src/fstab-generator/fstab-generator.c +++ b/src/fstab-generator/fstab-generator.c @@ -827,16 +827,14 @@ static int parse_proc_cmdline_item(const char *key, const char *value, void *dat if (proc_cmdline_value_missing(key, value)) return 0; - if (free_and_strdup(&arg_root_what, value) < 0) - return log_oom(); + return free_and_strdup_warn(&arg_root_what, value); } else if (streq(key, "rootfstype")) { if (proc_cmdline_value_missing(key, value)) return 0; - if (free_and_strdup(&arg_root_fstype, value) < 0) - return log_oom(); + return free_and_strdup_warn(&arg_root_fstype, value); } else if (streq(key, "rootflags")) { @@ -851,24 +849,21 @@ static int parse_proc_cmdline_item(const char *key, const char *value, void *dat if (proc_cmdline_value_missing(key, value)) return 0; - if (free_and_strdup(&arg_root_hash, value) < 0) - return log_oom(); + return free_and_strdup_warn(&arg_root_hash, value); } else if (streq(key, "mount.usr")) { if (proc_cmdline_value_missing(key, value)) return 0; - if (free_and_strdup(&arg_usr_what, value) < 0) - return log_oom(); + return free_and_strdup_warn(&arg_usr_what, value); } else if (streq(key, "mount.usrfstype")) { if (proc_cmdline_value_missing(key, value)) return 0; - if (free_and_strdup(&arg_usr_fstype, value) < 0) - return log_oom(); + return free_and_strdup_warn(&arg_usr_fstype, value); } else if (streq(key, "mount.usrflags")) { diff --git a/src/mount/mount-tool.c b/src/mount/mount-tool.c index 39e67b57f8..8b6fc520c5 100644 --- a/src/mount/mount-tool.c +++ b/src/mount/mount-tool.c @@ -241,13 +241,15 @@ static int parse_argv(int argc, char *argv[]) { break; case 't': - if (free_and_strdup(&arg_mount_type, optarg) < 0) - return log_oom(); + r = free_and_strdup_warn(&arg_mount_type, optarg); + if (r < 0) + return r; break; case 'o': - if (free_and_strdup(&arg_mount_options, optarg) < 0) - return log_oom(); + r = free_and_strdup_warn(&arg_mount_options, optarg); + if (r < 0) + return r; break; case ARG_OWNER: { @@ -271,8 +273,9 @@ static int parse_argv(int argc, char *argv[]) { break; case ARG_DESCRIPTION: - if (free_and_strdup(&arg_description, optarg) < 0) - return log_oom(); + r = free_and_strdup_warn(&arg_description, optarg); + if (r < 0) + return r; break; case 'p': diff --git a/src/network/networkd-manager.c b/src/network/networkd-manager.c index 0e00cde87a..d7e0662453 100644 --- a/src/network/networkd-manager.c +++ b/src/network/networkd-manager.c @@ -1178,8 +1178,9 @@ int manager_set_hostname(Manager *m, const char *hostname) { log_debug("Setting transient hostname: '%s'", strna(hostname)); - if (free_and_strdup(&m->dynamic_hostname, hostname) < 0) - return log_oom(); + r = free_and_strdup_warn(&m->dynamic_hostname, hostname); + if (r < 0) + return r; if (!m->bus || sd_bus_is_ready(m->bus) <= 0) { log_debug("Not connected to system bus, setting hostname later."); @@ -1227,8 +1228,9 @@ int manager_set_timezone(Manager *m, const char *tz) { assert(tz); log_debug("Setting system timezone: '%s'", tz); - if (free_and_strdup(&m->dynamic_timezone, tz) < 0) - return log_oom(); + r = free_and_strdup_warn(&m->dynamic_timezone, tz); + if (r < 0) + return r; if (!m->bus || sd_bus_is_ready(m->bus) <= 0) { log_debug("Not connected to system bus, setting timezone later."); diff --git a/src/nspawn/nspawn-settings.c b/src/nspawn/nspawn-settings.c index b43d61468e..74a00c5275 100644 --- a/src/nspawn/nspawn-settings.c +++ b/src/nspawn/nspawn-settings.c @@ -706,10 +706,7 @@ int config_parse_hostname( return 0; } - if (free_and_strdup(s, empty_to_null(rvalue)) < 0) - return log_oom(); - - return 0; + return free_and_strdup_warn(s, empty_to_null(rvalue)); } int config_parse_oom_score_adjust( diff --git a/src/run-generator/run-generator.c b/src/run-generator/run-generator.c index 11e7d9e831..29b511f8e1 100644 --- a/src/run-generator/run-generator.c +++ b/src/run-generator/run-generator.c @@ -39,16 +39,14 @@ static int parse(const char *key, const char *value, void *data) { if (proc_cmdline_value_missing(key, value)) return 0; - if (free_and_strdup(&arg_success_action, value) < 0) - return log_oom(); + return free_and_strdup_warn(&arg_success_action, value); } else if (proc_cmdline_key_streq(key, "systemd.run_failure_action")) { if (proc_cmdline_value_missing(key, value)) return 0; - if (free_and_strdup(&arg_failure_action, value) < 0) - return log_oom(); + return free_and_strdup_warn(&arg_failure_action, value); } return 0; diff --git a/src/shared/conf-parser.c b/src/shared/conf-parser.c index 615782862b..c3d7e536ec 100644 --- a/src/shared/conf-parser.c +++ b/src/shared/conf-parser.c @@ -731,10 +731,7 @@ int config_parse_string( assert(rvalue); assert(data); - if (free_and_strdup(s, empty_to_null(rvalue)) < 0) - return log_oom(); - - return 0; + return free_and_strdup_warn(s, empty_to_null(rvalue)); } int config_parse_path( diff --git a/src/udev/net/link-config.c b/src/udev/net/link-config.c index 0653ae7a9c..9209f8f4b4 100644 --- a/src/udev/net/link-config.c +++ b/src/udev/net/link-config.c @@ -702,10 +702,7 @@ int config_parse_ifalias( return 0; } - if (free_and_strdup(s, rvalue) < 0) - return log_oom(); - - return 0; + return free_and_strdup_warn(s, rvalue); } int config_parse_rx_tx_queues( diff --git a/src/udev/udev-rules.c b/src/udev/udev-rules.c index 96a391c07c..fb4de17a5c 100644 --- a/src/udev/udev-rules.c +++ b/src/udev/udev-rules.c @@ -2055,8 +2055,9 @@ static int udev_rule_apply_token_to_event( token->value); break; } - if (free_and_strdup(&event->name, buf) < 0) - return log_oom(); + r = free_and_strdup_warn(&event->name, buf); + if (r < 0) + return r; log_rule_debug(dev, rules, "NAME '%s'", event->name); break;