[LTP] [PATCH 0/9] Metadata parser fixes

[LTP] [PATCH 0/9] Metadata parser fixes

[Cyril Hrubis]

This fixes a few metadata parser bugs, mostly array and structure
initializers. After this patchset the .save_restore entries should be
well formatted and we can make the shell version 1:1 with the C parsed
metadata.

The TODO is the filesystem parsing which uses named initializers (e.g.
.fs_type = "ext3") and the structure field names end up in the result
verbatim. Rather than that the parser should make it a JSON object
instead. I will fix that later on.

Cyril Hrubis (9):
  metadata: metaparse: Parse operators *, +, and | properly
  metadata: metaparse: Implement recursive include
  metadata: metaparse: Apply macros in arrays.
  metadata: data_storage: Add JSON null type
  metadata: data_storage: Add two array functions
  metadata: metaparse: Better array parsing.
  metadata: metaparse: Ignore ',' in array inside parenthesis
  metadata: metaparse: Add a few pre-defined macros
  metadata: parse.sh: Add -Itestcases/kernel/include

 metadata/data_storage.h |  53 ++++++++++-
 metadata/metaparse.c    | 195 ++++++++++++++++++++++++++++++++++------
 metadata/parse.sh       |   2 +-
 3 files changed, 221 insertions(+), 29 deletions(-)

-- 
2.45.2


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

[LTP] [PATCH 1/9] metadata: metaparse: Parse operators *, +, and | properly

[Cyril Hrubis]

This is needed for cases where we assing integer values to tst_test
structure members such as:

TST_SR_SKIP_MISSING|TST_SR_TCONF flags in .save_restore
(1*TST_MB) in .min_swap_avail
[N+1, "TST_NEEDS"] in .hugepages

etc.

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

diff --git a/metadata/metaparse.c b/metadata/metaparse.c
index 2384c73c8..bf9559712 100644
--- a/metadata/metaparse.c
+++ b/metadata/metaparse.c
@@ -173,6 +173,9 @@ static char *next_token2(FILE *f, char *buf, size_t buf_len, struct data_node *d
 		case '[':
 		case ']':
 		case '#':
+		case '|':
+		case '+':
+		case '*':
 			if (i) {
 				ungetc(c, f);
 				goto exit;
-- 
2.45.2


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

[LTP] [PATCH 2/9] metadata: metaparse: Implement recursive include

[Cyril Hrubis]

This is really needed in order to expand all relevant variables in the
tst_test structure. For example PATH_KSM is hidden in
kernel/include/ksm_helper.h which is included from
kernel/mem/include/mem.h which is included by ksm0*.c testcases.

We also have a list of headers to explicitly skip, that define many
macros that are not useful in tst_test structure expansion.

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

diff --git a/metadata/metaparse.c b/metadata/metaparse.c
index bf9559712..486475780 100644
--- a/metadata/metaparse.c
+++ b/metadata/metaparse.c
@@ -237,6 +237,20 @@ static FILE *open_file(const char *dir, const char *fname)
 	return f;
 }
 
+/**
+ * List of includes to be skipped.
+ *
+ * These define many macros or include many include files that are mostly
+ * useless to values expanded in tst_test structure. Or macros that shouldn't
+ * be expanded at all.
+ */
+static const char *skip_includes[] = {
+	"\"tst_test.h\"",
+	"\"config.h\"",
+	"\"tst_taint.h\"",
+	NULL
+};
+
 static FILE *open_include(FILE *f)
 {
 	char buf[256], *fname;
@@ -249,6 +263,20 @@ static FILE *open_include(FILE *f)
 	if (buf[0] != '"')
 		return NULL;
 
+	for (i = 0; skip_includes[i]; i++) {
+		if (!strcmp(skip_includes[i], buf)) {
+			if (verbose)
+				fprintf(stderr, "INCLUDE SKIP %s\n", buf);
+			return NULL;
+		}
+	}
+
+	if (!strncmp(buf, "\"lapi/", 6)) {
+		if (verbose)
+			fprintf(stderr, "INCLUDE SKIP %s\n", buf);
+		return NULL;
+	}
+
 	fname = buf + 1;
 
 	if (!buf[0])
@@ -641,12 +669,20 @@ static void parse_macro(FILE *f)
 	hsearch(e, ENTER);
 }
 
-static void parse_include_macros(FILE *f)
+static void parse_include_macros(FILE *f, int level)
 {
 	FILE *inc;
 	const char *token;
 	int hash = 0;
 
+	/**
+	 * Allow only three levels of include indirection.
+	 *
+	 * Should be more than enough (TM).
+	 */
+	if (level >= 3)
+		return;
+
 	inc = open_include(f);
 	if (!inc)
 		return;
@@ -662,6 +698,8 @@ static void parse_include_macros(FILE *f)
 
 		if (!strcmp(token, "define"))
 			parse_macro(inc);
+		else if (!strcmp(token, "include"))
+			parse_include_macros(inc, level+1);
 
 		hash = 0;
 	}
@@ -697,7 +735,7 @@ static struct data_node *parse_file(const char *fname)
 						parse_macro(f);
 
 					if (!strcmp(token, "include"))
-						parse_include_macros(f);
+						parse_include_macros(f, 0);
 				}
 			}
 
