[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <8ca3747c14bacccf87408280663c0598d0dc824e.1700061513.git.petrm@nvidia.com>
Date: Wed, 15 Nov 2023 16:31:59 +0100
From: Petr Machata <petrm@...dia.com>
To: David Ahern <dsahern@...il.com>, Stephen Hemminger
<stephen@...workplumber.org>, <netdev@...r.kernel.org>
CC: Patrisious Haddad <phaddad@...dia.com>, Petr Machata <petrm@...dia.com>
Subject: [PATCH iproute2-next 3/3] lib: utils: Add parse_one_of_deprecated(), parse_on_off_deprecated()
The functions parse_on_off() and parse_one_of() currently use matches() for
string comparison under the hood. This has some odd consequences. In
particular, "o" can be used as a shorthand for "off", which is not obvious,
because "o" is the prefix of both. By sheer luck, the end result actually
makes some sense: "on" means on, anything else means off (or errors out).
Similar issues are in principle also possible for parse_one_of() uses,
though currently this does not come up.
Ideally parse_on_off() would accept the strings "on" and "off" and no
others, likewise for parse_one_of().
Therefore in this patch, rename the old matches()-based parsers to
parse_one_of_deprecated() and parse_on_off_deprecated(). Introduce
new strcmp()-based parsers, parse_one_of() and parse_on_off(), for
future users.
All existing users have been converted to the _deprecated() variants,
with one exception for RDMA's sys_set_privileged_qkey_args(), which was
introduced recently enough that it is unlikely to have users relying
on the broken behavior.
Signed-off-by: Petr Machata <petrm@...dia.com>
---
bridge/link.c | 48 ++++++++++++++++++++++++++--------------
bridge/vlan.c | 4 ++--
dcb/dcb_ets.c | 6 +++--
dcb/dcb_pfc.c | 5 +++--
include/utils.h | 4 ++++
ip/iplink.c | 15 ++++++++-----
ip/iplink_bridge_slave.c | 2 +-
ip/ipmacsec.c | 27 +++++++++++++---------
ip/ipstats.c | 3 ++-
lib/utils.c | 11 +++++++++
10 files changed, 84 insertions(+), 41 deletions(-)
diff --git a/bridge/link.c b/bridge/link.c
index 1c8faa85..2a83c914 100644
--- a/bridge/link.c
+++ b/bridge/link.c
@@ -374,37 +374,44 @@ static int brlink_modify(int argc, char **argv)
d = *argv;
} else if (strcmp(*argv, "guard") == 0) {
NEXT_ARG();
- bpdu_guard = parse_on_off("guard", *argv, &ret);
+ bpdu_guard = parse_on_off_deprecated("guard",
+ *argv, &ret);
if (ret)
return ret;
} else if (strcmp(*argv, "hairpin") == 0) {
NEXT_ARG();
- hairpin = parse_on_off("hairpin", *argv, &ret);
+ hairpin = parse_on_off_deprecated("hairpin",
+ *argv, &ret);
if (ret)
return ret;
} else if (strcmp(*argv, "fastleave") == 0) {
NEXT_ARG();
- fast_leave = parse_on_off("fastleave", *argv, &ret);
+ fast_leave = parse_on_off_deprecated("fastleave",
+ *argv, &ret);
if (ret)
return ret;
} else if (strcmp(*argv, "root_block") == 0) {
NEXT_ARG();
- root_block = parse_on_off("root_block", *argv, &ret);
+ root_block = parse_on_off_deprecated("root_block",
+ *argv, &ret);
if (ret)
return ret;
} else if (strcmp(*argv, "learning") == 0) {
NEXT_ARG();
- learning = parse_on_off("learning", *argv, &ret);
+ learning = parse_on_off_deprecated("learning",
+ *argv, &ret);
if (ret)
return ret;
} else if (strcmp(*argv, "learning_sync") == 0) {
NEXT_ARG();
- learning_sync = parse_on_off("learning_sync", *argv, &ret);
+ learning_sync = parse_on_off_deprecated("learning_sync",
+ *argv, &ret);
if (ret)
return ret;
} else if (strcmp(*argv, "flood") == 0) {
NEXT_ARG();
- flood = parse_on_off("flood", *argv, &ret);
+ flood = parse_on_off_deprecated("flood",
+ *argv, &ret);
if (ret)
return ret;
} else if (strcmp(*argv, "mcast_router") == 0) {
@@ -412,17 +419,20 @@ static int brlink_modify(int argc, char **argv)
mcast_router = atoi(*argv);
} else if (strcmp(*argv, "mcast_flood") == 0) {
NEXT_ARG();
- mcast_flood = parse_on_off("mcast_flood", *argv, &ret);
+ mcast_flood = parse_on_off_deprecated("mcast_flood",
+ *argv, &ret);
if (ret)
return ret;
} else if (strcmp(*argv, "bcast_flood") == 0) {
NEXT_ARG();
- bcast_flood = parse_on_off("bcast_flood", *argv, &ret);
+ bcast_flood = parse_on_off_deprecated("bcast_flood",
+ *argv, &ret);
if (ret)
return ret;
} else if (strcmp(*argv, "mcast_to_unicast") == 0) {
NEXT_ARG();
- mcast_to_unicast = parse_on_off("mcast_to_unicast", *argv, &ret);
+ mcast_to_unicast = parse_on_off_deprecated("mcast_to_unicast",
+ *argv, &ret);
if (ret)
return ret;
} else if (strcmp(*argv, "mcast_max_groups") == 0) {
@@ -466,33 +476,37 @@ static int brlink_modify(int argc, char **argv)
flags |= BRIDGE_FLAGS_MASTER;
} else if (strcmp(*argv, "neigh_suppress") == 0) {
NEXT_ARG();
- neigh_suppress = parse_on_off("neigh_suppress", *argv, &ret);
+ neigh_suppress = parse_on_off_deprecated("neigh_suppress",
+ *argv, &ret);
if (ret)
return ret;
} else if (strcmp(*argv, "neigh_vlan_suppress") == 0) {
NEXT_ARG();
- neigh_vlan_suppress = parse_on_off("neigh_vlan_suppress",
- *argv, &ret);
+ neigh_vlan_suppress =
+ parse_on_off_deprecated("neigh_vlan_suppress",
+ *argv, &ret);
if (ret)
return ret;
} else if (strcmp(*argv, "vlan_tunnel") == 0) {
NEXT_ARG();
- vlan_tunnel = parse_on_off("vlan_tunnel", *argv, &ret);
+ vlan_tunnel = parse_on_off_deprecated("vlan_tunnel",
+ *argv, &ret);
if (ret)
return ret;
} else if (strcmp(*argv, "isolated") == 0) {
NEXT_ARG();
- isolated = parse_on_off("isolated", *argv, &ret);
+ isolated = parse_on_off_deprecated("isolated",
+ *argv, &ret);
if (ret)
return ret;
} else if (strcmp(*argv, "locked") == 0) {
NEXT_ARG();
- locked = parse_on_off("locked", *argv, &ret);
+ locked = parse_on_off_deprecated("locked", *argv, &ret);
if (ret)
return ret;
} else if (strcmp(*argv, "mab") == 0) {
NEXT_ARG();
- macauth = parse_on_off("mab", *argv, &ret);
+ macauth = parse_on_off_deprecated("mab", *argv, &ret);
if (ret)
return ret;
} else if (strcmp(*argv, "backup_port") == 0) {
diff --git a/bridge/vlan.c b/bridge/vlan.c
index dfc62f83..338b9b0c 100644
--- a/bridge/vlan.c
+++ b/bridge/vlan.c
@@ -361,8 +361,8 @@ static int vlan_option_set(int argc, char **argv)
int ret;
NEXT_ARG();
- neigh_suppress = parse_on_off("neigh_suppress", *argv,
- &ret);
+ neigh_suppress = parse_on_off_deprecated("neigh_suppress",
+ *argv, &ret);
if (ret)
return ret;
addattr8(&req.n, sizeof(req),
diff --git a/dcb/dcb_ets.c b/dcb/dcb_ets.c
index c2088105..ea8786ff 100644
--- a/dcb/dcb_ets.c
+++ b/dcb/dcb_ets.c
@@ -61,7 +61,8 @@ static int dcb_ets_parse_mapping_tc_tsa(__u32 key, char *value, void *data)
__u8 tsa;
int ret;
- tsa = parse_one_of("TSA", value, tsa_names, ARRAY_SIZE(tsa_names), &ret);
+ tsa = parse_one_of_deprecated("TSA", value, tsa_names,
+ ARRAY_SIZE(tsa_names), &ret);
if (ret)
return ret;
@@ -274,7 +275,8 @@ static int dcb_cmd_ets_set(struct dcb *dcb, const char *dev, int argc, char **ar
return 0;
} else if (matches(*argv, "willing") == 0) {
NEXT_ARG();
- ets.willing = parse_on_off("willing", *argv, &ret);
+ ets.willing = parse_on_off_deprecated("willing",
+ *argv, &ret);
if (ret)
return ret;
} else if (matches(*argv, "tc-tsa") == 0) {
diff --git a/dcb/dcb_pfc.c b/dcb/dcb_pfc.c
index aaa09022..a89c274f 100644
--- a/dcb/dcb_pfc.c
+++ b/dcb/dcb_pfc.c
@@ -73,7 +73,7 @@ static int dcb_pfc_parse_mapping_prio_pfc(__u32 key, char *value, void *data)
dcb_pfc_to_array(pfc_en, pfc->pfc_en);
- enabled = parse_on_off("PFC", value, &ret);
+ enabled = parse_on_off_deprecated("PFC", value, &ret);
if (ret)
return ret;
@@ -185,7 +185,8 @@ static int dcb_cmd_pfc_set(struct dcb *dcb, const char *dev, int argc, char **ar
continue;
} else if (matches(*argv, "macsec-bypass") == 0) {
NEXT_ARG();
- pfc.mbc = parse_on_off("macsec-bypass", *argv, &ret);
+ pfc.mbc = parse_on_off_deprecated("macsec-bypass",
+ *argv, &ret);
if (ret)
return ret;
} else if (matches(*argv, "delay") == 0) {
diff --git a/include/utils.h b/include/utils.h
index add55bfa..c9318c20 100644
--- a/include/utils.h
+++ b/include/utils.h
@@ -342,7 +342,11 @@ int do_batch(const char *name, bool force,
int parse_one_of(const char *msg, const char *realval, const char * const *list,
size_t len, int *p_err);
+int parse_one_of_deprecated(const char *msg, const char *realval,
+ const char * const *list, size_t len, int *p_err);
+
bool parse_on_off(const char *msg, const char *realval, int *p_err);
+bool parse_on_off_deprecated(const char *msg, const char *realval, int *p_err);
int parse_mapping_num_all(__u32 *keyp, const char *key);
int parse_mapping_gen(int *argcp, char ***argvp,
diff --git a/ip/iplink.c b/ip/iplink.c
index 9a548dd3..e868bc66 100644
--- a/ip/iplink.c
+++ b/ip/iplink.c
@@ -470,7 +470,8 @@ static int iplink_parse_vf(int vf, int *argcp, char ***argvp,
struct ifla_vf_spoofchk ivs;
NEXT_ARG();
- ivs.setting = parse_on_off("spoofchk", *argv, &ret);
+ ivs.setting = parse_on_off_deprecated("spoofchk",
+ *argv, &ret);
if (ret)
return ret;
ivs.vf = vf;
@@ -481,7 +482,8 @@ static int iplink_parse_vf(int vf, int *argcp, char ***argvp,
struct ifla_vf_rss_query_en ivs;
NEXT_ARG();
- ivs.setting = parse_on_off("query_rss", *argv, &ret);
+ ivs.setting = parse_on_off_deprecated("query_rss",
+ *argv, &ret);
if (ret)
return ret;
ivs.vf = vf;
@@ -492,7 +494,8 @@ static int iplink_parse_vf(int vf, int *argcp, char ***argvp,
struct ifla_vf_trust ivt;
NEXT_ARG();
- ivt.setting = parse_on_off("trust", *argv, &ret);
+ ivt.setting = parse_on_off_deprecated("trust",
+ *argv, &ret);
if (ret)
return ret;
ivt.vf = vf;
@@ -738,7 +741,8 @@ int iplink_parse(int argc, char **argv, struct iplink_req *req, char **type)
int carrier;
NEXT_ARG();
- carrier = parse_on_off("carrier", *argv, &err);
+ carrier = parse_on_off_deprecated("carrier",
+ *argv, &err);
if (err)
return err;
@@ -893,7 +897,8 @@ int iplink_parse(int argc, char **argv, struct iplink_req *req, char **type)
unsigned int proto_down;
NEXT_ARG();
- proto_down = parse_on_off("protodown", *argv, &err);
+ proto_down = parse_on_off_deprecated("protodown",
+ *argv, &err);
if (err)
return err;
addattr8(&req->n, sizeof(*req), IFLA_PROTO_DOWN,
diff --git a/ip/iplink_bridge_slave.c b/ip/iplink_bridge_slave.c
index 3821923b..361d5880 100644
--- a/ip/iplink_bridge_slave.c
+++ b/ip/iplink_bridge_slave.c
@@ -319,7 +319,7 @@ static void bridge_slave_parse_on_off(char *arg_name, char *arg_val,
struct nlmsghdr *n, int type)
{
int ret;
- __u8 val = parse_on_off(arg_name, arg_val, &ret);
+ __u8 val = parse_on_off_deprecated(arg_name, arg_val, &ret);
if (ret)
exit(1);
diff --git a/ip/ipmacsec.c b/ip/ipmacsec.c
index 476a6d1d..e958a37b 100644
--- a/ip/ipmacsec.c
+++ b/ip/ipmacsec.c
@@ -593,7 +593,8 @@ static int do_offload(enum cmd c, int argc, char **argv)
if (argc == 0)
ipmacsec_usage();
- offload = parse_one_of("offload", *argv, offload_str, ARRAY_SIZE(offload_str), &ret);
+ offload = parse_one_of_deprecated("offload", *argv, offload_str,
+ ARRAY_SIZE(offload_str), &ret);
if (ret)
ipmacsec_usage();
@@ -1402,7 +1403,7 @@ static int macsec_parse_opt(struct link_util *lu, int argc, char **argv,
NEXT_ARG();
int i;
- i = parse_on_off("encrypt", *argv, &ret);
+ i = parse_on_off_deprecated("encrypt", *argv, &ret);
if (ret != 0)
return ret;
addattr8(n, MACSEC_BUFLEN, IFLA_MACSEC_ENCRYPT, i);
@@ -1410,7 +1411,7 @@ static int macsec_parse_opt(struct link_util *lu, int argc, char **argv,
NEXT_ARG();
int i;
- i = parse_on_off("send_sci", *argv, &ret);
+ i = parse_on_off_deprecated("send_sci", *argv, &ret);
if (ret != 0)
return ret;
send_sci = i;
@@ -1420,7 +1421,7 @@ static int macsec_parse_opt(struct link_util *lu, int argc, char **argv,
NEXT_ARG();
int i;
- i = parse_on_off("end_station", *argv, &ret);
+ i = parse_on_off_deprecated("end_station", *argv, &ret);
if (ret != 0)
return ret;
es = i;
@@ -1429,7 +1430,7 @@ static int macsec_parse_opt(struct link_util *lu, int argc, char **argv,
NEXT_ARG();
int i;
- i = parse_on_off("scb", *argv, &ret);
+ i = parse_on_off_deprecated("scb", *argv, &ret);
if (ret != 0)
return ret;
scb = i;
@@ -1438,7 +1439,7 @@ static int macsec_parse_opt(struct link_util *lu, int argc, char **argv,
NEXT_ARG();
int i;
- i = parse_on_off("protect", *argv, &ret);
+ i = parse_on_off_deprecated("protect", *argv, &ret);
if (ret != 0)
return ret;
addattr8(n, MACSEC_BUFLEN, IFLA_MACSEC_PROTECT, i);
@@ -1446,7 +1447,7 @@ static int macsec_parse_opt(struct link_util *lu, int argc, char **argv,
NEXT_ARG();
int i;
- i = parse_on_off("replay", *argv, &ret);
+ i = parse_on_off_deprecated("replay", *argv, &ret);
if (ret != 0)
return ret;
replay_protect = !!i;
@@ -1457,8 +1458,10 @@ static int macsec_parse_opt(struct link_util *lu, int argc, char **argv,
invarg("expected replay window size", *argv);
} else if (strcmp(*argv, "validate") == 0) {
NEXT_ARG();
- validate = parse_one_of("validate", *argv, validate_str,
- ARRAY_SIZE(validate_str), &ret);
+ validate = parse_one_of_deprecated("validate",
+ *argv, validate_str,
+ ARRAY_SIZE(validate_str),
+ &ret);
if (ret != 0)
return ret;
addattr8(n, MACSEC_BUFLEN,
@@ -1472,8 +1475,10 @@ static int macsec_parse_opt(struct link_util *lu, int argc, char **argv,
invarg("expected an { 0..3 }", *argv);
} else if (strcmp(*argv, "offload") == 0) {
NEXT_ARG();
- offload = parse_one_of("offload", *argv, offload_str,
- ARRAY_SIZE(offload_str), &ret);
+ offload = parse_one_of_deprecated("offload", *argv,
+ offload_str,
+ ARRAY_SIZE(offload_str),
+ &ret);
if (ret != 0)
return ret;
addattr8(n, MACSEC_BUFLEN,
diff --git a/ip/ipstats.c b/ip/ipstats.c
index 3f94ff1e..ce6c8a0e 100644
--- a/ip/ipstats.c
+++ b/ip/ipstats.c
@@ -1282,7 +1282,8 @@ static int ipstats_set(int argc, char **argv)
return -EINVAL;
}
at = IFLA_STATS_SET_OFFLOAD_XSTATS_L3_STATS;
- enable = parse_on_off("l3_stats", *argv, &err);
+ enable = parse_on_off_deprecated("l3_stats",
+ *argv, &err);
if (err)
return err;
} else if (strcmp(*argv, "help") == 0) {
diff --git a/lib/utils.c b/lib/utils.c
index 7aa3409f..19a2b63d 100644
--- a/lib/utils.c
+++ b/lib/utils.c
@@ -1754,6 +1754,12 @@ __parse_one_of(const char *msg, const char *realval,
int parse_one_of(const char *msg, const char *realval, const char * const *list,
size_t len, int *p_err)
+{
+ return __parse_one_of(msg, realval, list, len, p_err, strcmp);
+}
+
+int parse_one_of_deprecated(const char *msg, const char *realval,
+ const char * const *list, size_t len, int *p_err)
{
return __parse_one_of(msg, realval, list, len, p_err, matches);
}
@@ -1768,6 +1774,11 @@ static bool __parse_on_off(const char *msg, const char *realval, int *p_err,
}
bool parse_on_off(const char *msg, const char *realval, int *p_err)
+{
+ return __parse_on_off(msg, realval, p_err, strcmp);
+}
+
+bool parse_on_off_deprecated(const char *msg, const char *realval, int *p_err)
{
return __parse_on_off(msg, realval, p_err, matches);
}
--
2.41.0
Powered by blists - more mailing lists