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.

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);
+        Bond *b = ASSERT_PTR(userdata);
-        if (r < 0) {
-                log_syntax(unit, LOG_WARNING, filename, line, r,
-                           "Failed to parse actor system priority '%s', ignoring: %m", rvalue);
-                return 0;
-        }
 
-        if (v == 0) {
+        return config_parse_uint16_bounded(
-                log_syntax(unit, LOG_WARNING, filename, line, 0,
+                        unit, filename, line, section, section_line, lvalue, rvalue,
-                           "Failed to parse actor system priority '%s'. Range is [1,65535], ignoring.",
+                        1, UINT16_MAX, true,
-                           rvalue);
+                        &b->ad_actor_sys_prio);
-                return 0;
-        }
-
-        b->ad_actor_sys_prio = v;
-
-        return 0;
 }
 
 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);
+        Bond *b = ASSERT_PTR(userdata);
-        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) {
+        return config_parse_uint16_bounded(
-                log_syntax(unit, LOG_WARNING, filename, line, 0,
+                        unit, filename, line, section, section_line, lvalue, rvalue,
-                           "Failed to parse user port key '%s'. Range is [0…1023], ignoring.", rvalue);
+                        0, 1023, /* ignoring= */ true,
-                return 0;
+                        &b->ad_user_port_key);
-        }
-
-        b->ad_user_port_key = v;
-
-        return 0;
 }
 
 int config_parse_ad_actor_system(

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);
+        return config_parse_uint8_bounded(
-        if (r < 0) {
+                        unit, filename, line, section, section_line, lvalue, rvalue,
-                log_syntax(unit, LOG_WARNING, filename, line, r,
+                        2, 3, true,
-                           "Failed to parse bridge's multicast IGMP version number '%s', ignoring assignment: %m",
+                        &b->igmp_version);
-                           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;
 }
 
 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. */
+        uint16_t *prio = ASSERT_PTR(data);
 
-        r = safe_atou16(rvalue, &i);
+        return config_parse_uint16_bounded(
-        if (r < 0) {
+                        unit, filename, line, section, section_line, lvalue, rvalue,
-                log_syntax(unit, LOG_WARNING, filename, line, r,
+                        0, LINK_BRIDGE_PORT_PRIORITY_MAX, true,
-                           "Failed to parse bridge port priority, ignoring: %s", rvalue);
+                        prio);
-                return 0;
-        }
-
-        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;
 }
 
 static void bridge_init(NetDev *n) {

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);
+        Geneve *v = ASSERT_PTR(userdata);
-        if (r < 0) {
-                log_syntax(unit, LOG_WARNING, filename, line, r, "Failed to parse Geneve VNI '%s'.", rvalue);
-                return 0;
-        }
 
-        if (f > GENEVE_VID_MAX){
+        return config_parse_uint32_bounded(
-                log_syntax(unit, LOG_WARNING, filename, line, 0, "Geneve VNI out is of range '%s'.", rvalue);
+                        unit, filename, line, section, section_line, lvalue, rvalue,
-                return 0;
+                        0, GENEVE_VID_MAX, true,
-        }
+                        &v->id);
-
-        v->id = f;
-
-        return 0;
 }
 
 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 {
+                return 0;
-                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 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) {

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);
+        uint32_t *id = ASSERT_PTR(data);
-        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) {
+        return config_parse_uint32_bounded(
-                log_syntax(unit, LOG_WARNING, filename, line, 0,
+                        unit, filename, line, section, section_line, lvalue, rvalue,
-                           "Invalid L2TP tunnel id. Ignoring assignment: %s", rvalue);
+                        1, UINT32_MAX, true,
-                return 0;
+                        id);
-        }
-
-        *id = k;
-
-        return 0;
 }
 
 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);
+        uint32_t *id = streq(lvalue, "SessionId") ? &session->session_id : &session->peer_session_id;
-        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) {
+        r = config_parse_uint32_bounded(
-                log_syntax(unit, LOG_WARNING, filename, line, 0,
+                        unit, filename, line, section, section_line, lvalue, rvalue,
-                           "Invalid L2TP session id. Ignoring assignment: %s", rvalue);
+                        1, UINT32_MAX, true,
-                return 0;
+                        id);
-        }
+        if (r <= 0)
-
+                return r;
-        if (streq(lvalue, "SessionId"))
+        TAKE_PTR(session);
-                session->session_id = k;
-        else
-                session->peer_session_id = k;
-
-        session = NULL;
         return 0;
 }
 

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);
+        return config_parse_uint32_bounded(
-        if (r < 0) {
+                        unit, filename, line, section, section_line, lvalue, rvalue,
-                log_syntax(unit, LOG_WARNING, filename, line, r,
+                        0, UINT32_MAX - 1, true,
-                           "Failed to parse BroadcastMulticastQueueLength=%s, ignoring assignment: %m", rvalue);
+                        &m->bc_queue_length);
-                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;
 }
 
 static void macvlan_done(NetDev *n) {

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);
+        r = config_parse_uint8_bounded(
-        if (r < 0) {
+                        unit, filename, line, section, section_line, lvalue, rvalue,
-                log_syntax(unit, LOG_WARNING, filename, line, r,
+                        0, UINT8_MAX, true,
-                           "Failed to parse Tunnel Encapsulation Limit option, ignoring assignment: %s", rvalue);
+                        &t->encap_limit);
-                return 0;
+        if (r <= 0)
-        }
+                return r;
-
-        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;
         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);
+        return config_parse_uint8_bounded(
-        if (r < 0) {
+                        unit, filename, line, section, section_line, lvalue, rvalue,
-                log_syntax(unit, LOG_WARNING, filename, line, r,
+                        0, 2, true,
-                           "Failed to parse erspan version \"%s\", ignoring: %m", rvalue);
+                        v);
-                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;
 }
 
 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);
+        return config_parse_uint32_bounded(
-        if (r < 0) {
+                        unit, filename, line, section, section_line, lvalue, rvalue,
-                log_syntax(unit, LOG_WARNING, filename, line, r,
+                        0, 0x100000 - 1, true,
-                           "Failed to parse erspan index \"%s\", ignoring: %m", rvalue);
+                        v);
-                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;
 }
 
 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);
+        return config_parse_uint16_bounded(
-        if (r < 0) {
+                        unit, filename, line, section, section_line, lvalue, rvalue,
-                log_syntax(unit, LOG_WARNING, filename, line, r,
+                        0, 63, true,
-                           "Failed to parse erspan hwid \"%s\", ignoring: %m", rvalue);
+                        v);
-                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;
 }
 
 static void netdev_tunnel_init(NetDev *netdev) {

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");

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;
+}