[PATCH v2 24/61] net/iavf: use kvargs numeric helpers

[Stephen Hemminger]

parse_u16() ignores the end pointer, so "8junk" is accepted, and assigns
a u64 into a u16 without a range check, so "65536" wraps to zero. Zero
is still rejected, as it is the unset marker for quanta_size.

parse_bool() checks neither errno nor the end pointer.

iavf_parse_watchdog_period() uses atoi(), which cannot report an error
at all, so a malformed value becomes zero and the negative check never
fires.

no-poll-on-link-down, auto_reset and enable_lldp are booleans, so make
the fields bool and use rte_kvargs_handle_bool(). That is every caller
of the local parse_bool(), so it goes away too. A bare key now enables
the option, and the usual spellings are accepted.

Signed-off-by: Stephen Hemminger <stephen at networkplumber.org>
---
 drivers/net/intel/iavf/iavf.h        |  6 +--
 drivers/net/intel/iavf/iavf_ethdev.c | 55 +++++++++-------------------
 2 files changed, 20 insertions(+), 41 deletions(-)

diff --git a/drivers/net/intel/iavf/iavf.h b/drivers/net/intel/iavf/iavf.h
index eb7cf2b58d..8f31fbabbf 100644
--- a/drivers/net/intel/iavf/iavf.h
+++ b/drivers/net/intel/iavf/iavf.h
@@ -324,10 +324,10 @@ struct iavf_devargs {
 	uint8_t proto_xtr[IAVF_MAX_QUEUE_NUM];
 	uint16_t quanta_size;
 	uint32_t watchdog_period;
-	int auto_reset;
-	int no_poll_on_link_down;
+	bool auto_reset;
+	bool no_poll_on_link_down;
 	uint64_t mbuf_check;
-	int enable_lldp;
+	bool enable_lldp;
 };
 
 struct iavf_security_ctx;
diff --git a/drivers/net/intel/iavf/iavf_ethdev.c b/drivers/net/intel/iavf/iavf_ethdev.c
index a8abe0154c..27d6f73b94 100644
--- a/drivers/net/intel/iavf/iavf_ethdev.c
+++ b/drivers/net/intel/iavf/iavf_ethdev.c
@@ -3,6 +3,7 @@
  */
 
 #include <ctype.h>
+#include <limits.h>
 #include <sys/queue.h>
 #include <stdalign.h>
 #include <stdio.h>
@@ -13,6 +14,7 @@
 #include <stdarg.h>
 #include <inttypes.h>
 #include <rte_byteorder.h>
+#include <rte_kvargs.h>
 #include <rte_common.h>
 #include <rte_os_shim.h>
 
@@ -2401,11 +2403,10 @@ static int
 parse_u16(__rte_unused const char *key, const char *value, void *args)
 {
 	u16 *num = (u16 *)args;
-	u16 tmp;
+	uint64_t tmp;
 
-	errno = 0;
-	tmp = strtoull(value, NULL, 10);
-	if (errno || !tmp) {
+	/* zero is not accepted, it is the "unset" marker for this argument */
+	if (rte_kvargs_to_uint(value, 1, UINT16_MAX, &tmp) < 0) {
 		PMD_DRV_LOG(WARNING, "%s: \"%s\" is not a valid u16",
 			    key, value);
 		return -1;
@@ -2416,35 +2417,13 @@ parse_u16(__rte_unused const char *key, const char *value, void *args)
 	return 0;
 }
 
-static int
-parse_bool(const char *key, const char *value, void *args)
-{
-	int *i = (int *)args;
-	char *end;
-	int num;
-
-	num = strtoul(value, &end, 10);
-
-	if (num != 0 && num != 1) {
-		PMD_DRV_LOG(WARNING, "invalid value:\"%s\" for key:\"%s\", "
-			"value must be 0 or 1",
-			value, key);
-		return -1;
-	}
-
-	*i = num;
-	return 0;
-}
-
 static int
 iavf_parse_watchdog_period(__rte_unused const char *key, const char *value, void *args)
 {
 	int *num = (int *)args;
-	int tmp;
+	uint64_t tmp;
 
-	errno = 0;
-	tmp = atoi(value);
-	if (tmp < 0) {
+	if (rte_kvargs_to_uint(value, 0, INT_MAX, &tmp) < 0) {
 		PMD_DRV_LOG(WARNING, "%s: \"%s\" is not greater than or equal to zero",
 				key, value);
 		return -1;
@@ -2515,8 +2494,8 @@ static int iavf_parse_devargs(struct rte_eth_dev *dev)
 	int ret;
 	int watchdog_period = -1;
 
-	ad->devargs.auto_reset = 1;
-	ad->devargs.no_poll_on_link_down = 1;
+	ad->devargs.auto_reset = true;
+	ad->devargs.no_poll_on_link_down = true;
 
 	if (!devargs)
 		return 0;
@@ -2550,8 +2529,8 @@ static int iavf_parse_devargs(struct rte_eth_dev *dev)
 	else
 		ad->devargs.watchdog_period = watchdog_period;
 
-	ret = rte_kvargs_process(kvlist, IAVF_NO_POLL_ON_LINK_DOWN_ARG,
-				 &parse_bool, &ad->devargs.no_poll_on_link_down);
+	ret = rte_kvargs_process_opt(kvlist, IAVF_NO_POLL_ON_LINK_DOWN_ARG,
+				 rte_kvargs_handle_bool, &ad->devargs.no_poll_on_link_down);
 	if (ret)
 		goto bail;
 
@@ -2568,15 +2547,15 @@ static int iavf_parse_devargs(struct rte_eth_dev *dev)
 	if (ret)
 		goto bail;
 
-	ret = rte_kvargs_process(kvlist, IAVF_ENABLE_AUTO_RESET_ARG,
-				 &parse_bool, &ad->devargs.auto_reset);
+	ret = rte_kvargs_process_opt(kvlist, IAVF_ENABLE_AUTO_RESET_ARG,
+				 rte_kvargs_handle_bool, &ad->devargs.auto_reset);
 	if (ret)
 		goto bail;
 
-	if (ad->devargs.auto_reset != 0 && ad->devargs.no_poll_on_link_down == 0) {
+	if (ad->devargs.auto_reset && !ad->devargs.no_poll_on_link_down) {
 		PMD_INIT_LOG(WARNING,
 			"no-poll-on-link-down=0 is incompatible with auto_reset=1, ignoring");
-		ad->devargs.no_poll_on_link_down = 1;
+		ad->devargs.no_poll_on_link_down = true;
 	}
 
 	if (rte_kvargs_count(kvlist, IAVF_ENABLE_PTYPE_LLDP_ARG) > 0) {
@@ -2586,8 +2565,8 @@ static int iavf_parse_devargs(struct rte_eth_dev *dev)
 		goto bail;
 	}
 
-	ret = rte_kvargs_process(kvlist, IAVF_ENABLE_LLDP_ARG,
-				 &parse_bool, &ad->devargs.enable_lldp);
+	ret = rte_kvargs_process_opt(kvlist, IAVF_ENABLE_LLDP_ARG,
+				 rte_kvargs_handle_bool, &ad->devargs.enable_lldp);
 	if (ret)
 		goto bail;
 
-- 
2.53.0