[CI i-g-t 07/10] runner: Fix use of newline on arguments

[Peter Senna Tschudin]

On 08.02.2025 00:09, Lucas De Marchi wrote:
> Currently there's no support for newlines on arguments passed to runner.
> However it's also a silent failure:
> 
> 	# igt_runner --test-list '/tmp/test
> 	list2.txt' build/tests/ /tmp/results
> 
> 	# head /tmp/results/metadata.txt
> 	disk_usage_limit : 0
> 	test_list : /tmp/test
> 	list2.txt
> 	name : results
> 	...
> 
> 	# ./build/runner/igt_resume /tmp/results
> 	[9840425.334900] All tests already executed.
> 	resume failed at generating results
> 	Done.
> 
> Embedding a newline like this is very dubious for test-list, but it's
> used for e.g. hooks. In future we will add the command line to the
> metadata and possibly migrate the hooks, so add support for
> escaping/unescaping the string on save/restore.
> 
> The method chosen is slightly different than the one used for hooks:
> instead of adding a escape char and keeping the char escaped, this just
> prefers using an hex representation of the char with a \x<HEX>h
> sequence. This makes it easier when unescaping since the reader can
> continue reading one line per iteration. In future this can also be
> adopted by the hooks or even migrating the hooks to use metadata.txt.
> 
> Another fix is that now we just skip null values on the serialization
> side.  Previously it would serialize "(null)" and then load that string
> instead of NULL. Add code_coverage_script to the runner_test to cover
> that, which would previously fail.
> 

Tested-by: Peter Senna Tschudin <peter.senna at linux.intel.com>
Reviewed-by: Peter Senna Tschudin <peter.senna at linux.intel.com>
> Reviewed-by: Gustavo Sousa <gustavo.sousa at intel.com>
> Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>
> ---
>  runner/runner_tests.c |  1 +
>  runner/settings.c     | 85 +++++++++++++++++++++++++++++++++++++++++--
>  2 files changed, 83 insertions(+), 3 deletions(-)
> 
> diff --git a/runner/runner_tests.c b/runner/runner_tests.c
> index 8441763f2..93b3ebc9f 100644
> --- a/runner/runner_tests.c
> +++ b/runner/runner_tests.c
> @@ -200,6 +200,7 @@ static void assert_settings_equal(struct settings *one, struct settings *two)
>  	igt_assert_eq(one->use_watchdog, two->use_watchdog);
>  	igt_assert_eqstr(one->test_root, two->test_root);
>  	igt_assert_eqstr(one->results_path, two->results_path);
> +	igt_assert_eqstr(one->code_coverage_script, two->code_coverage_script);
>  	igt_assert_eq(one->piglit_style_dmesg, two->piglit_style_dmesg);
>  	igt_assert_eq(one->dmesg_warn_level, two->dmesg_warn_level);
>  	igt_assert_eq(one->prune_mode, two->prune_mode);
> diff --git a/runner/settings.c b/runner/settings.c
> index 693c5484e..97b62ed36 100644
> --- a/runner/settings.c
> +++ b/runner/settings.c
> @@ -7,6 +7,7 @@
>  #include <errno.h>
>  #include <fcntl.h>
>  #include <getopt.h>
> +#include <inttypes.h>
>  #include <libgen.h>
>  #include <limits.h>
>  #include <stdio.h>
> @@ -1052,10 +1053,80 @@ static bool serialize_hook_strs(struct settings *settings, int dirfd)
>  	return true;
>  }
>  
> +/*
> + * Serialize @s to @f, escaping '\' and '\n'. See unescape_str()
> + */
> +static void escape_str(const char *s, FILE *f)
> +{
> +	while (*s) {
> +		size_t len = strcspn(s, "\\\n");
> +
> +		if (len > 0) {
> +			fwrite(s, len, 1, f);
> +			s += len;
> +		}
> +
> +		if (*s) {
> +			fprintf(f, "\\x%xh", *s);
> +			s++;
> +		}
> +	}
> +}
> +
> +/*
> + * Unescape a '\' and '\n': undo escape_str
> + *
> + * Escape chars using the form '\x<hex>h' so they don't interfere with the line
> + * parser.
> + *
> + * Return the number of chars saved in buf and optionally
> + * the number of chars scanned in @n_src if that is non-nul.
> + */
> +static ssize_t unescape_str(char *buf, size_t *n_src)
> +{
> +	size_t dst_len = 0;
> +	char *s = buf;
> +
> +	while (*s) {
> +		char next = *(s + 1);
> +
> +		if (*s != '\\') {
> +			buf[dst_len++] = *s++;
> +		} else if (next == 'x') {
> +			unsigned long num;
> +
> +			s += 2;
> +
> +			num = strtoul(s, &s, 16);
> +			/* cover both error due to overflow or invalid char */
> +			if (num > UINT8_MAX || *s != 'h')
> +				return -EINVAL;
> +
> +			buf[dst_len++] = num;
> +			s++;
> +		} else {
> +			return -EINVAL;
> +		}
> +	}
> +
> +	buf[dst_len] = '\0';
> +
> +	if (n_src)
> +		*n_src = s - buf;
> +
> +	return dst_len;
> +}
> +
>  #define SERIALIZE_LINE(f, s, name, fmt) fprintf(f, "%s : " fmt "\n", #name, s->name)
>  #define SERIALIZE_INT(f, s, name) SERIALIZE_LINE(f, s, name, "%d")
>  #define SERIALIZE_UL(f, s, name) SERIALIZE_LINE(f, s, name, "%lu")
> -#define SERIALIZE_STR(f, s, name) SERIALIZE_LINE(f, s, name, "%s")
> +#define SERIALIZE_STR(f, s, name) do {		\
> +		if (s->name) {			\
> +			fputs(#name " : ", f);	\
> +			escape_str(s->name, f);	\
> +			fputc('\n', f);		\
> +		}				\
> +	} while (0)
>  bool serialize_settings(struct settings *settings)
>  {
>  	FILE *f;
> @@ -1171,9 +1242,17 @@ static char *parse_str(char **val)
>  {
>  	char *ret = *val;
>  
> -	*val = NULL;
> +	/*
> +	 * Unescaping a string is guaranteed to produce a string that is
> +	 * smaller or of the same size. Just modify it in place and leak the
> +	 * buffer
> +	 */
> +	if (unescape_str(ret, NULL) >= 0) {
> +		*val = NULL;
> +		return ret;
> +	}
>  
> -	return ret;
> +	return NULL;
>  }
>  
>  #define PARSE_LINE(s, name, val, field, _f)	\