[LTP] [PATCH 0/3] Switch bitflags in metadata to bool

[LTP] [PATCH 0/3] Switch bitflags in metadata to bool

[Cyril Hrubis]

Cyril Hrubis (3):
  metaparse: data_storage: Add bool type
  metadata: metaparse: Add bool mappings to typemap
  lib: shell: Switch cmd.optional to bool

 metadata/data_storage.h                 | 31 +++++++++
 metadata/metaparse.c                    | 85 +++++++++++++++++++++++--
 testcases/lib/tests/shell_loader_cmd.sh |  4 +-
 testcases/lib/tst_run_shell.c           |  4 +-
 4 files changed, 113 insertions(+), 11 deletions(-)

-- 
2.52.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH 1/3] metaparse: data_storage: Add bool type

[Cyril Hrubis]

Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
---
 metadata/data_storage.h | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/metadata/data_storage.h b/metadata/data_storage.h
index 6ca5d7d90..77073f37d 100644
--- a/metadata/data_storage.h
+++ b/metadata/data_storage.h
@@ -17,6 +17,7 @@ enum data_type {
 	DATA_HASH,
 	DATA_STRING,
 	DATA_INT,
+	DATA_BOOL,
 	DATA_NULL,
 };
 
@@ -49,6 +50,11 @@ struct data_node_int {
 	long val;
 };
 
+struct data_node_bool {
+	enum data_type type;
+	bool val;
+};
+
 struct data_node {
 	union {
 		enum data_type type;
@@ -56,6 +62,7 @@ struct data_node {
 		struct data_node_array array;
 		struct data_node_string string;
 		struct data_node_int i;
+		struct data_node_bool b;
 	};
 };
 
@@ -70,6 +77,8 @@ static inline const char* data_type_name(enum data_type type)
 		return "string";
 	case DATA_INT:
 		return "int";
+	case DATA_BOOL:
+		return "bool";
 	case DATA_NULL:
 		return "null";
 	default:
@@ -104,6 +113,19 @@ static inline struct data_node *data_node_int(long i)
 	return node;
 }
 
+static inline struct data_node *data_node_bool(bool b)
+{
+	struct data_node *node = malloc(sizeof(struct data_node_int));
+
+	if (!node)
+		return NULL;
+
+	node->type = DATA_BOOL;
+	node->b.val = b;
+
+	return node;
+}
+
 static inline struct data_node *data_node_null(void)
 {
 	struct data_node *node = malloc(sizeof(struct data_node));
@@ -175,6 +197,7 @@ static inline void data_node_free(struct data_node *self)
 	switch (self->type) {
 	case DATA_STRING:
 	case DATA_INT:
+	case DATA_BOOL:
 	case DATA_NULL:
 	break;
 	case DATA_HASH:
@@ -314,6 +337,10 @@ static inline void data_node_print_(struct data_node *self, unsigned int padd)
 		data_print_padd(padd);
 		printf("%li\n", self->i.val);
 	break;
+	case DATA_BOOL:
+		data_print_padd(padd);
+		printf("%s\n", self->b.val ? "true" : "false");
+	break;
 	case DATA_STRING:
 		data_print_padd(padd);
 		printf("'%s'\n", self->string.val);
@@ -407,6 +434,10 @@ static inline void data_to_json_(struct data_node *self, FILE *f, unsigned int p
 		padd = do_padd ? padd : 0;
 		data_fprintf(f, padd, "%li", self->i.val);
 	break;
+	case DATA_BOOL:
+		padd = do_padd ? padd : 0;
+		data_fprintf(f, padd, "%s", self->b.val ? "true" : "false");
+	break;
 	case DATA_STRING:
 		padd = do_padd ? padd : 0;
 		data_fprintf_esc(f, padd, self->string.val);
-- 
2.52.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/3] metaparse: data_storage: Add bool type

[Petr Vorel]

Hi Cyril,

LGTM, thanks for preparing it!
Reviewed-by: Petr Vorel <pvorel@suse.cz>

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH 2/3] metadata: metaparse: Add bool mappings to typemap

[Cyril Hrubis]

- add all boolean flags in tst_test to the typemap

- make typemap recursive (adds child pointer to the typemap struct)

- add typemap mappings for the need_cmds optional key

- change the implied tags to bool

Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
---
 metadata/metaparse.c | 85 ++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 78 insertions(+), 7 deletions(-)

diff --git a/metadata/metaparse.c b/metadata/metaparse.c
index 36736ac06..3bca70879 100644
--- a/metadata/metaparse.c
+++ b/metadata/metaparse.c
@@ -968,11 +968,40 @@ static struct data_node *parse_file(const char *fname)
 	return res;
 }
 
