[dpdk-dev] [PATCH 2/2] dev: use rte_kvargs

[Gaetan Rivet]

Signed-off-by: Gaetan Rivet <gaetan.rivet at 6wind.com>
---

Cc: Neil Horman <nhorman at tuxdriver.com>

I find using rte_parse_kv cleaner.
The function rte_dev_iterator_init is already ugly enough as it is.
This is really not helping.

 lib/librte_eal/common/eal_common_dev.c | 127 +++++++++++++++++++++------------
 lib/librte_eal/linuxapp/eal/Makefile   |   1 +
 2 files changed, 83 insertions(+), 45 deletions(-)

diff --git a/lib/librte_eal/common/eal_common_dev.c b/lib/librte_eal/common/eal_common_dev.c
index 21703b777..9f1a0ebda 100644
--- a/lib/librte_eal/common/eal_common_dev.c
+++ b/lib/librte_eal/common/eal_common_dev.c
@@ -15,6 +15,7 @@
 #include <rte_devargs.h>
 #include <rte_debug.h>
 #include <rte_errno.h>
+#include <rte_kvargs.h>
 #include <rte_log.h>
 
 #include "eal_private.h"
@@ -270,12 +271,15 @@ rte_eal_hotplug_remove(const char *busname, const char *devname)
 }
 
 int __rte_experimental
-rte_dev_iterator_init(struct rte_dev_iterator *it, const char *str)
+rte_dev_iterator_init(struct rte_dev_iterator *it,
+		      const char *devstr)
 {
-	struct rte_bus *bus = NULL;
+	struct rte_kvargs *kvlist = NULL;
 	struct rte_class *cls = NULL;
-	struct rte_kvarg kv;
-	char *slash;
+	struct rte_bus *bus = NULL;
+	struct rte_kvargs_pair *kv;
+	char *slash = NULL;
+	char *str = NULL;
 
 	/* Having both busstr and clsstr NULL is illegal,
 	 * marking this iterator as invalid unless
@@ -283,98 +287,131 @@ rte_dev_iterator_init(struct rte_dev_iterator *it, const char *str)
 	 */
 	it->busstr = NULL;
 	it->clsstr = NULL;
+	str = strdup(devstr);
+	if (str == NULL) {
+		rte_errno = ENOMEM;
+		goto get_out;
+	}
+	slash = strchr(str, '/');
+	if (slash != NULL) {
+		slash[0] = '\0';
+		slash = strchr(devstr, '/') + 1;
+	}
 	/* Safety checks and prep-work */
-	if (rte_parse_kv(str, &kv)) {
+	kvlist = rte_kvargs_parse(str, NULL);
+	if (kvlist == NULL) {
 		RTE_LOG(ERR, EAL, "Could not parse: %s\n", str);
 		rte_errno = EINVAL;
-		return -rte_errno;
+		goto get_out;
 	}
 	it->device = NULL;
 	it->class_device = NULL;
-	if (strcmp(kv.key, "bus") == 0) {
-		bus = rte_bus_find_by_name(kv.value);
+	kv = &kvlist->pairs[0];
+	if (strcmp(kv->key, "bus") == 0) {
+		bus = rte_bus_find_by_name(kv->value);
 		if (bus == NULL) {
 			RTE_LOG(ERR, EAL, "Could not find bus \"%s\"\n",
-				kv.value);
+				kv->value);
 			rte_errno = EFAULT;
-			return -rte_errno;
+			goto get_out;
 		}
-		slash = strchr(str, '/');
 		if (slash != NULL) {
-			if (rte_parse_kv(slash + 1, &kv)) {
+			rte_kvargs_free(kvlist);
+			kvlist = rte_kvargs_parse(slash, NULL);
+			if (kvlist == NULL) {
 				RTE_LOG(ERR, EAL, "Could not parse: %s\n",
-					slash + 1);
+					slash);
 				rte_errno = EINVAL;
-				return -rte_errno;
+				goto get_out;
 			}
-			cls = rte_class_find_by_name(kv.value);
+			kv = &kvlist->pairs[0];
+			if (strcmp(kv->key, "class")) {
+				RTE_LOG(ERR, EAL, "Additional layer must be a class\n");
+				rte_errno = EINVAL;
+				goto get_out;
+			}
+			cls = rte_class_find_by_name(kv->value);
 			if (cls == NULL) {
 				RTE_LOG(ERR, EAL, "Could not find class \"%s\"\n",
-					kv.value);
+					kv->value);
 				rte_errno = EFAULT;
-				return -rte_errno;
+				goto get_out;
 			}
 		}
-	} else if (strcmp(kv.key, "class") == 0) {
-		cls = rte_class_find_by_name(kv.value);
+	} else if (strcmp(kv->key, "class") == 0) {
+		cls = rte_class_find_by_name(kv->value);
 		if (cls == NULL) {
 			RTE_LOG(ERR, EAL, "Could not find class \"%s\"\n",
-				kv.value);
+				kv->value);
 			rte_errno = EFAULT;
-			return -rte_errno;
+			goto get_out;
 		}
 	} else {
 		rte_errno = EINVAL;
-		return -rte_errno;
+		goto get_out;
 	}
 	/* The string should have at least
 	 * one layer specified.
 	 */
 	if (bus == NULL && cls == NULL) {
 		rte_errno = EINVAL;
-		return -rte_errno;
+		goto get_out;
 	}
 	if ((bus != NULL && bus->dev_iterate == NULL) ||
 	    (cls != NULL && cls->dev_iterate == NULL)) {
 		rte_errno = ENOTSUP;
-		return -rte_errno;
+		goto get_out;
 	}
 	if (bus != NULL) {
-		it->busstr = str;
+		it->busstr = devstr;
 		if (cls != NULL)
-			it->clsstr = slash + 1;
+			it->clsstr = slash;
 	} else if (cls != NULL) {
-		it->clsstr = str;
+		it->clsstr = devstr;
 	}
