Re: [PATCH v3 6/7] perf test: Format log directories for shell tests

[Namhyung Kim <namhyung@kernel.org>]

On Mon, Jul 21, 2025 at 03:26:41PM +0200, Jakub Brnak wrote:
> From: Veronika Molnarova <vmolnaro@redhat.com>
> 
> The name of the log directory can be taken from the test suite
> description, which possibly could contain whitespace characters. This
> can cause further issues if the name is not quoted correctly.
> 
> Replace the whitespace characters with an underscore to prevent the
> possible issues caused by the name splitting.
> 
> Signed-off-by: Michael Petlan <mpetlan@redhat.com>
> Signed-off-by: Veronika Molnarova <vmolnaro@redhat.com>
> Signed-off-by: Jakub Brnak <jbrnak@redhat.com>
> ---
>  tools/perf/tests/builtin-test.c | 21 +++++++++++++++++++--
>  1 file changed, 19 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
> index 89b180798224..9cb0788d3307 100644
> --- a/tools/perf/tests/builtin-test.c
> +++ b/tools/perf/tests/builtin-test.c
> @@ -283,6 +283,21 @@ static bool test_exclusive(const struct test_suite *t, int test_case)
>  	return t->test_cases[test_case].exclusive;
>  }
>  
> +/* Replace non-alphanumeric characters with _ */
> +static void check_dir_name(const char *src, char *dst)

It's not about just checking the name.  Maybe replace_dir_name()?

Thanks,
Namhyung

> +{
> +	size_t i;
> +	size_t len = strlen(src);
> +
> +	for (i = 0; i < len; i++) {
> +		if (!isalnum(src[i]))
> +			dst[i] = '_';
> +		else
> +			dst[i] = src[i];
> +	}
> +	dst[i] = '\0';
> +}
> +
>  static int delete_file(const char *fpath, const struct stat *sb __maybe_unused,
>  						 int typeflag, struct FTW *ftwbuf)
>  {
> @@ -328,10 +343,12 @@ static bool create_logs(struct test_suite *t, int pass){
>  
>  static char *setup_shell_logs(const char *name)
>  {
> -	char template[PATH_MAX];
> +	char template[PATH_MAX], valid_name[strlen(name)+1];
>  	char *temp_dir;
>  
> -	if (snprintf(template, PATH_MAX, "/tmp/perf_test_%s.XXXXXX", name) < 0) {
> +	check_dir_name(name, valid_name);
> +
> +	if (snprintf(template, PATH_MAX, "/tmp/perf_test_%s.XXXXXX", valid_name) < 0) {

It'd be better to do snprintf() first and replace the name so that it
doesn't add the valid_name buffer.

Thanks,
Namhyung


>  		pr_err("Failed to create log dir template");
>  		return NULL; /* Skip the testsuite */
>  	}
> -- 
> 2.50.1
>