-static struct typemap {
+struct typemap {
 	const char *id;
 	enum data_type type;
-} tst_test_typemap[] = {
+	struct typemap *child;
+};
+
+static struct typemap needs_cmds_typemap[] = {
+	{.id = "optional", .type = DATA_BOOL},
+	{}
+};
+
+static struct typemap tst_test_typemap[] = {
 	{.id = "test_variants", .type = DATA_INT},
+	/* All bitflags in tst_test struct */
+	{.id = "needs_tmpdir", .type = DATA_BOOL},
+	{.id = "needs_root", .type = DATA_BOOL},
+	{.id = "forks_child", .type = DATA_BOOL},
+	{.id = "needs_device", .type = DATA_BOOL},
+	{.id = "needs_checkpoints", .type = DATA_BOOL},
+	{.id = "needs_overlay", .type = DATA_BOOL},
+	{.id = "format_device", .type = DATA_BOOL},
+	{.id = "mount_device", .type = DATA_BOOL},
+	{.id = "needs_rofs", .type = DATA_BOOL},
+	{.id = "child_needs_reinit", .type = DATA_BOOL},
+	{.id = "runs_script", .type = DATA_BOOL},
+	{.id = "needs_devfs", .type = DATA_BOOL},
+	{.id = "restore_wallclock", .type = DATA_BOOL},
+	{.id = "all_filesystems", .type = DATA_BOOL},
+	{.id = "skip_in_lockdown", .type = DATA_BOOL},
+	{.id = "skip_in_secureboot", .type = DATA_BOOL},
+	{.id = "skip_in_compat", .type = DATA_BOOL},
+	{.id = "needs_hugetlbfs", .type = DATA_BOOL},
+	{.id = "needs_cgroup_nsdelegate", .type = DATA_BOOL},
+	{.id = "needs_cmds", .child = needs_cmds_typemap},
 	{}
 };
 
@@ -996,18 +1025,55 @@ static void convert_str2int(struct data_node *res, const char *id, const char *s
 	data_node_hash_add(res, id, data_node_int(val));
 }
 
-static void check_normalize_types(struct data_node *res)
+static void convert_str2bool(struct data_node *res, const char *id, const char *str_val)
+{
+	long val;
+	char *endptr;
+
+	errno = 0;
+	val = strtol(str_val, &endptr, 10);
+
+	if (errno || *endptr) {
+		fprintf(stderr,	"Cannot convert %s value %s to bool!\n", id, str_val);
+		exit(1);
+	}
+
+	if (verbose)
+		fprintf(stderr, "NORMALIZING %s TO BOOL %li\n", id, val);
+
+	data_node_hash_del(res, id);
+	data_node_hash_add(res, id, data_node_bool(val));
+}
+
+static void check_normalize_types(struct data_node *res, const char *id, struct typemap *typemaps)
 {
 	unsigned int i;
 
-	for (i = 0; tst_test_typemap[i].id; i++) {
+	if (res->type == DATA_ARRAY) {
+		for (i = 0; i < res->array.array_used; i++)
+			check_normalize_types(res->array.array[i], id, typemaps);
+
+		return;
+	}
+
+	if (res->type != DATA_HASH) {
+		fprintf(stderr, "Typemap '%s' type %s has no children!\n", id, data_type_name(res->type));
+		exit(1);
+	}
+
+	for (i = 0; typemaps[i].id; i++) {
 		struct data_node *n;
-		struct typemap *typemap = &tst_test_typemap[i];
+		struct typemap *typemap = &typemaps[i];
 
 		n = data_node_hash_get(res, typemap->id);
 		if (!n)
 			continue;
 
+		if (typemap->child) {
+			check_normalize_types(n, typemap->id, typemap->child);
+			continue;
+		}
+
 		if (n->type == typemap->type)
 			continue;
 
@@ -1016,6 +1082,11 @@ static void check_normalize_types(struct data_node *res)
 			continue;
 		}
 
+		if (n->type == DATA_STRING && typemap->type == DATA_BOOL) {
+			convert_str2bool(res, typemap->id, n->string.val);
+			continue;
+		}
+
 		fprintf(stderr, "Cannot convert %s from %s to %s!\n",
 			typemap->id, data_type_name(n->type),
 			data_type_name(typemap->type));
@@ -1125,14 +1196,14 @@ int main(int argc, char *argv[])
 	}
 
 	/* Normalize types */
-	check_normalize_types(res);
+	check_normalize_types(res, "",  tst_test_typemap);
 
 	for (i = 0; implies[i].flag; i++) {
 		if (data_node_hash_get(res, implies[i].flag)) {
 			for (j = 0; implies[i].implies[j]; j++) {
 				if (!data_node_hash_get(res, implies[i].implies[j]))
 					data_node_hash_add(res, implies[i].implies[j],
-							   data_node_string("1"));
+							   data_node_bool(true));
 			}
 		}
 	}
-- 
2.52.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 2/3] metadata: metaparse: Add bool mappings to typemap