-	it->devstr = str;
+	it->devstr = devstr;
 	it->bus = bus;
 	it->cls = cls;
-	return 0;
+get_out:
+	rte_kvargs_free(kvlist);
+	free(str);
+	return -rte_errno;
+}
+
+/* '\0' forbidden in sym */
+static const char *
+strfirstof(const char *str,
+	   const char *sym)
+{
+	const char *s;
+
+	for (s = str; s[0] != '\0'; s++) {
+		const char *c;
+
+		for (c = sym; c[0] != '\0'; c++) {
+			if (c[0] == s[0])
+				return s;
+		}
+	}
+	return NULL;
 }
 
 static char *
 dev_str_sane_cpy(const char *str)
 {
-	struct rte_kvarg kv;
-	char *end;
+	const char *end;
 	char *cpy;
 
-	if (rte_parse_kv(str, &kv)) {
-		rte_errno = EINVAL;
-		return NULL;
-	}
-	/* copying '\0' is valid. */
-	if (kv.next != NULL)
-		cpy = strdup(kv.next);
-	else
+	end = strfirstof(str, ",/");
+	if (end != NULL &&
+	    end[0] == ',') {
+		cpy = strdup(end + 1);
+	} else {
+		/* '/' or '\0' */
 		cpy = strdup("");
-	if (cpy == NULL) {
-		rte_errno = ENOMEM;
-		return NULL;
 	}
-	end = strchr(cpy, '/');
-	if (end != NULL)
-		end[0] = '\0';
+	if (cpy == NULL)
+		rte_errno = ENOMEM;
 	return cpy;
 }
 
diff --git a/lib/librte_eal/linuxapp/eal/Makefile b/lib/librte_eal/linuxapp/eal/Makefile
index a3edbbe76..87caa23a1 100644
--- a/lib/librte_eal/linuxapp/eal/Makefile
+++ b/lib/librte_eal/linuxapp/eal/Makefile
@@ -27,6 +27,7 @@ LDLIBS += -lrt
 ifeq ($(CONFIG_RTE_EAL_NUMA_AWARE_HUGEPAGES),y)
 LDLIBS += -lnuma
 endif
+LDLIBS += -lrte_kvargs
 
 # specific to linuxapp exec-env
 SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) := eal.c
-- 
2.11.0