[PATCH i-g-t v1 2/2] runner/executor: Report command used for test exec
Gustavo Sousa
gustavo.sousa at intel.com
Thu Jul 17 12:52:48 UTC 2025
Quoting Kamil Konieczny (2025-07-16 12:31:02-03:00)
>Add printing of command line used for executing a test, when
>the log level is normal or above.
>
>Signed-off-by: Kamil Konieczny <kamil.konieczny at linux.intel.com>
>---
> runner/executor.c | 41 ++++++++++++++++++++++++++++++++++++-----
> 1 file changed, 36 insertions(+), 5 deletions(-)
>
>diff --git a/runner/executor.c b/runner/executor.c
>index 7565f2571..6ee446cdb 100644
>--- a/runner/executor.c
>+++ b/runner/executor.c
>@@ -1623,17 +1623,20 @@ static void free_test_command(struct igt_vec *arg_vec)
> igt_vec_fini(arg_vec);
> }
>
>-static void build_test_command(struct settings *settings,
>- struct job_list_entry *entry,
>- struct igt_vec *arg_vec)
>+static char *build_test_command(struct settings *settings,
>+ struct job_list_entry *entry,
>+ struct igt_vec *arg_vec)
> {
>+ char *cmd_line;
>+ size_t cmd_size;
> size_t rootlen;
> char *arg;
>
> igt_vec_init(arg_vec, sizeof(char *));
>
> rootlen = strlen(settings->test_root);
>- arg = malloc(rootlen + strlen(entry->binary) + 2);
>+ cmd_size = rootlen + strlen(entry->binary) + 2;
I think adding to cmd_size for each time we add an element to arg_vec is
a bit error prone. Maybe a more robust solution would be for cmd_size to
be calculated at the end, by iterating over elements of arg_vec.
That said, see further below that I sugest that we build the log string
in a separate function.
>+ arg = malloc(cmd_size);
> strcpy(arg, settings->test_root);
> arg[rootlen] = '/';
> strcpy(arg + rootlen + 1, entry->binary);
>@@ -1645,6 +1648,7 @@ static void build_test_command(struct settings *settings,
> size_t i;
>
> arg = strdup("--run-subtest");
>+ cmd_size += strlen(arg) + 1;
> igt_vec_push(arg_vec, &arg);
>
> if ((dynbegin = strchr(entry->subtests[0], '@')) != NULL)
>@@ -1668,25 +1672,47 @@ static void build_test_command(struct settings *settings,
> argsize += sublen + 1;
> }
>
>+ cmd_size += strlen(arg) + 1;
> igt_vec_push(arg_vec, &arg);
>
> if (dynbegin) {
> arg = strdup("--dynamic-subtest");
>+ cmd_size += strlen(arg) + 1;
> igt_vec_push(arg_vec, &arg);
>+
> arg = strdup(dynbegin + 1);
>+ cmd_size += strlen(arg) + 1;
> igt_vec_push(arg_vec, &arg);
> }
> }
>
> for (size_t i = 0; i < igt_vec_length(&settings->hook_strs); i++) {
> arg = strdup("--hook");
>+ cmd_size += strlen(arg) + 1;
> igt_vec_push(arg_vec, &arg);
> arg = strdup(*((char **)igt_vec_elem(&settings->hook_strs, i)));
>+ cmd_size += strlen(arg) + 1;
> igt_vec_push(arg_vec, &arg);
> }
>
>+ cmd_size *= 2;
>+ cmd_line = malloc(cmd_size);
>+ if (cmd_line) {
>+ char *elem;
>+ int len;
>+
>+ elem = *((char **)igt_vec_elem(arg_vec, 0));
>+ len = snprintf(cmd_line, cmd_size, "%s", elem);
>+ for (size_t i = 1; i < igt_vec_length(arg_vec) && len < cmd_size; i++) {
>+ elem = *((char **)igt_vec_elem(arg_vec, i));
>+ len += snprintf(cmd_line + len, cmd_size - len, " %s", elem);
I think this would give a broken cmdline if there are spaces in
arguments (e.g. hook strings via --hook or environment variables via
-e).
Since we are just using the generated string for logging, that's not
very problematic on our side, but it could be inconvenient for anyone
that wants to copy/paste the string into the shell.
>+ }
>+ }
>+
> arg = NULL;
> igt_vec_push(arg_vec, &arg);
>+
>+ return cmd_line;
> }
>
> static void __attribute__((noreturn))
>@@ -1788,6 +1814,7 @@ static int execute_next_entry(struct execute_state *state,
> bool *abort_already_written)
> {
> struct igt_vec arg_vec;
>+ char *cmd_line;
> int dirfd;
> int outputs[_F_LAST];
> int kmsgfd;
>@@ -1837,7 +1864,7 @@ static int execute_next_entry(struct execute_state *state,
> lseek(kmsgfd, 0, SEEK_END);
> }
>
>- build_test_command(settings, entry, &arg_vec);
>+ cmd_line = build_test_command(settings, entry, &arg_vec);
>
> if (settings->log_level >= LOG_LEVEL_NORMAL) {
> char buf[100];
>@@ -1856,6 +1883,8 @@ static int execute_next_entry(struct execute_state *state,
> free(displayname);
>
> outf("%s\n", buf);
>+ if (cmd_line)
>+ outf("Exec test: %s\n", cmd_line);
I think a cleaner solution would be to have a specific function that
takse arg_vec and builds the string to be output. Then we would call
that function here. The benefits IMO are:
* We do not overload build_test_command() to do an extra thing (i.e.
build the log string).
* The log string is built only if necessary.
--
Gustavo Sousa
> }
>
> /*
>@@ -1908,6 +1937,8 @@ static int execute_next_entry(struct execute_state *state,
> out_kmsgfd:
> close(kmsgfd);
> free_test_command(&arg_vec);
>+ if (cmd_line)
>+ free(cmd_line);
> out_pipe:
> close(outpipe[0]);
> close(outpipe[1]);
>--
>2.50.1
>
More information about the igt-dev
mailing list