[Accel-config] [PATCH v1 3/8] accel-config: Rewrite filter functions

From: Ramesh Thomas <ramesh.thomas(a)intel.com>

The filter functions were using redundant assumptions and had several
bugs. Rewrite them using current design and discard redundant functions
and operations. The functions parse arguments containing names of wqs,
groups and engines prefixed with parent device, validate and return the
pointer to the corresponding structure. This simplifies scanning for
devices, wqs, groups and engines and helps keeping interfaces of all
commands consistent.

Signed-off-by: Ramesh Thomas <ramesh.thomas(a)intel.com>
---
 util/filter.c | 369 ++++++++++++++++++++++++--------------------------
 util/filter.h |  36 ++---
 2 files changed, 198 insertions(+), 207 deletions(-)

diff --git a/util/filter.c b/util/filter.c
index 5e5840d..ba41019 100644
--- a/util/filter.c
+++ b/util/filter.c
@@ -13,274 +13,263 @@
 #include <accfg/libaccel_config.h>
 #include <accfg/lib/private.h>
 
-#define NUMA_NO_NODE    (-1)
-
-struct accfg_device *util_device_filter(struct accfg_device *device,
-					 const char *__ident)
+int match_device(struct accfg_device *dev, struct accfg_json_container *jc)
 {
-	char *end = NULL, *ident, *save;
-	int device_id, id;
-	const char *devname, *name;
-
-	if (!__ident)
-		return device;
-	ident = strdup(__ident);
-
-	if (!ident)
-		return NULL;
-	for (name = strtok_r(ident, " ", &save); name;
-	     name = strtok_r(NULL, " ", &save)) {
-		if (strcmp(name, "all") == 0)
-			break;
+	return !strcmp(accfg_device_get_devname(dev), jc->device_name);
+}
 
-		device_id = strtoul(ident, &end, 0);
+int scan_device_type_id(const char *name, char *dev_type,
+		unsigned int *dev_id)
+{
+	char type[MAX_DEV_LEN];
+	unsigned int id;
 
-		if (end == ident || end[0])
-			device_id = UINT_MAX;
+	if (sscanf(name, "%[a-z]%u", type, &id) != 2)
+		return -EINVAL;
 
-		devname = accfg_device_get_devname(device);
-		id = accfg_device_get_id(device);
-		if ((unsigned int)device_id < UINT_MAX && device_id == id)
-			break;
-		if ((unsigned int)device_id == UINT_MAX && strcmp(devname, name) == 0)
-			break;
-	}
-	free(ident);
+	if (dev_type)
+		strcpy(dev_type, type);
 
-	if (name)
-		return device;
+	if (dev_id)
+		*dev_id = id;
 
-	return NULL;
+	return 0;
 }
 
-struct accfg_group *util_group_filter(struct accfg_group *group,
-				       const char *__ident)
+int scan_parent_child_names(const char *name, char *parent_name,
+		char *child_name)
 {
-	char *ident, *save;
-	const char *name;
-	int device_id, group_id;
+	char p_name[MAX_DEV_LEN], c_name[MAX_DEV_LEN];
 
-	if (!__ident)
-		return group;
+	if (sscanf(name, "%[^/]/%s", p_name, c_name) != 2)
+		return -EINVAL;
 
-	ident = strdup(__ident);
-	if (!ident)
-		return NULL;
+	if (parent_name)
+		strcpy(parent_name, p_name);
 
-	for (name = strtok_r(ident, " ", &save); name;
-	     name = strtok_r(NULL, " ", &save)) {
-		if (strcmp(name, "all") == 0)
-			break;
+	if (child_name)
+		strcpy(child_name, c_name);
 
-		if (strcmp(name, accfg_group_get_devname(group)) == 0)
-			break;
+	return 0;
+}
 
-		if (sscanf(name, "%d.%d", &device_id, &group_id) == 2
-		    && accfg_group_get_id(group) == group_id
-		    && accfg_group_get_device_id(group) == device_id) {
-			break;
-		}
-	}
-	free(ident);
-	if (name)
-		return group;
+int scan_parent_child_ids(const char *name, unsigned int *parent_id,
+		unsigned int *child_id)
+{
+	unsigned int p_id, c_id;
+
+	if (sscanf(name, "%*[a-z]%u.%u", &p_id, &c_id) != 2)
+		return -EINVAL;
+
+	if (parent_id)
+		*parent_id = p_id;
 
-	return NULL;
+	if (child_id)
+		*child_id = c_id;
+
+	return 0;
 }
 
-struct accfg_wq *util_wq_filter(struct accfg_wq *wq, const char *__ident)
+int parse_device_name(struct accfg_ctx *ctx, const char *name,
+		struct accfg_device **device)
 {
-	struct accfg_group *group = accfg_wq_get_group(wq);
-	int group_id, wq_id;
-	const char *name;
-	char *ident, *save;
+	struct accfg_device *dev;
+	char dev_type[MAX_DEV_LEN];
+	int rc;
 
-	if (!__ident)
-		return wq;
+	rc = scan_device_type_id(name, dev_type, NULL);
+	if (rc || !accfg_device_type_validate(dev_type))
+		return -EINVAL;
 
-	ident = strdup(__ident);
-	if (!ident)
-		return NULL;
-
-	for (name = strtok_r(ident, " ", &save); name;
-	     name = strtok_r(NULL, " ", &save)) {
-		if (strcmp(name, accfg_wq_get_devname(wq)) == 0)
+	accfg_device_foreach(ctx, dev)
+		if (!strcmp(name, accfg_device_get_devname(dev)))
 			break;
 
-		if (sscanf(name, "%d.%d", &group_id, &wq_id) == 2
-		    && accfg_group_get_id(group) == group_id
-		    && accfg_wq_get_group_id(wq) == wq_id)
-			break;
+	if (!dev) {
+		fprintf(stderr, "%s device not found\n", name);
+		return -EINVAL;
 	}
-	free(ident);
 
-	if (name)
-		return wq;
-	return NULL;
+	if (device)
+		*device = dev;
+
+	return 0;
 }
 
-struct accfg_engine *util_engine_filter(struct accfg_engine *engine,
-					 const char *__ident)
+int parse_wq_name(struct accfg_ctx *ctx, const char *name,
+		struct accfg_device **device, struct accfg_wq **wq)
 {
-	struct accfg_group *group = accfg_engine_get_group(engine);
-	int group_id, engine_id;
-	const char *name;
-	char *ident, *save;
+	struct accfg_device *dev;
+	struct accfg_wq *q;
+	char dev_name[MAX_DEV_LEN], wq_name[MAX_DEV_LEN];
+	int rc;
 
-	if (!__ident)
-		return engine;
+	rc = scan_parent_child_names(name, dev_name, wq_name);
+	if (rc)
+		return rc;
 
-	ident = strdup(__ident);
-	if (!ident)
-		return NULL;
+	rc = parse_device_name(ctx, dev_name, &dev);
+	if (rc)
+		return rc;
 
-	for (name = strtok_r(ident, " ", &save); name;
-	     name = strtok_r(NULL, " ", &save)) {
-		if (strcmp(name, accfg_engine_get_devname(engine)) == 0)
+	accfg_wq_foreach(dev, q)
+		if (!strcmp(wq_name, accfg_wq_get_devname(q)))
 			break;
 
-		if (sscanf(name, "%d.%d", &group_id, &engine_id) == 2
-		    && accfg_group_get_id(group) == group_id
-		    && accfg_engine_get_id(engine) == engine_id)
-			break;
+	if (!q) {
+		fprintf(stderr, "%s workqueue not found\n", name);
+		return -EINVAL;
 	}
-	free(ident);
 
-	if (name)
-		return engine;
-	return NULL;
-}
+	if (device)
+		*device = dev;
 
-struct accfg_device *util_device_filter_by_group(struct accfg_device *device,
-						  const char *ident)
-{
-	struct accfg_group *group;
-
-	if (!ident || strcmp(ident, "all") == 0)
-		return device;
+	if (wq)
+		*wq = q;
 
-	accfg_group_foreach(device, group) {
-		if (util_group_filter(group, ident))
-			return device;
-	}
-	return NULL;
+	return 0;
 }
 
-struct accfg_device *util_device_filter_by_wq(struct accfg_device *device,
-					       const char *ident)
+int parse_group_name(struct accfg_ctx *ctx, const char *name,
+		struct accfg_device **device, struct accfg_group **group)
 {
-	struct accfg_group *group;
-	struct accfg_wq *wq;
+	struct accfg_device *dev;
+	struct accfg_group *g;
+	char dev_name[MAX_DEV_LEN], group_name[MAX_DEV_LEN];
+	int rc;
 
-	if (!ident || strcmp(ident, "all") == 0)
-		return device;
+	rc = scan_parent_child_names(name, dev_name, group_name);
+	if (rc)
+		return rc;
 
-	accfg_group_foreach(device, group)
-		accfg_wq_foreach(device, wq)
-			if (util_wq_filter(wq, ident))
-				return device;
-	return NULL;
-}
+	rc = parse_device_name(ctx, dev_name, &dev);
+	if (rc)
+		return rc;
 
-struct accfg_device *util_device_filter_by_engine(struct accfg_device *device,
-						   const char *ident)
-{
-	struct accfg_group *group;
-	struct accfg_engine *engine;
+	accfg_group_foreach(dev, g)
+		if (!strcmp(group_name, accfg_group_get_devname(g)))
+			break;
 
-	if (!ident || strcmp(ident, "all") == 0)
-		return device;
+	if (!g) {
+		fprintf(stderr, "%s group not found\n", name);
+		return -EINVAL;
+	}
+
+	if (device)
+		*device = dev;
 
-	accfg_group_foreach(device, group)
-		accfg_engine_foreach(device, engine)
-			if (util_engine_filter(engine, ident))
-				return device;
-	return NULL;
+	if (group)
+		*group = g;
+
+	return 0;
 }
 
-struct accfg_group *util_group_filter_by_wq(struct accfg_group *group,
-					     const char *ident)
+int parse_engine_name(struct accfg_ctx *ctx, const char *name,
+		struct accfg_device **device, struct accfg_engine **engine)
 {
-	struct accfg_wq *wq;
-	struct accfg_device *device = group->device;
+	struct accfg_device *dev;
+	struct accfg_engine *e;
+	char dev_name[MAX_DEV_LEN], engine_name[MAX_DEV_LEN];
+	int rc;
 
-	if (!ident || strcmp(ident, "all") == 0)
-		return group;
+	rc = scan_parent_child_names(name, dev_name, engine_name);
+	if (rc)
+		return rc;
 
-	accfg_wq_foreach(device, wq)
-		if (util_wq_filter(wq, ident))
-			return group;
-	return NULL;
-}
+	rc = parse_device_name(ctx, dev_name, &dev);
+	if (rc)
+		return rc;
 
-struct accfg_group *util_group_filter_by_engine(struct accfg_group *group,
-						 const char *ident)
-{
-	struct accfg_engine *engine;
-	struct accfg_device *device = group->device;
+	accfg_engine_foreach(dev, e)
+		if (!strcmp(engine_name, accfg_engine_get_devname(e)))
+			break;
 
-	if (!ident || strcmp(ident, "all") == 0)
-		return group;
+	if (!e) {
+		fprintf(stderr, "%s engine not found\n", name);
+		return -EINVAL;
+	}
 
-	accfg_engine_foreach(device, engine)
-		if (util_engine_filter(engine, ident))
-			return group;
-	return NULL;
-}
+	if (device)
+		*device = dev;
+
+	if (engine)
+		*engine = e;
 
-int match_device(struct accfg_device *dev, struct accfg_json_container *jc)
-{
-	if ((accfg_device_get_id(dev) == jc->device_id) &&
-		strcmp(accfg_device_get_devname(dev),
-		jc->device_name) == 0)
-		return 1;
 	return 0;
 }
 
 int util_filter_walk(struct accfg_ctx *ctx, struct util_filter_ctx *fctx,
 		     struct util_filter_params *param)
 {
-	struct accfg_device *device;
-	struct accfg_wq *wq;
-	struct accfg_engine *engine;
-	struct accfg_group *group;
+	struct accfg_device *device, *dev  = NULL;
+	struct accfg_wq *wq, *q = NULL;
+	struct accfg_engine *engine, *e  = NULL;
+	struct accfg_group *group, *g = NULL;
+	bool b, found = false;
+	int rc = 0;
+
+	if (param->device)
+		rc = parse_device_name(ctx, param->device, &dev);
+	else if (param->group)
+		rc = parse_group_name(ctx, param->group, &dev, &g);
+	else if (param->wq)
+		rc = parse_wq_name(ctx, param->wq, &dev, &q);
+	else if (param->engine)
+		rc = parse_engine_name(ctx, param->engine, &dev, &e);
+	else
+		found = true;
+
+	if (rc)
+		return rc;
 
 	accfg_device_foreach(ctx, device) {
-		if (!util_device_filter(device, param->device)
-		    || !util_device_filter_by_group(device, param->group)
-		    || !util_device_filter_by_wq(device, param->wq)
-		    || !util_device_filter_by_engine(device, param->engine))
+		if (dev && dev != device)
 			continue;
 
 		if (!fctx->filter_device(device, fctx))
 			continue;
 
+		if (param->device)
+			found = true;
+
 		accfg_group_foreach(device, group) {
-			if (!util_group_filter(group, param->group))
-				continue;
-			if (!fctx->filter_group(group, fctx))
+			if (g && g != group)
 				continue;
+			b = fctx->filter_group(group, fctx);
+			if (g) {
+				found = b;
+				break;
+			}
 		}
 
 		accfg_wq_foreach(device, wq) {
-			if (!fctx->filter_wq)
-				break;
-
-			if (!util_wq_filter(wq, param->wq))
+			if (q && q != wq)
 				continue;
-
-			fctx->filter_wq(wq, fctx);
+			b = fctx->filter_wq(wq, fctx);
+			if (q) {
+				found = b;
+				break;
+			}
 		}
 
 		accfg_engine_foreach(device, engine) {
-			if (!fctx->filter_engine)
+			if (e && e != engine)
+				continue;
+			b = fctx->filter_engine(engine, fctx);
+			if (e) {
+				found = b;
 				break;
+			}
+		}
 
-			if (!util_engine_filter(engine, param->engine))
-				continue;
+		if (dev)
+			break;
+	}
 
-			fctx->filter_engine(engine, fctx);
-		}
+	if (!found) {
+		fprintf(stderr, "No matching device found\n");
+		return -EINVAL;
 	}
+
 	return 0;
 }
diff --git a/util/filter.h b/util/filter.h
index aa13efe..5d4eed1 100644
--- a/util/filter.h
+++ b/util/filter.h
@@ -6,23 +6,10 @@
 #include <ccan/list/list.h>
 #include <ccan/short_types/short_types.h>
 
-struct accfg_device *util_device_filter(struct accfg_device *device, const char *ident);
-struct accfg_group *util_group_filter(struct accfg_group *group,
-		const char *ident);
-struct accfg_wq *util_wq_filter(struct accfg_wq *wq,
-		const char *ident);
-struct accfg_engine *util_engine_filter(struct accfg_engine *engine, const char *ident);
-
-struct accfg_device *util_device_filter_by_group(struct accfg_device *device,
-		const char *ident);
-struct accfg_group *util_group_filter_by_wq(struct accfg_group *group,
-		const char *ident);
-struct accfg_group *util_group_filter_by_engine(struct accfg_group *group,
-		const char *ident);
-struct accfg_wq *util_wq_filter_by_group(struct accfg_wq *wq,
-		const char *ident);
-struct accfg_engine *util_engine_filter_by_group(struct accfg_engine *engine,
-		const char *ident);
+struct accfg_device;
+struct accfg_group;
+struct accfg_wq;
+struct accfg_engine;
 struct json_object;
 
 /* json object hierarchy for device */
@@ -99,6 +86,21 @@ struct util_filter_params {
 };
 
 struct accfg_ctx;
+
+int scan_device_type_id(const char *name, char *dev_type,
+		unsigned int *dev_id);
+int scan_parent_child_names(const char *name, char *parent_name,
+		char *child_name);
+int scan_parent_child_ids(const char *name, unsigned int *parent_id,
+		unsigned int *child_id);
+int parse_device_name(struct accfg_ctx *ctx, const char *name,
+		struct accfg_device **device);
+int parse_wq_name(struct accfg_ctx *ctx, const char *name,
+		struct accfg_device **device, struct accfg_wq **wq);
+int parse_group_name(struct accfg_ctx *ctx, const char *name,
+		struct accfg_device **device, struct accfg_group **group);
+int parse_engine_name(struct accfg_ctx *ctx, const char *name,
+		struct accfg_device **device, struct accfg_engine **engine);
 int util_filter_walk(struct accfg_ctx *ctx, struct util_filter_ctx *fctx,
 		struct util_filter_params *param);
 int match_device(struct accfg_device *device, struct accfg_json_container *jc);
-- 
2.26.3