[Petr Vorel]

Hi Cyril,

> - add all boolean flags in tst_test to the typemap

> - make typemap recursive (adds child pointer to the typemap struct)

> - add typemap mappings for the need_cmds optional key

> - change the implied tags to bool

LGTM.
Reviewed-by: Petr Vorel <pvorel@suse.cz>

Later (after the release) it'd be nice to add a minimal doc (i.e. describe
structs in data_storage.h). I know that the usage is quite obvious when one has
knowledge of struct tst_test, but it's getting more complex (and will, because
if we want to get tst_net.sh support, at least getopts will need to be somehow
supported unless handled in shell).

...
>  	/* Normalize types */
> -	check_normalize_types(res);
> +	check_normalize_types(res, "",  tst_test_typemap);
very nit: 2 spaces.

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 2/3] metadata: metaparse: Add bool mappings to typemap

[Cyril Hrubis]

Hi!
> Later (after the release) it'd be nice to add a minimal doc (i.e. describe
> structs in data_storage.h).

Added to my TODO, not sure when I get to it though.


Thanks for the reviews, patchset pushed.


-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH 3/3] lib: shell: Switch cmd.optional to bool

[Cyril Hrubis]

Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
---
 testcases/lib/tests/shell_loader_cmd.sh | 4 ++--
 testcases/lib/tst_run_shell.c           | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/testcases/lib/tests/shell_loader_cmd.sh b/testcases/lib/tests/shell_loader_cmd.sh
index 4e8e61eb6..1fba2a193 100755
--- a/testcases/lib/tests/shell_loader_cmd.sh
+++ b/testcases/lib/tests/shell_loader_cmd.sh
@@ -8,11 +8,11 @@
 #  "needs_cmds": [
 #   {
 #    "cmd": "ls",
-#    "optional": 1
+#    "optional": true
 #   },
 #   {
 #    "cmd": "mkfs.ext4 >= 1.0.0",
-#    "optional": 1
+#    "optional": true
 #   }
 #  ]
 # }
diff --git a/testcases/lib/tst_run_shell.c b/testcases/lib/tst_run_shell.c
index ddcb606b2..c12361ef5 100644
--- a/testcases/lib/tst_run_shell.c
+++ b/testcases/lib/tst_run_shell.c
@@ -186,7 +186,7 @@ enum cmd_ids {
 
 static ujson_obj_attr cmd_attrs[] = {
 	UJSON_OBJ_ATTR_IDX(CMD, "cmd", UJSON_STR),
-	UJSON_OBJ_ATTR_IDX(OPTIONAL, "optional", UJSON_INT),
+	UJSON_OBJ_ATTR_IDX(OPTIONAL, "optional", UJSON_BOOL),
 };
 
 static ujson_obj cmd_obj = {
@@ -299,7 +299,7 @@ static struct tst_cmd *parse_cmds(ujson_reader *reader, ujson_val *val)
 				ret[i].cmd = strdup(val->val_str);
 			break;
 			case OPTIONAL:
-				ret[i].optional = val->val_int;
+				ret[i].optional = val->val_bool;
 			break;
 			}
 		}
-- 
2.52.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 3/3] lib: shell: Switch cmd.optional to bool

[Petr Vorel]

Hi Cyril,

obviously correct, thanks!
Reviewed-by: Petr Vorel <pvorel@suse.cz>

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp