Re: [PATCH i-g-t v3 09/10] runner/settings: Serialize command line

[Gustavo Sousa <gustavo.sousa@intel.com>]

Quoting Lucas De Marchi (2025-01-30 14:21:48-03:00)
>Serialize the command line to metadata.txt. The expected format in the
>metadata.txt is like below:
>
>        cmdline.argc : 6
>        cmdline.argv[0] : ./build/runner/igt_runner
>        cmdline.argv[1] : -o
>        cmdline.argv[2] : --test-list
>        cmdline.argv[3] : /tmp/testlist.txt
>        cmdline.argv[4] : build/tests/
>        cmdline.argv[5] : /tmp/results
>
>Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
>---
> runner/settings.c | 21 +++++++++++++++++++++
> runner/settings.h |  4 ++++
> 2 files changed, 25 insertions(+)
>
>diff --git a/runner/settings.c b/runner/settings.c
>index 62afd3534..84d8c85e9 100644
>--- a/runner/settings.c
>+++ b/runner/settings.c
>@@ -529,6 +529,14 @@ static void free_hook_strs(struct igt_vec *hook_strs)
>         igt_vec_fini(hook_strs);
> }
> 
>+static void free_array_deep(void **arr, size_t n)
>+{

We need to make sure that arr is not NULL before going into the for-loop
below. That would be the case if "settings->cmdline.argv = calloc(argc,
sizeof(*settings->cmdline.argv))" failed.

With that fixed,

    Reviewed-by: Gustavo Sousa <gustavo.sousa@intel.com>

>+        for (size_t i = 0; i < n; i++)
>+                free(arr[i]);
>+
>+        free(arr);
>+}
>+
> static bool file_exists_at(int dirfd, const char *filename)
> {
>         return faccessat(dirfd, filename, F_OK, 0) == 0;
>@@ -646,6 +654,7 @@ void clear_settings(struct settings *settings)
>         free_regexes(&settings->exclude_regexes);
>         free_env_vars(&settings->env_vars);
>         free_hook_strs(&settings->hook_strs);
>+        free_array_deep((void **)settings->cmdline.argv, settings->cmdline.argc);
> 
>         init_settings(settings);
> }
>@@ -875,6 +884,16 @@ bool parse_options(int argc, char **argv,
>                 goto error;
>         }
> 
>+        settings->cmdline.argv = calloc(argc, sizeof(*settings->cmdline.argv));
>+        if (!settings->cmdline.argv)
>+                goto error;
>+
>+        settings->cmdline.argc = argc;
>+        for (int i = 0; i < argc; i++) {
>+                settings->cmdline.argv[i] = strdup(argv[i]);
>+                if (!settings->cmdline.argv[i])
>+                        goto error;
>+        }
> 
>         return true;
> 
>@@ -1202,6 +1221,7 @@ bool serialize_settings(struct settings *settings)
>         SERIALIZE_INT(f, settings, enable_code_coverage);
>         SERIALIZE_INT(f, settings, cov_results_per_test);
>         SERIALIZE_STR(f, settings, code_coverage_script);
>+        SERIALIZE_STR_ARRAY(f, settings, cmdline.argv, cmdline.argc);
> 
>         if (settings->sync) {
>                 fflush(f);
>@@ -1315,6 +1335,7 @@ bool read_settings_from_file(struct settings *settings, FILE *f)
>                 PARSE_INT(settings, name, val, enable_code_coverage);
>                 PARSE_INT(settings, name, val, cov_results_per_test);
>                 PARSE_STR(settings, name, val, code_coverage_script);
>+                PARSE_STR_ARRAY(settings, name, val, cmdline.argv, cmdline.argc);
> 
>                 printf("Warning: Unknown field in settings file: %s = %s\n",
>                        name, val);
>diff --git a/runner/settings.h b/runner/settings.h
>index f69f09778..2266118a7 100644
>--- a/runner/settings.h
>+++ b/runner/settings.h
>@@ -75,6 +75,10 @@ struct settings {
>         char *code_coverage_script;
>         bool enable_code_coverage;
>         bool cov_results_per_test;
>+        struct {
>+                int argc;
>+                char **argv;
>+        } cmdline;
> };
> 
> /**
>-- 
>2.48.0
>