[PATCH i-g-t v1 1/2] runner/executor: Build test command line before fork
Gustavo Sousa
gustavo.sousa at intel.com
Thu Jul 17 12:53:04 UTC 2025
Quoting Kamil Konieczny (2025-07-16 12:31:01-03:00)
>Create a separate test command line building in main runner
>process before forking, so it could be later processed for other
>purposes like reporting.
>
>Signed-off-by: Kamil Konieczny <kamil.konieczny at linux.intel.com>
>---
> runner/executor.c | 64 ++++++++++++++++++++++++++++++-----------------
> 1 file changed, 41 insertions(+), 23 deletions(-)
>
>diff --git a/runner/executor.c b/runner/executor.c
>index 847abe481..7565f2571 100644
>--- a/runner/executor.c
>+++ b/runner/executor.c
>@@ -1615,28 +1615,29 @@ static int monitor_output(pid_t child,
> return killed;
> }
>
>-static void __attribute__((noreturn))
>-execute_test_process(int outfd, int errfd, int socketfd,
>- struct settings *settings,
>- struct job_list_entry *entry)
>+static void free_test_command(struct igt_vec *arg_vec)
> {
>- struct igt_vec arg_vec;
>- char *arg;
>- size_t rootlen;
>+ for (size_t i = 0; i < igt_vec_length(arg_vec); i++)
>+ free(*((char **)igt_vec_elem(arg_vec, i)));
>
>- dup2(outfd, STDOUT_FILENO);
>- dup2(errfd, STDERR_FILENO);
>+ igt_vec_fini(arg_vec);
>+}
>
>- setpgid(0, 0);
>+static void build_test_command(struct settings *settings,
>+ struct job_list_entry *entry,
>+ struct igt_vec *arg_vec)
>+{
>+ size_t rootlen;
>+ char *arg;
>
>- igt_vec_init(&arg_vec, sizeof(char *));
>+ igt_vec_init(arg_vec, sizeof(char *));
>
> rootlen = strlen(settings->test_root);
> arg = malloc(rootlen + strlen(entry->binary) + 2);
> strcpy(arg, settings->test_root);
> arg[rootlen] = '/';
> strcpy(arg + rootlen + 1, entry->binary);
>- igt_vec_push(&arg_vec, &arg);
>+ igt_vec_push(arg_vec, &arg);
>
> if (entry->subtest_count) {
> size_t argsize;
>@@ -1644,7 +1645,7 @@ execute_test_process(int outfd, int errfd, int socketfd,
> size_t i;
>
> arg = strdup("--run-subtest");
>- igt_vec_push(&arg_vec, &arg);
>+ igt_vec_push(arg_vec, &arg);
>
> if ((dynbegin = strchr(entry->subtests[0], '@')) != NULL)
> argsize = dynbegin - entry->subtests[0];
>@@ -1667,35 +1668,49 @@ execute_test_process(int outfd, int errfd, int socketfd,
> argsize += sublen + 1;
> }
>
>- igt_vec_push(&arg_vec, &arg);
>+ igt_vec_push(arg_vec, &arg);
>
> if (dynbegin) {
> arg = strdup("--dynamic-subtest");
>- igt_vec_push(&arg_vec, &arg);
>+ igt_vec_push(arg_vec, &arg);
> arg = strdup(dynbegin + 1);
>- igt_vec_push(&arg_vec, &arg);
>+ igt_vec_push(arg_vec, &arg);
> }
> }
>
> for (size_t i = 0; i < igt_vec_length(&settings->hook_strs); i++) {
> arg = strdup("--hook");
>- igt_vec_push(&arg_vec, &arg);
>+ igt_vec_push(arg_vec, &arg);
> arg = strdup(*((char **)igt_vec_elem(&settings->hook_strs, i)));
>- igt_vec_push(&arg_vec, &arg);
>+ igt_vec_push(arg_vec, &arg);
> }
>
> arg = NULL;
>- igt_vec_push(&arg_vec, &arg);
>+ igt_vec_push(arg_vec, &arg);
>+}
>+
>+static void __attribute__((noreturn))
>+execute_test_process(int outfd, int errfd, int socketfd,
>+ struct settings *settings,
>+ struct job_list_entry *entry,
>+ struct igt_vec *arg_vec)
>+{
>+ char *arg;
>+
>+ dup2(outfd, STDOUT_FILENO);
>+ dup2(errfd, STDERR_FILENO);
>+
>+ setpgid(0, 0);
>
> if (socketfd >= 0) {
> struct runnerpacket *packet;
>
>- packet = runnerpacket_exec(arg_vec.elems);
>+ packet = runnerpacket_exec(arg_vec->elems);
> write(socketfd, packet, packet->size);
> }
>
>- arg = *((char **)igt_vec_elem(&arg_vec, 0));
>- execv(arg, arg_vec.elems);
>+ arg = *((char **)igt_vec_elem(arg_vec, 0));
>+ execv(arg, arg_vec->elems);
> fprintf(stderr, "Cannot execute %s\n", arg);
> exit(IGT_EXIT_INVALID);
> }
>@@ -1772,6 +1787,7 @@ static int execute_next_entry(struct execute_state *state,
> char **abortreason,
> bool *abort_already_written)
> {
>+ struct igt_vec arg_vec;
> int dirfd;
> int outputs[_F_LAST];
> int kmsgfd;
>@@ -1821,6 +1837,7 @@ static int execute_next_entry(struct execute_state *state,
> lseek(kmsgfd, 0, SEEK_END);
> }
>
>+ build_test_command(settings, entry, &arg_vec);
>
> if (settings->log_level >= LOG_LEVEL_NORMAL) {
> char buf[100];
>@@ -1871,7 +1888,7 @@ static int execute_next_entry(struct execute_state *state,
> }
> setenv("IGT_SENTINEL_ON_STDERR", "1", 1);
>
>- execute_test_process(outfd, errfd, socketfd, settings, entry);
>+ execute_test_process(outfd, errfd, socketfd, settings, entry, &arg_vec);
> /* unreachable */
> }
>
>@@ -1890,6 +1907,7 @@ static int execute_next_entry(struct execute_state *state,
>
> out_kmsgfd:
> close(kmsgfd);
>+ free_test_command(&arg_vec);
Since arg_vec is built after kmsgfd is opened, I think it would make
sense to call free_test_command(&arg_vec) before close(kmsgfd), to keep
it consistent with the "stacked" approach and avoid any gotchas in the
future regarding ordering of operations.
In this case, a rename of the label out_kmsgfd would also be warranted.
--
Gustavo Sousa
> out_pipe:
> close(outpipe[0]);
> close(outpipe[1]);
>--
>2.50.1
>
More information about the igt-dev
mailing list