Re: [igt-dev] [PATCH i-g-t] runner: Fix handling of metadata values containing spaces

[Petri Latvala <petri.latvala@intel.com>]

On Wed, Jun 10, 2020 at 12:04:17PM -0400, Lyude wrote:
> From: Lyude Paul <lyude@redhat.com>
> 
> Noticed while running some tests that adding any kind of spaces into the
> name of a test run would stop igt_resume from working for said test run.
> Turns out that when we parse test metadata, we accidentally use the
> '%ms' specifier with fscanf() which finishes parsing strings when any
> kind of whitespace is encountered.
> 
> So, fix this by using the proper %m[^\n] specifier, which dynamically
> allocates it's result and doesn't stop reading the string until a
> newline is encountered. Additionally, add a test for this.
> 
> Signed-off-by: Lyude Paul <lyude@redhat.com>

Yeah, and running igt_resume breaks in the same way. And it's not
limited to just the name, the file paths are also broken if they have
spaces. Testing just the name field should be enough to cover those
though.

Thanks for the fix,

Reviewed-by: Petri Latvala <petri.latvala@intel.com>


> ---
>  runner/runner_tests.c | 12 ++++++++++++
>  runner/settings.c     |  2 +-
>  2 files changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/runner/runner_tests.c b/runner/runner_tests.c
> index 60e00960..48b02107 100644
> --- a/runner/runner_tests.c
> +++ b/runner/runner_tests.c
> @@ -1351,6 +1351,18 @@ igt_main
>  
>  			fclose(f);
>  		}
> +
> +		igt_subtest("metadata-read-spaces") {
> +			char metadata[] = "name : foo bar\n";
> +			FILE *f = fmemopen(metadata, strlen(metadata), "r");
> +			igt_assert(f);
> +
> +			igt_assert(read_settings_from_file(settings, f));
> +
> +			igt_assert_eqstr(settings->name, "foo bar");
> +
> +			fclose(f);
> +		}
>  	}
>  
>  	igt_subtest_group {
> diff --git a/runner/settings.c b/runner/settings.c
> index d18e55d1..25f248ef 100644
> --- a/runner/settings.c
> +++ b/runner/settings.c
> @@ -679,7 +679,7 @@ bool read_settings_from_file(struct settings *settings, FILE *f)
>  
>  	settings->dmesg_warn_level = -1;
>  
> -	while (fscanf(f, "%ms : %ms", &name, &val) == 2) {
> +	while (fscanf(f, "%ms : %m[^\n]", &name, &val) == 2) {
>  		int numval = atoi(val);
>  		PARSE_LINE(settings, name, val, abort_mask, numval);
>  		PARSE_LINE(settings, name, val, test_list, val ? strdup(val) : NULL);
> -- 
> 2.26.2
> 
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev