network: use a common helper to parse bounded ranges

This compresses repetetive code and makes it easier to add new options
in networkd. The formatting of error messages becomes uniform. The
error message always specifies the rvalue literally, instead of using
a "descriptive name". This makes the message much easier to handle for
the user.

I opted to add just one parser, and wrap it with inline functions to proxy
the type. This is less verbose than copying functions for each type
separately, and the compiler should be able to get rid of the inline wrapper
almost entirely.

asserts are reordered to use the same order as the parameter list.
This makes the code easier to read.

No functional change intended, apart from the difference in error message
formatting.

diff --git a/src/network/netdev/bond.c b/src/network/netdev/bond.c
index 601bff0a9c0c19cc2fe9a407cf08e0e1832bd8fe..704a4c6b4c1cecee85e5fb08b9d4e2b38c9f184e 100644 (file)
--- a/src/network/netdev/bond.c
+++ b/src/network/netdev/bond.c
@@ -303,32 +303,18 @@ int config_parse_ad_actor_sys_prio(
                 const char *rvalue,
                 void *data,
                 void *userdata) {
-        Bond *b = userdata;
-        uint16_t v;
-        int r;
 
         assert(filename);
         assert(lvalue);
         assert(rvalue);
         assert(data);
 
-        r = safe_atou16(rvalue, &v);
-        if (r < 0) {
-                log_syntax(unit, LOG_WARNING, filename, line, r,
-                           "Failed to parse actor system priority '%s', ignoring: %m", rvalue);
-                return 0;
-        }
+        Bond *b = ASSERT_PTR(userdata);
 
-        if (v == 0) {
-                log_syntax(unit, LOG_WARNING, filename, line, 0,
-                           "Failed to parse actor system priority '%s'. Range is [1,65535], ignoring.",
-                           rvalue);
-                return 0;
-        }
-
-        b->ad_actor_sys_prio = v;
-
-        return 0;
+        return config_parse_uint16_bounded(
+                        unit, filename, line, section, section_line, lvalue, rvalue,
+                        1, UINT16_MAX, true,
+                        &b->ad_actor_sys_prio);
 }
 
 int config_parse_ad_user_port_key(
@@ -342,31 +328,18 @@ int config_parse_ad_user_port_key(
                 const char *rvalue,
                 void *data,
                 void *userdata) {
-        Bond *b = userdata;
-        uint16_t v;
-        int r;
 
         assert(filename);
         assert(lvalue);
         assert(rvalue);
         assert(data);
 
-        r = safe_atou16(rvalue, &v);
-        if (r < 0) {
-                log_syntax(unit, LOG_WARNING, filename, line, r,
-                           "Failed to parse user port key '%s', ignoring: %m", rvalue);
-                return 0;
-        }
-
-        if (v > 1023) {
-                log_syntax(unit, LOG_WARNING, filename, line, 0,
-                           "Failed to parse user port key '%s'. Range is [0…1023], ignoring.", rvalue);
-                return 0;
-        }
-
-        b->ad_user_port_key = v;
+        Bond *b = ASSERT_PTR(userdata);
 
-        return 0;
+        return config_parse_uint16_bounded(
+                        unit, filename, line, section, section_line, lvalue, rvalue,
+                        0, 1023, /* ignoring= */ true,
+                        &b->ad_user_port_key);
 }
 
 int config_parse_ad_actor_system(

diff --git a/src/network/netdev/bridge.c b/src/network/netdev/bridge.c
index b65c3b49fc22afcadd21d45da95d0f5eae3b3525..3783139d158feda2d36f8cc8ad5d506084434ded 100644 (file)
--- a/src/network/netdev/bridge.c
+++ b/src/network/netdev/bridge.c
@@ -189,36 +189,22 @@ int config_parse_bridge_igmp_version(
                 void *data,
                 void *userdata) {
 
-        Bridge *b = userdata;
-        uint8_t u;
-        int r;
-
         assert(filename);
         assert(lvalue);
         assert(rvalue);
         assert(data);
 
+        Bridge *b = ASSERT_PTR(userdata);
+
         if (isempty(rvalue)) {
                 b->igmp_version = 0; /* 0 means unset. */
                 return 0;
         }
 
-        r = safe_atou8(rvalue, &u);
-        if (r < 0) {
-                log_syntax(unit, LOG_WARNING, filename, line, r,
-                           "Failed to parse bridge's multicast IGMP version number '%s', ignoring assignment: %m",
-                           rvalue);
-                return 0;
-        }
-        if (!IN_SET(u, 2, 3)) {
-                log_syntax(unit, LOG_WARNING, filename, line, 0,
-                           "Invalid bridge's multicast IGMP version number '%s', ignoring assignment.", rvalue);
-                return 0;
-        }
-
-        b->igmp_version = u;
-
-        return 0;
+        return config_parse_uint8_bounded(
+                        unit, filename, line, section, section_line, lvalue, rvalue,
+                        2, 3, true,
+                        &b->igmp_version);
 }
 
 int config_parse_bridge_port_priority(
@@ -233,33 +219,16 @@ int config_parse_bridge_port_priority(
                 void *data,
                 void *userdata) {
 
-        uint16_t i;
-        int r;
-
         assert(filename);
         assert(lvalue);
         assert(rvalue);
-        assert(data);
-
-        /* This is used in networkd-network-gperf.gperf. */
 
-        r = safe_atou16(rvalue, &i);
-        if (r < 0) {
-                log_syntax(unit, LOG_WARNING, filename, line, r,
-                           "Failed to parse bridge port priority, ignoring: %s", rvalue);
-                return 0;
-        }
+        uint16_t *prio = ASSERT_PTR(data);
 
-        if (i > LINK_BRIDGE_PORT_PRIORITY_MAX) {
-                log_syntax(unit, LOG_WARNING, filename, line, 0,
-                           "Bridge port priority is larger than maximum %u, ignoring: %s",
-                           LINK_BRIDGE_PORT_PRIORITY_MAX, rvalue);
-                return 0;
-        }
-
-        *((uint16_t *)data) = i;
-
-        return 0;
+        return config_parse_uint16_bounded(
+                        unit, filename, line, section, section_line, lvalue, rvalue,
+                        0, LINK_BRIDGE_PORT_PRIORITY_MAX, true,
+                        prio);
 }
 
 static void bridge_init(NetDev *n) {

diff --git a/src/network/netdev/geneve.c b/src/network/netdev/geneve.c
index 7d5a8679d1422b8a79b9903b8197e802afafc832..83c73aff0773000646645c45c8ff7cd5f11b923d 100644 (file)
--- a/src/network/netdev/geneve.c
+++ b/src/network/netdev/geneve.c
@@ -116,29 +116,17 @@ int config_parse_geneve_vni(
                 void *data,
                 void *userdata) {
 
-        Geneve *v = userdata;
-        uint32_t f;
-        int r;
-
         assert(filename);
         assert(lvalue);
         assert(rvalue);
         assert(data);
 
-        r = safe_atou32(rvalue, &f);
-        if (r < 0) {
-                log_syntax(unit, LOG_WARNING, filename, line, r, "Failed to parse Geneve VNI '%s'.", rvalue);
-                return 0;
-        }
+        Geneve *v = ASSERT_PTR(userdata);
 
-        if (f > GENEVE_VID_MAX){
-                log_syntax(unit, LOG_WARNING, filename, line, 0, "Geneve VNI out is of range '%s'.", rvalue);
-                return 0;
-        }
-
-        v->id = f;
-
-        return 0;
+        return config_parse_uint32_bounded(
+                        unit, filename, line, section, section_line, lvalue, rvalue,
+                        0, GENEVE_VID_MAX, true,
+                        &v->id);
 }
 
 int config_parse_geneve_address(
@@ -230,35 +218,22 @@ int config_parse_geneve_ttl(
                 void *data,
                 void *userdata) {
 
-        Geneve *v = userdata;
-        unsigned f;
-        int r;
-
         assert(filename);
         assert(lvalue);
         assert(rvalue);
         assert(data);
 
-        if (streq(rvalue, "inherit"))
+        Geneve *v = ASSERT_PTR(userdata);
+
+        if (streq(rvalue, "inherit")) {
                 v->inherit = true;
-        else {
-                r = safe_atou(rvalue, &f);
-                if (r < 0) {
-                        log_syntax(unit, LOG_WARNING, filename, line, r,
-                                   "Failed to parse Geneve TTL '%s', ignoring assignment: %m", rvalue);
-                        return 0;
-                }
-
-                if (f > 255) {
-                        log_syntax(unit, LOG_WARNING, filename, line, 0,
-                                   "Invalid Geneve TTL '%s'. TTL must be <= 255. Ignoring assignment.", rvalue);
-                        return 0;
-                }
-
-                v->ttl = f;
+                return 0;
         }
 
-        return 0;
+        return config_parse_uint8_bounded(
+                        unit, filename, line, section, section_line, lvalue, rvalue,
+                        0, UINT8_MAX, true,
+                        &v->ttl);
 }
 
 static int netdev_geneve_verify(NetDev *netdev, const char *filename) {

diff --git a/src/network/netdev/l2tp-tunnel.c b/src/network/netdev/l2tp-tunnel.c
index 547de2418c498d8ec5b5bbf74cf613bb9b83c08e..17d5391cd38698fc35ebf8477614082dd3236709 100644 (file)
--- a/src/network/netdev/l2tp-tunnel.c
+++ b/src/network/netdev/l2tp-tunnel.c
@@ -626,30 +626,16 @@ int config_parse_l2tp_tunnel_id(
                 void *data,
                 void *userdata) {
 
-        uint32_t *id = data, k;
-        int r;
-
         assert(filename);
         assert(lvalue);
         assert(rvalue);
-        assert(data);
-
-        r = safe_atou32(rvalue, &k);
-        if (r < 0) {
-                log_syntax(unit, LOG_WARNING, filename, line, r,
-                           "Failed to parse L2TP tunnel id. Ignoring assignment: %s", rvalue);
-                return 0;
-        }
 
-        if (k == 0) {
-                log_syntax(unit, LOG_WARNING, filename, line, 0,
-                           "Invalid L2TP tunnel id. Ignoring assignment: %s", rvalue);
-                return 0;
-        }
-
-        *id = k;
+        uint32_t *id = ASSERT_PTR(data);
 
-        return 0;
+        return config_parse_uint32_bounded(
+                        unit, filename, line, section, section_line, lvalue, rvalue,
+                        1, UINT32_MAX, true,
+                        id);
 }
 
 int config_parse_l2tp_session_id(
@@ -664,40 +650,29 @@ int config_parse_l2tp_session_id(
                 void *data,
                 void *userdata) {
 
-        _cleanup_(l2tp_session_free_or_set_invalidp) L2tpSession *session = NULL;
-        L2tpTunnel *t = userdata;
-        uint32_t k;
-        int r;
-
         assert(filename);
         assert(section);
         assert(lvalue);
         assert(rvalue);
         assert(data);
 
+        L2tpTunnel *t = ASSERT_PTR(userdata);
+        _cleanup_(l2tp_session_free_or_set_invalidp) L2tpSession *session = NULL;
+        int r;
+
         r = l2tp_session_new_static(t, filename, section_line, &session);
         if (r < 0)
                 return log_oom();
 
-        r = safe_atou32(rvalue, &k);
-        if (r < 0) {
-                log_syntax(unit, LOG_WARNING, filename, line, r,
-                           "Failed to parse L2TP session id. Ignoring assignment: %s", rvalue);
-                return 0;
-        }
-
-        if (k == 0) {
-                log_syntax(unit, LOG_WARNING, filename, line, 0,
-                           "Invalid L2TP session id. Ignoring assignment: %s", rvalue);
-                return 0;
-        }
-
-        if (streq(lvalue, "SessionId"))
-                session->session_id = k;
-        else
-                session->peer_session_id = k;
+        uint32_t *id = streq(lvalue, "SessionId") ? &session->session_id : &session->peer_session_id;
 
-        session = NULL;
+        r = config_parse_uint32_bounded(
+                        unit, filename, line, section, section_line, lvalue, rvalue,
+                        1, UINT32_MAX, true,
+                        id);
+        if (r <= 0)
+                return r;
+        TAKE_PTR(session);
         return 0;
 }
 

diff --git a/src/network/netdev/macvlan.c b/src/network/netdev/macvlan.c
index 1114bb0cb1b5fbd95cab1c95e3e18b1112d9d1c5..6083cd2b44e6e73385017998a287f52227419a4a 100644 (file)
--- a/src/network/netdev/macvlan.c
+++ b/src/network/netdev/macvlan.c
@@ -84,36 +84,23 @@ int config_parse_macvlan_broadcast_queue_size(
                 void *data,
                 void *userdata) {
 
-        MacVlan *m = ASSERT_PTR(userdata);
-        uint32_t v;
-        int r;
-
         assert(filename);
         assert(section);
         assert(lvalue);
         assert(rvalue);
         assert(data);
 
+        MacVlan *m = ASSERT_PTR(userdata);
+
         if (isempty(rvalue)) {
                 m->bc_queue_length = UINT32_MAX;
                 return 0;
         }
 
-        r = safe_atou32(rvalue, &v);
-        if (r < 0) {
-                log_syntax(unit, LOG_WARNING, filename, line, r,
-                           "Failed to parse BroadcastMulticastQueueLength=%s, ignoring assignment: %m", rvalue);
-                return 0;
-        }
-
-        if (v == UINT32_MAX) {
-                log_syntax(unit, LOG_WARNING, filename, line, 0,
-                           "Invalid BroadcastMulticastQueueLength=%s, ignoring assignment: %m", rvalue);
-                return 0;
-        }
-
-        m->bc_queue_length = v;
-        return 0;
+        return config_parse_uint32_bounded(
+                        unit, filename, line, section, section_line, lvalue, rvalue,
+                        0, UINT32_MAX - 1, true,
+                        &m->bc_queue_length);
 }
 
 static void macvlan_done(NetDev *n) {

diff --git a/src/network/netdev/tunnel.c b/src/network/netdev/tunnel.c
index 881be3c405b9e45622c616076ecf8318f2fa2656..ced04c77ecaec8e3f6f670a59328060556614891 100644 (file)
--- a/src/network/netdev/tunnel.c
+++ b/src/network/netdev/tunnel.c
@@ -972,33 +972,26 @@ int config_parse_encap_limit(
                 void *data,
                 void *userdata) {
 
-        Tunnel *t = ASSERT_PTR(userdata);
-        int k, r;
-
         assert(filename);
         assert(rvalue);
 
+        Tunnel *t = ASSERT_PTR(userdata);
+        int r;
+
         if (streq(rvalue, "none")) {
-                t->flags |= IP6_TNL_F_IGN_ENCAP_LIMIT;
                 t->encap_limit = 0;
+                t->flags |= IP6_TNL_F_IGN_ENCAP_LIMIT;
                 return 0;
         }
 
-        r = safe_atoi(rvalue, &k);
-        if (r < 0) {
-                log_syntax(unit, LOG_WARNING, filename, line, r,
-                           "Failed to parse Tunnel Encapsulation Limit option, ignoring assignment: %s", rvalue);
-                return 0;
-        }
-
-        if (k > 255 || k < 0) {
-                log_syntax(unit, LOG_WARNING, filename, line, 0,
-                           "Invalid Tunnel Encapsulation value, ignoring assignment: %d", k);
-                return 0;
-        }
-
-        t->encap_limit = k;
+        r = config_parse_uint8_bounded(
+                        unit, filename, line, section, section_line, lvalue, rvalue,
+                        0, UINT8_MAX, true,
+                        &t->encap_limit);
+        if (r <= 0)
+                return r;
         t->flags &= ~IP6_TNL_F_IGN_ENCAP_LIMIT;
+
         return 0;
 }
 
@@ -1051,32 +1044,21 @@ int config_parse_erspan_version(
                 void *data,
                 void *userdata) {
 
-        uint8_t n, *v = ASSERT_PTR(data);
-        int r;
-
         assert(filename);
         assert(lvalue);
         assert(rvalue);
 
+        uint8_t *v = ASSERT_PTR(data);
+
         if (isempty(rvalue)) {
                 *v = 1; /* defaults to 1 */
                 return 0;
         }
 
-        r = safe_atou8(rvalue, &n);
-        if (r < 0) {
-                log_syntax(unit, LOG_WARNING, filename, line, r,
-                           "Failed to parse erspan version \"%s\", ignoring: %m", rvalue);
-                return 0;
-        }
-        if (!IN_SET(n, 0, 1, 2)) {
-                log_syntax(unit, LOG_WARNING, filename, line, 0,
-                           "Invalid erspan version \"%s\", which must be 0, 1 or 2, ignoring.", rvalue);
-                return 0;
-        }
-
-        *v = n;
-        return 0;
+        return config_parse_uint8_bounded(
+                        unit, filename, line, section, section_line, lvalue, rvalue,
+                        0, 2, true,
+                        v);
 }
 
 int config_parse_erspan_index(
@@ -1091,32 +1073,21 @@ int config_parse_erspan_index(
                 void *data,
                 void *userdata) {
 
-        uint32_t n, *v = ASSERT_PTR(data);
-        int r;
-
         assert(filename);
         assert(lvalue);
         assert(rvalue);
 
+        uint32_t *v = ASSERT_PTR(data);
+
         if (isempty(rvalue)) {
                 *v = 0; /* defaults to 0 */
                 return 0;
         }
 
-        r = safe_atou32(rvalue, &n);
-        if (r < 0) {
-                log_syntax(unit, LOG_WARNING, filename, line, r,
-                           "Failed to parse erspan index \"%s\", ignoring: %m", rvalue);
-                return 0;
-        }
-        if (n >= 0x100000) {
-                log_syntax(unit, LOG_WARNING, filename, line, 0,
-                           "Invalid erspan index \"%s\", which must be less than 0x100000, ignoring.", rvalue);
-                return 0;
-        }
-
-        *v = n;
-        return 0;
+        return config_parse_uint32_bounded(
+                        unit, filename, line, section, section_line, lvalue, rvalue,
+                        0, 0x100000 - 1, true,
+                        v);
 }
 
 int config_parse_erspan_direction(
@@ -1131,12 +1102,12 @@ int config_parse_erspan_direction(
                 void *data,
                 void *userdata) {
 
-        uint8_t *v = ASSERT_PTR(data);
-
         assert(filename);
         assert(lvalue);
         assert(rvalue);
 
+        uint8_t *v = ASSERT_PTR(data);
+
         if (isempty(rvalue) || streq(rvalue, "ingress"))
                 *v = 0; /* defaults to ingress */
         else if (streq(rvalue, "egress"))
@@ -1160,32 +1131,21 @@ int config_parse_erspan_hwid(
                 void *data,
                 void *userdata) {
 
-        uint16_t n, *v = ASSERT_PTR(data);
-        int r;
-
         assert(filename);
         assert(lvalue);
         assert(rvalue);
 
+        uint16_t *v = ASSERT_PTR(data);
+
         if (isempty(rvalue)) {
                 *v = 0; /* defaults to 0 */
                 return 0;
         }
 
-        r = safe_atou16(rvalue, &n);
-        if (r < 0) {
-                log_syntax(unit, LOG_WARNING, filename, line, r,
-                           "Failed to parse erspan hwid \"%s\", ignoring: %m", rvalue);
-                return 0;
-        }
-        if (n >= 64) {
-                log_syntax(unit, LOG_WARNING, filename, line, 0,
-                           "Invalid erspan index \"%s\", which must be less than 64, ignoring.", rvalue);
-                return 0;
-        }
-
-        *v = n;
-        return 0;
+        return config_parse_uint16_bounded(
+                        unit, filename, line, section, section_line, lvalue, rvalue,
+                        0, 63, true,
+                        v);
 }
 
 static void netdev_tunnel_init(NetDev *netdev) {

diff --git a/src/shared/conf-parser.c b/src/shared/conf-parser.c
index af5aac7b291c966e3fa191477b1a6502e72f8a57..a60588ad9ee74b704dc98e79fa41b0696118b1f6 100644 (file)
--- a/src/shared/conf-parser.c
+++ b/src/shared/conf-parser.c
@@ -1908,6 +1908,44 @@ int config_parse_in_addr_non_null(
         return 0;
 }
 
+int config_parse_unsigned_bounded(
+                const char *unit,
+                const char *filename,
+                unsigned line,
+                const char *section,
+                unsigned section_line,
+                const char *name,
+                const char *value,
+                unsigned min,
+                unsigned max,
+                bool ignoring,
+                unsigned *ret) {
+
+        int r;
+
+        assert(filename);
+        assert(name);
+        assert(value);
+        assert(ret);
+
+        r = safe_atou_bounded(value, min, max, ret);
+        if (r == -ERANGE)
+                log_syntax(unit, LOG_WARNING, filename, line, r,
+                           "Invalid '%s=%s', allowed range is %u..%u%s.",
+                           name, value, min, max, ignoring ? ", ignoring" : "");
+        else if (r < 0)
+                log_syntax(unit, LOG_WARNING, filename, line, r,
+                           "Failed to parse '%s=%s'%s: %m",
+                           name, value, ignoring ? ", ignoring" : "");
+
+        if (r >= 0)
+                return 1;  /* Return 1 if something was set */
+        else if (ignoring)
+                return 0;
+        else
+                return r;
+}
+
 DEFINE_CONFIG_PARSE(config_parse_percent, parse_percent, "Failed to parse percent value");
 DEFINE_CONFIG_PARSE(config_parse_permyriad, parse_permyriad, "Failed to parse permyriad value");
 DEFINE_CONFIG_PARSE_PTR(config_parse_sec_fix_0, parse_sec_fix_0, usec_t, "Failed to parse time value");

diff --git a/src/shared/conf-parser.h b/src/shared/conf-parser.h
index 140883969cc19ab3dd89b33421783f3e66906a61..f1b03a8d152b5f1bcda7b9600602e5266b216215 100644 (file)
--- a/src/shared/conf-parser.h
+++ b/src/shared/conf-parser.h
@@ -385,3 +385,97 @@ typedef enum ConfigParseStringFlags {
                                                                                \
                 return free_and_replace(*enums, xs);                           \
         }
+
+int config_parse_unsigned_bounded(
+                const char *unit,
+                const char *filename,
+                unsigned line,
+                const char *section,
+                unsigned section_line,
+                const char *name,
+                const char *value,
+                unsigned min,
+                unsigned max,
+                bool ignoring,
+                unsigned *ret);
+
+static inline int config_parse_uint32_bounded(
+                const char *unit,
+                const char *filename,
+                unsigned line,
+                const char *section,
+                unsigned section_line,
+                const char *name,
+                const char *value,
+                uint32_t min,
+                uint32_t max,
+                bool ignoring,
+                uint32_t *ret) {
+
+        unsigned t;
+        int r;
+
+        r = config_parse_unsigned_bounded(
+                        unit, filename, line, section, section_line, name, value,
+                        min, max, ignoring,
+                        &t);
+        if (r <= 0)
+                return r;
+        assert(t <= UINT32_MAX);
+        *ret = t;
+        return 1;
+}
+
+static inline int config_parse_uint16_bounded(
+                const char *unit,
+                const char *filename,
+                unsigned line,
+                const char *section,
+                unsigned section_line,
+                const char *name,
+                const char *value,
+                uint16_t min,
+                uint16_t max,
+                bool ignoring,
+                uint16_t *ret) {
+
+        unsigned t;
+        int r;
+
+        r = config_parse_unsigned_bounded(
+                        unit, filename, line, section, section_line, name, value,
+                        min, max, ignoring,
+                        &t);
+        if (r <= 0)
+                return r;
+        assert(t <= UINT16_MAX);
+        *ret = t;
+        return 1;
+}
+
+static inline int config_parse_uint8_bounded(
+                const char *unit,
+                const char *filename,
+                unsigned line,
+                const char *section,
+                unsigned section_line,
+                const char *name,
+                const char *value,
+                uint8_t min,
+                uint8_t max,
+                bool ignoring,
+                uint8_t *ret) {
+
+        unsigned t;
+        int r;
+
+        r = config_parse_unsigned_bounded(
+                        unit, filename, line, section, section_line, name, value,
+                        min, max, ignoring,
+                        &t);
+        if (r <= 0)
+                return r;
+        assert(t <= UINT8_MAX);
+        *ret = t;
+        return 1;
+}