From: Cyril Hrubis <chrubis@suse.cz>
To: Andrea Cervesato <andrea.cervesato@suse.de>
Cc: Linux Test Project <ltp@lists.linux.it>
Subject: Re: [LTP] [PATCH v2 1/2] metadata: add tests grouping support
Date: Fri, 19 Jun 2026 10:40:28 +0200 [thread overview]
Message-ID: <ajUAfFOaQgGJejCW@yuki.lan> (raw)
In-Reply-To: <20260618-metadata_groups-v2-1-6ab298932233@suse.com>
Hi!
> Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
> ---
> metadata/metaparse.c | 159 +++++++++++++++++++++++++++++++++++--------
> metadata/tests/groups.c | 11 +++
> metadata/tests/groups.c.json | 12 ++++
> 3 files changed, 152 insertions(+), 30 deletions(-)
>
> diff --git a/metadata/metaparse.c b/metadata/metaparse.c
> index 561cbb9d2d54689988c9aa49d591628696bcf847..94e795c275aebda41737558d06f7d9c8c1a2f4f7 100644
> --- a/metadata/metaparse.c
> +++ b/metadata/metaparse.c
> @@ -17,6 +17,7 @@
> #include "data_storage.h"
>
> #define INCLUDE_PATH_MAX 5
> +#define GROUPS_TAG "groups"
>
> static int verbose;
> static char *cmdline_includepath[INCLUDE_PATH_MAX];
> @@ -50,7 +51,49 @@ static const char *eat_asterisk_space(const char *c)
> return c;
> }
>
> -static void multiline_comment(FILE *f, struct data_node *doc)
> +/*
> + * Add a group to the groups array, skipping 'kernel' as it's too generic.
> + * Returns 0 if no group was added, 1 otherwise.
> + */
> +static int add_group(struct data_node *groups, const char *name)
> +{
> + if (name && strcmp(name, "kernel")) {
> + data_node_array_add(groups, data_node_string(name));
> + return 1;
> + }
> +
> + return 0;
> +}
> +
> +/*
> + * Parse a '@groups foo bar baz' doc comment line, adding each
> + * whitespace-separated name to the groups array. Returns 1 if the line
> + * was a @groups tag (and should be consumed), 0 otherwise.
> + */
> +static int parse_groups(struct data_node *groups, char *line)
> +{
> + char *s;
> + char *name;
> +
> + if (!groups)
> + return 0;
> +
> + s = (char *)eat_asterisk_space(line);
It's way cleaner to drop the const from the eat_asterisk_space() fuction
than this ugly cast.
> + if (strncmp(s, GROUPS_TAG, sizeof(GROUPS_TAG) - 1))
> + return 0;
> +
> + s += sizeof(GROUPS_TAG) - 1;
> + if (*s && *s != ' ' && *s != '\t')
> + return 0;
I guess that we should WARN() here about empty groups tag and return 1
so that it's "consumed".
> + for (name = strtok(s, " \t"); name; name = strtok(NULL, " \t"))
> + add_group(groups, name);
> +
> + return 1;
> +}
> +
> +static void multiline_comment(FILE *f, struct data_node *doc,
> + struct data_node *groups)
> {
> int c;
> int state = 0;
> @@ -62,12 +105,14 @@ static void multiline_comment(FILE *f, struct data_node *doc)
>
> if (doc) {
> if (c == '\n') {
> - struct data_node *line;
> + char *str;
> buf[bufp] = 0;
> - line = data_node_string(eat_asterisk_space(buf));
> - if (data_node_array_add(doc, line))
> - WARN("doc string comment truncated");
> + str = (char *)eat_asterisk_space(buf);
> bufp = 0;
> + if (parse_groups(groups, str))
> + continue;
> + if (data_node_array_add(doc, data_node_string(str)))
> + WARN("doc string comment truncated");
> continue;
> }
>
> @@ -100,7 +145,8 @@ static void multiline_comment(FILE *f, struct data_node *doc)
>
> static const char doc_prefix[] = "\\\n";
>
> -static void maybe_doc_comment(FILE *f, struct data_node *doc)
> +static void maybe_doc_comment(FILE *f, struct data_node *doc,
> + struct data_node *groups)
> {
> int c, i;
>
> @@ -113,14 +159,15 @@ static void maybe_doc_comment(FILE *f, struct data_node *doc)
> if (c == '*')
> ungetc(c, f);
>
> - multiline_comment(f, NULL);
> + multiline_comment(f, NULL, NULL);
> return;
> }
>
> - multiline_comment(f, doc);
> + multiline_comment(f, doc, groups);
> }
>
> -static void maybe_comment(FILE *f, struct data_node *doc)
> +static void maybe_comment(FILE *f, struct data_node *doc,
> + struct data_node *groups)
> {
> int c = getc(f);
>
> @@ -129,7 +176,7 @@ static void maybe_comment(FILE *f, struct data_node *doc)
> remove_to_newline(f);
> break;
> case '*':
> - maybe_doc_comment(f, doc);
> + maybe_doc_comment(f, doc, groups);
> break;
> default:
> ungetc(c, f);
> @@ -137,7 +184,8 @@ static void maybe_comment(FILE *f, struct data_node *doc)
> }
> }
>
> -static char *next_token2(FILE *f, char *buf, size_t buf_len, struct data_node *doc)
> +static char *next_token2(FILE *f, char *buf, size_t buf_len,
> + struct data_node *doc, struct data_node *groups)
> {
> size_t i = 0;
> int c;
> @@ -194,7 +242,7 @@ static char *next_token2(FILE *f, char *buf, size_t buf_len, struct data_node *d
> buf[i++] = c;
> break;
> case '/':
> - maybe_comment(f, doc);
> + maybe_comment(f, doc, groups);
> break;
> case '"':
> in_str = 1;
> @@ -216,11 +264,11 @@ exit:
> return buf;
> }
>
> -static char *next_token(FILE *f, struct data_node *doc)
> +static char *next_token(FILE *f, struct data_node *doc, struct data_node *groups)
> {
> static char buf[4096];
>
> - return next_token2(f, buf, sizeof(buf), doc);
> + return next_token2(f, buf, sizeof(buf), doc, groups);
> }
>
> static FILE *open_file(const char *dir, const char *fname)
> @@ -383,7 +431,7 @@ static int array_is_hash(FILE *f)
> int in_id = 1;
> char *token;
>
> - while ((token = next_token(f, NULL))) {
> + while ((token = next_token(f, NULL, NULL))) {
>
> if (!strcmp(token, "}")) {
> if (in_id && !comma_last)
> @@ -402,7 +450,7 @@ static int array_is_hash(FILE *f)
> int level = 1;
>
> for (;;) {
> - token = next_token(f, NULL);
> + token = next_token(f, NULL, NULL);
>
> if (!token)
> goto ret;
> @@ -453,7 +501,7 @@ static int parse_array(FILE *f, const char *arr_id, struct data_node **ret)
> *ret = data_node_array();
>
> for (;;) {
> - if (!(token = next_token(f, NULL)))
> + if (!(token = next_token(f, NULL, NULL)))
> return 1;
>
> if (!strcmp(token, "{")) {
> @@ -529,14 +577,14 @@ static int parse_get_array_len(FILE *f)
> const char *token;
> int cnt = 0, depth = 0, prev_comma = 0;
>
> - if (!(token = next_token(f, NULL)))
> + if (!(token = next_token(f, NULL, NULL)))
> return 0;
>
> if (strcmp(token, "{"))
> return 0;
>
> for (;;) {
> - if (!(token = next_token(f, NULL)))
> + if (!(token = next_token(f, NULL, NULL)))
> return 0;
>
> if (!strcmp(token, "{"))
> @@ -565,7 +613,7 @@ static void look_for_array_size(FILE *f, const char *arr_id, struct data_node **
> int prev_buf = 1;
>
> for (;;) {
> - if (!(token = next_token2(f, buf[cur_buf], sizeof(buf[cur_buf]), NULL)))
> + if (!(token = next_token2(f, buf[cur_buf], sizeof(buf[cur_buf]), NULL, NULL)))
> break;
>
> if (!strcmp(token, "=") && !strcmp(buf[prev_buf], arr_id)) {
> @@ -595,13 +643,13 @@ static int parse_array_size(FILE *f, struct data_node **res)
>
> *res = NULL;
>
> - if (!(token = next_token(f, NULL)))
> + if (!(token = next_token(f, NULL, NULL)))
> return 1;
>
> if (strcmp(token, "("))
> return 1;
>
> - if (!(token = next_token(f, NULL)))
> + if (!(token = next_token(f, NULL, NULL)))
> return 1;
>
> arr_id = strdup(token);
> @@ -621,7 +669,7 @@ static int parse_array_size(FILE *f, struct data_node **res)
> rewind(f);
>
> for (;;) {
> - if (!(token = next_token(f, NULL)))
> + if (!(token = next_token(f, NULL, NULL)))
> break;
>
> if (token[0] == '#') {
> @@ -654,7 +702,8 @@ static int parse_array_size(FILE *f, struct data_node **res)
> return 0;
> }
>
> -static int parse_test_struct(FILE *f, struct data_node *doc, struct data_node *node)
> +static int parse_test_struct(FILE *f, struct data_node *doc,
> + struct data_node *groups, struct data_node *node)
> {
> char *token;
> char *id = NULL;
> @@ -662,7 +711,7 @@ static int parse_test_struct(FILE *f, struct data_node *doc, struct data_node *n
> struct data_node *ret;
>
> for (;;) {
> - if (!(token = next_token(f, doc)))
> + if (!(token = next_token(f, doc, groups)))
> return 1;
>
> if (!strcmp(token, "}"))
> @@ -842,7 +891,7 @@ static void parse_include_macros(FILE *f, int level)
> if (!inc)
> return;
>
> - while ((token = next_token(inc, NULL))) {
> + while ((token = next_token(inc, NULL, NULL))) {
> if (token[0] == '#') {
> hash = 1;
> continue;
> @@ -907,6 +956,47 @@ static void load_internal_macros(void)
> fprintf(stderr, "END PREDEFINED MACROS\n");
> }
>
> +/*
> + * Add groups derived from the source file path.
> + *
> + * Groups are the two nearest parent directories (immediate parent
> + * first), skipping 'kernel' as it's too generic:
> + *
> + * testcases/kernel/syscalls/clone/clone01.c -> clone, syscalls
> + * testcases/kernel/kvm/kvm_pagefault01.c -> kvm
> + * testcases/cve/cve-2017-16939.c -> cve
> + */
> +static void add_path_groups(struct data_node *groups, const char *fname)
> +{
> + char *buf;
> + char *dirs[8];
> + int ndirs = 0;
> + char *p;
> +
> + if (strncmp(fname, "testcases/", 10))
> + return;
> +
> + buf = strdup(fname + 10);
> + if (!buf)
> + return;
We should do fprintf(stderr, "Allocation failed!\n"); exit(1); here
instead of silently dropping the tags if the strdup failed.
Maybe we should add ERR() macro that does print a message and calls
exit(1) since we have that pattern in many places, but that is a
completely unrelated refactor.
> + p = strtok(buf, "/");
> + while (p && ndirs < 8) {
> + dirs[ndirs++] = p;
> + p = strtok(NULL, "/");
> + }
> +
> + /* Last element is the filename, skip it */
> + ndirs--;
> +
> + if (ndirs >= 1)
> + add_group(groups, dirs[ndirs - 1]);
> + if (ndirs >= 2)
> + add_group(groups, dirs[ndirs - 2]);
> +
> + free(buf);
> +}
> +
> static struct data_node *parse_file(const char *fname)
> {
> int state = 0, found = 0;
> @@ -923,15 +1013,18 @@ static struct data_node *parse_file(const char *fname)
>
> struct data_node *res = data_node_hash();
> struct data_node *doc = data_node_array();
> + struct data_node *groups = data_node_array();
> +
> + add_path_groups(groups, fname);
>
> load_internal_macros();
>
> - while ((token = next_token(f, doc))) {
> + while ((token = next_token(f, doc, groups))) {
> if (state < 6 && !strcmp(tokens[state], token)) {
> state++;
> } else {
> if (token[0] == '#') {
> - token = next_token(f, doc);
> + token = next_token(f, doc, groups);
> if (token) {
> if (!strcmp(token, "define"))
> parse_macro(f);
> @@ -948,7 +1041,7 @@ static struct data_node *parse_file(const char *fname)
> continue;
>
> found = 1;
> - parse_test_struct(f, doc, res);
> + parse_test_struct(f, doc, groups, res);
> }
>
> if (data_node_array_len(doc)) {
> @@ -958,6 +1051,11 @@ static struct data_node *parse_file(const char *fname)
> data_node_free(doc);
> }
>
> + if (data_node_array_len(groups))
> + data_node_hash_add(res, "groups", groups);
> + else
> + data_node_free(groups);
The fuction that adds groups from path will add at least one group, so
this can be just data_node_hash_add(res, "groups", groups).
--
Cyril Hrubis
chrubis@suse.cz
--
Mailing list info: https://lists.linux.it/listinfo/ltp
next prev parent reply other threads:[~2026-06-19 8:40 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-06-18 14:10 [LTP] [PATCH v2 0/2] Support metadata groups Andrea Cervesato
2026-06-18 14:10 ` [LTP] [PATCH v2 1/2] metadata: add tests grouping support Andrea Cervesato
2026-06-18 16:40 ` [LTP] " linuxtestproject.agent
2026-06-19 8:40 ` Cyril Hrubis [this message]
2026-06-18 14:10 ` [LTP] [PATCH v2 2/2] doc: conf.py: Show groups in test catalog Andrea Cervesato
2026-06-19 8:44 ` Cyril Hrubis
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ajUAfFOaQgGJejCW@yuki.lan \
--to=chrubis@suse.cz \
--cc=andrea.cervesato@suse.de \
--cc=ltp@lists.linux.it \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox