Igt-dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Gustavo Sousa <gustavo.sousa@intel.com>
To: igt-dev@lists.freedesktop.org
Subject: [PATCH i-g-t 2/3] runner: Make it easier to extend argv
Date: Thu,  9 May 2024 12:24:30 -0300	[thread overview]
Message-ID: <20240509152442.189166-3-gustavo.sousa@intel.com> (raw)
In-Reply-To: <20240509152442.189166-1-gustavo.sousa@intel.com>

In an upcoming change, we will be adding the option to forward the
--hook option to the test cases, which will require updating
execute_test_process() to add the option when asked by the user.

The current implementation makes that task not quite straightforward:
filling of argv is already dependent on stuff like entry->subtest_count
and dynbegin; if we want to keep on using constant indices, we would
need several conditional branches for adding arguments for --hook.

One way of simplifying this is to treat argv as a stack and have a
"head" index variable which is incremented after we are done setting the
current value. However, we would lose the benefit of static array bounds
analysis from the compiler, which we do have with the current code.

Let's instead define a static mapping in argv_refs of the possible
arguments that execute_test_process() could define and then generate
argv at the end with only those values that were set. With that, we keep
benefits of static array bounds analysis and make it easier to extend
argv, which is just a matter of adding more stuff into argv_refs and
setting them when necessary.

Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
---
 runner/executor.c | 44 ++++++++++++++++++++++++++++++--------------
 1 file changed, 30 insertions(+), 14 deletions(-)

diff --git a/runner/executor.c b/runner/executor.c
index 4b374d2235b2..e9b037ebcaf9 100644
--- a/runner/executor.c
+++ b/runner/executor.c
@@ -1506,7 +1506,18 @@ execute_test_process(int outfd, int errfd, int socketfd,
 		     struct settings *settings,
 		     struct job_list_entry *entry)
 {
-	char *argv[6] = {};
+	char *arg0;
+	char *arg_run_subtest[2] = {};
+	char *arg_dyn_subtest[2] = {};
+	char **argv_refs[] = {
+		&arg0,
+		&arg_run_subtest[0],
+		&arg_run_subtest[1],
+		&arg_dyn_subtest[0],
+		&arg_dyn_subtest[1],
+		NULL,
+	};
+	char *argv[sizeof(argv_refs) / sizeof(argv_refs[0])] = {};
 	size_t rootlen;
 
 	dup2(outfd, STDOUT_FILENO);
@@ -1515,30 +1526,30 @@ execute_test_process(int outfd, int errfd, int socketfd,
 	setpgid(0, 0);
 
 	rootlen = strlen(settings->test_root);
-	argv[0] = malloc(rootlen + strlen(entry->binary) + 2);
-	strcpy(argv[0], settings->test_root);
-	argv[0][rootlen] = '/';
-	strcpy(argv[0] + rootlen + 1, entry->binary);
+	arg0 = malloc(rootlen + strlen(entry->binary) + 2);
+	strcpy(arg0, settings->test_root);
+	arg0[rootlen] = '/';
+	strcpy(arg0 + rootlen + 1, entry->binary);
 
 	if (entry->subtest_count) {
 		size_t argsize;
 		const char *dynbegin;
 		size_t i;
 
-		argv[1] = strdup("--run-subtest");
+		arg_run_subtest[0] = strdup("--run-subtest");
 
 		if ((dynbegin = strchr(entry->subtests[0], '@')) != NULL)
 			argsize = dynbegin - entry->subtests[0];
 		else
 			argsize = strlen(entry->subtests[0]);
 
-		argv[2] = malloc(argsize + 1);
-		memcpy(argv[2], entry->subtests[0], argsize);
-		argv[2][argsize] = '\0';
+		arg_run_subtest[1] = malloc(argsize + 1);
+		memcpy(arg_run_subtest[1], entry->subtests[0], argsize);
+		arg_run_subtest[1][argsize] = '\0';
 
 		if (dynbegin) {
-			argv[3] = strdup("--dynamic-subtest");
-			argv[4] = strdup(dynbegin + 1);
+			arg_dyn_subtest[0] = strdup("--dynamic-subtest");
+			arg_dyn_subtest[1] = strdup(dynbegin + 1);
 		}
 
 		for (i = 1; i < entry->subtest_count; i++) {
@@ -1547,13 +1558,18 @@ execute_test_process(int outfd, int errfd, int socketfd,
 
 			assert(dynbegin == NULL);
 
-			argv[2] = realloc(argv[2], argsize + sublen + 2);
-			argv[2][argsize] = ',';
-			strcpy(argv[2] + argsize + 1, sub);
+			arg_run_subtest[1] = realloc(arg_run_subtest[1], argsize + sublen + 2);
+			arg_run_subtest[1][argsize] = ',';
+			strcpy(arg_run_subtest[1] + argsize + 1, sub);
 			argsize += sublen + 1;
 		}
 	}
 
+	/* Build argv with only stuff that is set. */
+	for (size_t i = 0, j = 0; argv_refs[i]; i++)
+		if (*argv_refs[i])
+			argv[j++] = *argv_refs[i];
+
 	if (socketfd >= 0) {
 		struct runnerpacket *packet;
 
-- 
2.45.0


  parent reply	other threads:[~2024-05-09 15:25 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-05-09 15:24 [PATCH i-g-t 0/3] Add support for hook script Gustavo Sousa
2024-05-09 15:24 ` [PATCH i-g-t 1/3] igt_hook: Add feature Gustavo Sousa
2024-05-13 17:10   ` Kamil Konieczny
2024-05-15 17:10   ` Kamil Konieczny
2024-05-15 17:35     ` Gustavo Sousa
2024-05-16 10:40       ` Kamil Konieczny
2024-05-16 12:19         ` Gustavo Sousa
2024-05-16 16:30           ` Kamil Konieczny
2024-05-16 17:05             ` Gustavo Sousa
2024-05-20 19:03       ` Lucas De Marchi
2024-05-21 19:40   ` Lucas De Marchi
2024-06-19 21:12     ` Gustavo Sousa
2024-05-09 15:24 ` Gustavo Sousa [this message]
2024-05-09 15:24 ` [PATCH i-g-t 3/3] runner: Add option --hook Gustavo Sousa
2024-05-09 16:13 ` ✗ GitLab.Pipeline: warning for Add support for hook script Patchwork
2024-05-09 17:04   ` Gustavo Sousa
2024-05-20 19:44     ` Lucas De Marchi
2024-06-19 19:07       ` Gustavo Sousa
2024-05-09 16:24 ` ✓ CI.xeBAT: success " Patchwork
2024-05-09 16:33 ` ✓ Fi.CI.BAT: " Patchwork
2024-05-09 21:12 ` ✗ CI.xeFULL: failure " Patchwork
2024-05-10  5:30 ` ✗ Fi.CI.IGT: " Patchwork

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=20240509152442.189166-3-gustavo.sousa@intel.com \
    --to=gustavo.sousa@intel.com \
    --cc=igt-dev@lists.freedesktop.org \
    /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