-- 
2.45.2


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

[LTP] [PATCH 3/9] metadata: metaparse: Apply macros in arrays.

[Cyril Hrubis]

This fixes many cases mostly paths in save_restore:

...
-      "INTEGER_PROCFILE",
+      "/proc/sys/fs/pipe-max-size",
...
    "needs_drivers": [
-     "HW_MODULE"
+     "hwpoison_inject"
     ],
...
    "save_restore": [
      [
-      "CORE_PATTERN",
+      "/proc/sys/kernel/core_pattern",
...

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

diff --git a/metadata/metaparse.c b/metadata/metaparse.c
index 486475780..969293cfc 100644
--- a/metadata/metaparse.c
+++ b/metadata/metaparse.c
@@ -317,9 +317,28 @@ static void close_include(FILE *inc)
 	fclose(inc);
 }
 
+static void try_apply_macro(char **res)
+{
+	ENTRY macro = {
+		.key = *res,
+	};
+
+	ENTRY *ret;
+
+	ret = hsearch(macro, FIND);
+
+	if (!ret)
+		return;
+
+	if (verbose)
+		fprintf(stderr, "APPLYING MACRO %s=%s\n", ret->key, (char*)ret->data);
+
+	*res = ret->data;
+}
+
 static int parse_array(FILE *f, struct data_node *node)
 {
-	const char *token;
+	char *token;
 
 	for (;;) {
 		if (!(token = next_token(f, NULL)))
@@ -346,6 +365,8 @@ static int parse_array(FILE *f, struct data_node *node)
 		if (!strcmp(token, "NULL"))
 			continue;
 
+		try_apply_macro(&token);
+
 		struct data_node *str = data_node_string(token);
 
 		data_node_array_add(node, str);
@@ -354,25 +375,6 @@ static int parse_array(FILE *f, struct data_node *node)
 	return 0;
 }
 
-static void try_apply_macro(char **res)
-{
-	ENTRY macro = {
-		.key = *res,
-	};
-
-	ENTRY *ret;
-
-	ret = hsearch(macro, FIND);
-
-	if (!ret)
-		return;
-
-	if (verbose)
-		fprintf(stderr, "APPLYING MACRO %s=%s\n", ret->key, (char*)ret->data);
-
-	*res = ret->data;
-}
-
 static int parse_get_array_len(FILE *f)
 {
 	const char *token;
-- 
2.45.2


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

[LTP] [PATCH 4/9] metadata: data_storage: Add JSON null type

[Cyril Hrubis]

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

diff --git a/metadata/data_storage.h b/metadata/data_storage.h
index 91ea70a02..e72be6187 100644
--- a/metadata/data_storage.h
+++ b/metadata/data_storage.h
@@ -16,6 +16,7 @@ enum data_type {
 	DATA_HASH,
 	DATA_STRING,
 	DATA_INT,
+	DATA_NULL,
 };
 
 struct data_node_array {
@@ -68,6 +69,8 @@ static inline const char* data_type_name(enum data_type type)
 		return "string";
 	case DATA_INT:
 		return "int";
+	case DATA_NULL:
+		return "null";
 	default:
 		return "???";
 	}
@@ -100,6 +103,18 @@ static inline struct data_node *data_node_int(long i)
 	return node;
 }
 
+static inline struct data_node *data_node_null(void)
+{
+	struct data_node *node = malloc(sizeof(struct data_node));
+
+	if (!node)
+		return NULL;
+
+	node->type = DATA_NULL;
+
+	return node;
+}
+
 #define MAX_ELEMS 100
 
 static inline struct data_node *data_node_hash(void)
@@ -159,6 +174,7 @@ static inline void data_node_free(struct data_node *self)
 	switch (self->type) {
 	case DATA_STRING:
 	case DATA_INT:
+	case DATA_NULL:
 	break;
 	case DATA_HASH:
 		for (i = 0; i < self->hash.elems_used; i++) {
@@ -254,6 +270,10 @@ static inline void data_node_print_(struct data_node *self, unsigned int padd)
 		data_print_padd(padd);
 		printf("'%s'\n", self->string.val);
 	break;
+	case DATA_NULL:
+		data_print_padd(padd);
+		printf("null\n");
+	break;
 	case DATA_HASH:
 		for (i = 0; i < self->hash.elems_used; i++) {
 			data_print_padd(padd);
@@ -297,7 +317,6 @@ static inline void data_fprintf(FILE *f, unsigned int padd, const char *fmt, ...
 	va_end(va);
 }
 
-
 static inline void data_fprintf_esc(FILE *f, unsigned int padd, const char *str)
 {
 	while (padd-- > 0)
@@ -344,6 +363,10 @@ static inline void data_to_json_(struct data_node *self, FILE *f, unsigned int p
 		padd = do_padd ? padd : 0;
 		data_fprintf_esc(f, padd, self->string.val);
 	break;
+	case DATA_NULL:
+		padd = do_padd ? padd : 0;
+		data_fprintf(f, padd, "null");
+	break;
 	case DATA_HASH:
 		for (i = 0; i < self->hash.elems_used; i++) {
 			data_fprintf(f, padd, "\"%s\": ", self->hash.elems[i].id);
-- 
2.45.2


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

[LTP] [PATCH 5/9] metadata: data_storage: Add two array functions

[Cyril Hrubis]

- data_node_array_last() returns last element in an array or NULL if empty

- data_node_array_last_rem() removes and frees last element from an array

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

diff --git a/metadata/data_storage.h b/metadata/data_storage.h
index e72be6187..6427ea1ed 100644
--- a/metadata/data_storage.h
+++ b/metadata/data_storage.h
@@ -251,6 +251,34 @@ static inline unsigned int data_node_array_len(struct data_node *self)
 	return self->array.array_used;
 }
 
+
+static inline struct data_node *data_node_array_last(struct data_node *self)
+{
+	if (self->type != DATA_ARRAY)
+		return NULL;
+
+	unsigned int array_used = self->array.array_used;
+	if (!array_used)
+		return NULL;
+
+	return self->array.array[array_used-1];
+}
+
+static inline void data_node_array_last_rem(struct data_node *self)
+{
+	if (self->type != DATA_ARRAY)
+		return;
+
+	unsigned int array_used = self->array.array_used;
+	if (!array_used)
+		return;
+
+	data_node_free(self->array.array[array_used-1]);
+
+	self->array.array[array_used-1] = NULL;
+	self->array.array_used--;
+}
+
 static inline void data_print_padd(unsigned int i)
 {
 	while (i-- > 0)
-- 
2.45.2


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

[LTP] [PATCH 6/9] metadata: metaparse: Better array parsing.

[Cyril Hrubis]

Changes the array parser to concatenate array mebers i.e. everything
between ',' is concatenated to a single string.

We also convert NULL in the middle anonymous structure initialization
into JSON null. With this the save_restore generates a proper arrays.

This fixes a few problems such as:

...
    "hugepages": [
-     "(",
-     "50",
-     "+"
-     "1",
-     ")",
-     "*",
-     "5",
+     "(50+1)*5",
      "TST_NEEDS"
     ],
...
    "save_restore": [
      [
       "PATH_OC_HPAGES",
+      null,
       "TST_SR_TCONF"
      ]
...

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

diff --git a/metadata/metaparse.c b/metadata/metaparse.c
index 969293cfc..f133a65d6 100644
--- a/metadata/metaparse.c
+++ b/metadata/metaparse.c
@@ -336,9 +336,42 @@ static void try_apply_macro(char **res)
 	*res = ret->data;
 }
 
+static void finalize_array_entry(char **entry, struct data_node *node)
+{
+	if (!*entry)
+		return;
+
+	data_node_array_add(node, data_node_string(*entry));
+
+	free(*entry);
+	*entry = NULL;
+}
+
+static void str_append(char **res, char *append)
+{
+	char *cur_str = *res;
+
+	if (!cur_str) {
+		*res = strdup(append);
+		if (!*res)
+			goto err;
+		return;
+	}
+
+	if (asprintf(res, "%s%s", cur_str, append) < 0)
+		goto err;
+
+	free(cur_str);
+	return;
+err:
+	fprintf(stderr, "Allocation failed :(\n");
+	exit(1);
+}
+
 static int parse_array(FILE *f, struct data_node *node)
 {
 	char *token;
+	char *entry = NULL;
 
 	for (;;) {
 		if (!(token = next_token(f, NULL)))
@@ -356,20 +389,31 @@ static int parse_array(FILE *f, struct data_node *node)
 			continue;
 		}
 
-		if (!strcmp(token, "}"))
+		if (!strcmp(token, "}")) {
+			struct data_node *arr_last;
+
+			finalize_array_entry(&entry, node);
+
+			/* Remove NULL terminating entry, if present. */
+			arr_last = data_node_array_last(node);
+			if (arr_last && arr_last->type == DATA_NULL)
+				data_node_array_last_rem(node);
+
 			return 0;
+		}
 
-		if (!strcmp(token, ","))
+		if (!strcmp(token, ",")) {
+			finalize_array_entry(&entry, node);
 			continue;
+		}
 
-		if (!strcmp(token, "NULL"))
+		if (!strcmp(token, "NULL")) {
+			data_node_array_add(node, data_node_null());
 			continue;
+		}
 
 		try_apply_macro(&token);
-
-		struct data_node *str = data_node_string(token);
-
-		data_node_array_add(node, str);
+		str_append(&entry, token);
 	}
 
 	return 0;
-- 
2.45.2


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

[LTP] [PATCH 7/9] metadata: metaparse: Ignore ', ' in array inside parenthesis

[Cyril Hrubis]

This fixes the TST_CAP() macros:

    "caps": [
-     "TST_CAP("TST_CAP_DROP",
-     "CAP_NET_RAW")"
+     "TST_CAP(TST_CAP_DROP,CAP_NET_RAW)"
     ],

Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
---
 metadata/metaparse.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/metadata/metaparse.c b/metadata/metaparse.c
index f133a65d6..29c03e5c3 100644
--- a/metadata/metaparse.c
+++ b/metadata/metaparse.c
@@ -372,6 +372,7 @@ static int parse_array(FILE *f, struct data_node *node)
 {
 	char *token;
 	char *entry = NULL;
+	int parent_cnt = 0;
 
 	for (;;) {
 		if (!(token = next_token(f, NULL)))
@@ -402,7 +403,7 @@ static int parse_array(FILE *f, struct data_node *node)
 			return 0;
 		}
 
-		if (!strcmp(token, ",")) {
+		if (!strcmp(token, ",") && parent_cnt <= 0) {
 			finalize_array_entry(&entry, node);
 			continue;
 		}
@@ -412,6 +413,12 @@ static int parse_array(FILE *f, struct data_node *node)
 			continue;
 		}
 
+		if (!strcmp(token, "("))
+			parent_cnt++;
+
+		if (!strcmp(token, ")"))
+			parent_cnt--;
+
 		try_apply_macro(&token);
 		str_append(&entry, token);
 	}
-- 
2.45.2


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

[LTP] [PATCH 8/9] metadata: metaparse: Add a few pre-defined macros

[Cyril Hrubis]

This is used mostly for stripping macro prefixes such as:

...
    "save_restore": [
      [
       "/proc/sys/user/max_user_namespaces",
       null,
-      "TST_SR_SKIP"
+      "SKIP"
      ],
...
-   "needs_cgroup_ver": "TST_CG_V2",
+   "needs_cgroup_ver": "2",
...

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

diff --git a/metadata/metaparse.c b/metadata/metaparse.c
index 29c03e5c3..fa30be727 100644
--- a/metadata/metaparse.c
+++ b/metadata/metaparse.c
@@ -760,6 +760,51 @@ static void parse_include_macros(FILE *f, int level)
 	close_include(inc);
 }
 
+/* pre-defined macros that makes the output cleaner. */
+static const struct macro {
+	char *from;
+	char *to;
+} internal_macros[] = {
+	{"TST_CG_V2", "2"},
+	{"TST_CG_V1", "1"},
+	{"TST_KB", "1024"},
+	{"TST_MB", "1048576"},
+	{"TST_GB", "1073741824"},
+	{"TST_SR_TBROK", "TBROK"},
+	{"TST_SR_TCONF", "TCONF"},
+	{"TST_SR_SKIP", "SKIP"},
+	{"TST_SR_TBROK_MISSING", "TBROK_MISSING"},
+	{"TST_SR_TCONF_MISSING", "TCONF_MISSING"},
+	{"TST_SR_SKIP_MISSING", "SKIP_MISSING"},
+	{"TST_SR_TBROK_RO", "TBROK_RO"},
+	{"TST_SR_TCONF_RO", "TCONF_RO"},
+	{"TST_SR_SKIP_RO", "SKIP_RO"},
+	{}
+};
+
+static void load_internal_macros(void)
+{
+	unsigned int i;
+
+	if (verbose)
+		fprintf(stderr, "PREDEFINED MACROS\n");
+
+	for (i = 0; internal_macros[i].from; i++) {
+		ENTRY e = {
+			.key = internal_macros[i].from,
+			.data = internal_macros[i].to,
+		};
+
+		if (verbose)
+			fprintf(stderr, " MACRO %s=%s\n", e.key, (char*)e.data);
+
+		hsearch(e, ENTER);
+	}
+
+	if (verbose)
+		fprintf(stderr, "END PREDEFINED MACROS\n");
+}
+
 static struct data_node *parse_file(const char *fname)
 {
 	int state = 0, found = 0;
@@ -777,6 +822,8 @@ static struct data_node *parse_file(const char *fname)
 	struct data_node *res = data_node_hash();
 	struct data_node *doc = data_node_array();
 
+	load_internal_macros();
+
 	while ((token = next_token(f, doc))) {
 		if (state < 6 && !strcmp(tokens[state], token)) {
 			state++;
-- 
2.45.2


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

[LTP] [PATCH 9/9] metadata: parse.sh: Add -Itestcases/kernel/include

[Cyril Hrubis]

This does not completely fix macro expansions without the patchset I've
send that removes the mess in testcases/kernel/mem/{lib,include}/
library because before the patchset some headers e.g. the ksm_helper.h
was included indirectly from the testcases/kernel/mem/include/mem.h
header.

Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
---
 metadata/parse.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/metadata/parse.sh b/metadata/parse.sh
index 69bf5db65..7db2e2415 100755
--- a/metadata/parse.sh
+++ b/metadata/parse.sh
@@ -30,7 +30,7 @@ echo ' "tests": {'
 first=1
 
 for test in `find testcases/ -name '*.c'|sort`; do
-	a=$($top_builddir/metadata/metaparse -Iinclude -Itestcases/kernel/syscalls/utils/ "$test")
+	a=$($top_builddir/metadata/metaparse -Iinclude -Itestcases/kernel/syscalls/utils/ -Itestcases/kernel/include "$test")
 	if [ -n "$a" ]; then
 		if [ -z "$first" ]; then
 			echo ','
-- 
2.45.2


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

Re: [LTP] [PATCH 1/9] metadata: metaparse: Parse operators *, +, and | properly

[Petr Vorel]

HI Cyril,

> This is needed for cases where we assing integer values to tst_test
> structure members such as:

> TST_SR_SKIP_MISSING|TST_SR_TCONF flags in .save_restore
> (1*TST_MB) in .min_swap_avail
> [N+1, "TST_NEEDS"] in .hugepages

Thanks!

...
> +++ b/metadata/metaparse.c
> @@ -173,6 +173,9 @@ static char *next_token2(FILE *f, char *buf, size_t buf_len, struct data_node *d
>  		case '[':
>  		case ']':
>  		case '#':
> +		case '|':
> +		case '+':
> +		case '*':

I suppose we don't need '/' and '%' right?

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

Kind regards,
Petr

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

Re: [LTP] [PATCH 2/9] metadata: metaparse: Implement recursive include

[Petr Vorel]

Hi Cyril,

> This is really needed in order to expand all relevant variables in the
> tst_test structure. For example PATH_KSM is hidden in
> kernel/include/ksm_helper.h which is included from
> kernel/mem/include/mem.h which is included by ksm0*.c testcases.

> We also have a list of headers to explicitly skip, that define many
> macros that are not useful in tst_test structure expansion.

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

Kind regards,
Petr

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

Re: [LTP] [PATCH 3/9] metadata: metaparse: Apply macros in arrays.

[Petr Vorel]

Hi Cyril,

+1

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

Kind regards,
Petr

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

Re: [LTP] [PATCH 4/9] metadata: data_storage: Add JSON null type

[Petr Vorel]

Hi Cyril,

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

Kind regards,
Petr

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

Re: [LTP] [PATCH 5/9] metadata: data_storage: Add two array functions

[Petr Vorel]

Hi Cyril,

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

> - data_node_array_last() returns last element in an array or NULL if empty

> - data_node_array_last_rem() removes and frees last element from an array
Would you mind to add the above as /** */ doc to the functions before merging?
(no need to repost).

Kind regards,
Petr

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

Re: [LTP] [PATCH 1/9] metadata: metaparse: Parse operators *, +, and | properly

[Cyril Hrubis]

Hi!
> > +++ b/metadata/metaparse.c
> > @@ -173,6 +173,9 @@ static char *next_token2(FILE *f, char *buf, size_t buf_len, struct data_node *d
> >  		case '[':
> >  		case ']':
> >  		case '#':
> > +		case '|':
> > +		case '+':
> > +		case '*':
> 
> I suppose we don't need '/' and '%' right?

I guess that it wouldn't harm, these does not seem to be used now, but
someone may do so in the future.

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH 5/9] metadata: data_storage: Add two array functions

[Cyril Hrubis]

Hi!
> > - data_node_array_last() returns last element in an array or NULL if empty
> 
> > - data_node_array_last_rem() removes and frees last element from an array
> Would you mind to add the above as /** */ doc to the functions before merging?
> (no need to repost).

The whole file needs documentation, I will add that to my ever growing
TODO.

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH 6/9] metadata: metaparse: Better array parsing.

[Petr Vorel]

Hi Cyril,

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

Kind regards,
Petr

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

Re: [LTP] [PATCH 7/9] metadata: metaparse: Ignore ', ' in array inside parenthesis

[Petr Vorel]

Hi Cyril,

> This fixes the TST_CAP() macros:

>     "caps": [
> -     "TST_CAP("TST_CAP_DROP",
> -     "CAP_NET_RAW")"
> +     "TST_CAP(TST_CAP_DROP,CAP_NET_RAW)"
>      ],

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

Kind regards,
Petr

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

Re: [LTP] [PATCH 8/9] metadata: metaparse: Add a few pre-defined macros

[Petr Vorel]

Hi Cyril,

> This is used mostly for stripping macro prefixes such as:

> ...
>     "save_restore": [
>       [
>        "/proc/sys/user/max_user_namespaces",
>        null,
> -      "TST_SR_SKIP"
> +      "SKIP"
>       ],
> ...
> -   "needs_cgroup_ver": "TST_CG_V2",
> +   "needs_cgroup_ver": "2",
> ...

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

> diff --git a/metadata/metaparse.c b/metadata/metaparse.c
> index 29c03e5c3..fa30be727 100644
> --- a/metadata/metaparse.c
> +++ b/metadata/metaparse.c
> @@ -760,6 +760,51 @@ static void parse_include_macros(FILE *f, int level)
>  	close_include(inc);
>  }

> +/* pre-defined macros that makes the output cleaner. */
> +static const struct macro {
> +	char *from;
> +	char *to;
> +} internal_macros[] = {
> +	{"TST_CG_V2", "2"},
> +	{"TST_CG_V1", "1"},
> +	{"TST_KB", "1024"},
> +	{"TST_MB", "1048576"},
> +	{"TST_GB", "1073741824"},
I guess we need a number for calculation (e.g. 1 * TST_GB)
Otherwise 1 GB would be more readable than a big number.

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

Kind regards,
Petr

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

Re: [LTP] [PATCH 9/9] metadata: parse.sh: Add -Itestcases/kernel/include

[Petr Vorel]

Hi Cyril,

> This does not completely fix macro expansions without the patchset I've
> send that removes the mess in testcases/kernel/mem/{lib,include}/
> library because before the patchset some headers e.g. the ksm_helper.h
> was included indirectly from the testcases/kernel/mem/include/mem.h
> header.

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

I'll have look on the other patchset tomorrow.

Kind regards,
Petr

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

Re: [LTP] [PATCH 8/9] metadata: metaparse: Add a few pre-defined macros

[Cyril Hrubis]

Hi!
> > +/* pre-defined macros that makes the output cleaner. */
> > +static const struct macro {
> > +	char *from;
> > +	char *to;
> > +} internal_macros[] = {
> > +	{"TST_CG_V2", "2"},
> > +	{"TST_CG_V1", "1"},
> > +	{"TST_KB", "1024"},
> > +	{"TST_MB", "1048576"},
> > +	{"TST_GB", "1073741824"},
> I guess we need a number for calculation (e.g. 1 * TST_GB)
> Otherwise 1 GB would be more readable than a big number.

The metadata are supposed to be read by a computer, so indeed the number
is better. I guess that we can convert it to something more sensible in
the perl script that generates the html pages.

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH 8/9] metadata: metaparse: Add a few pre-defined macros

[Petr Vorel]

> Hi!
> > > +/* pre-defined macros that makes the output cleaner. */
> > > +static const struct macro {
> > > +	char *from;
> > > +	char *to;
> > > +} internal_macros[] = {
> > > +	{"TST_CG_V2", "2"},
> > > +	{"TST_CG_V1", "1"},
> > > +	{"TST_KB", "1024"},
> > > +	{"TST_MB", "1048576"},
> > > +	{"TST_GB", "1073741824"},
> > I guess we need a number for calculation (e.g. 1 * TST_GB)
> > Otherwise 1 GB would be more readable than a big number.

> The metadata are supposed to be read by a computer, so indeed the number
> is better. I guess that we can convert it to something more sensible in
> the perl script that generates the html pages.

+1 makes sense, I'll try to have look on it once it's merged.

Kind regards,
Petr

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

Re: [LTP] [PATCH 1/9] metadata: metaparse: Parse operators *, +, and | properly

[Cyril Hrubis]

Hi!
> I suppose we don't need '/' and '%' right?

So I've added only % because / would need a bigger surgery and pushed
the patchset. Thanks for the review.

-- 
Cyril Hrubis
chrubis@suse.cz

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