Re: [PATCH v2] perf beauty: Fix fsconfig generator

[Arnaldo Carvalho de Melo]

Em Wed, Apr 14, 2021 at 09:27:23PM +0300, Vitaly Chikunov escreveu:
> After gnulib update sed stopped matching `[[:space:]]*+' as before,
> causing the following compilation error:
> 
>   In file included from builtin-trace.c:719:
>   trace/beauty/generated/fsconfig_arrays.c:2:3: error: expected expression before ']' token
>       2 |  [] = "",
> 	|   ^
>   trace/beauty/generated/fsconfig_arrays.c:2:3: error: array index in initializer not of integer type
>   trace/beauty/generated/fsconfig_arrays.c:2:3: note: (near initialization for 'fsconfig_cmds')
> 
> Fix this by correcting the regular expression used in the generator.
> Also, clean up the script by removing redundant egrep, xargs, and printf
> invocations.
> 
> Fixes: d35293004a5e4 ("perf beauty: Add generator for fsconfig's 'cmd' arg values")
> Co-authored-by: Dmitry V. Levin <ldv@xxxxxxxxxxxx>
> Signed-off-by: Vitaly Chikunov <vt@xxxxxxxxxxxx>
> ---
>  tools/perf/trace/beauty/fsconfig.sh | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/perf/trace/beauty/fsconfig.sh b/tools/perf/trace/beauty/fsconfig.sh
> index 83fb24df05c9f..bc6ef7bb7a5f9 100755
> --- a/tools/perf/trace/beauty/fsconfig.sh
> +++ b/tools/perf/trace/beauty/fsconfig.sh
> @@ -10,8 +10,7 @@ fi
>  linux_mount=${linux_header_dir}/mount.h
>  
>  printf "static const char *fsconfig_cmds[] = {\n"
> -regex='^[[:space:]]*+FSCONFIG_([[:alnum:]_]+)[[:space:]]*=[[:space:]]*([[:digit:]]+)[[:space:]]*,[[:space:]]*.*'
> -egrep $regex ${linux_mount} | \
> -	sed -r "s/$regex/\2 \1/g"	| \
> -	xargs printf "\t[%s] = \"%s\",\n"
> +ms='[[:space:]]*'
> +sed -nr "s/^${ms}FSCONFIG_([[:alnum:]_]+)${ms}=${ms}([[:digit:]]+)${ms},.*/\t[\2] = \"\1\",/p" \
> +	${linux_mount}
>  printf "};\n"

It continues working:

[acme@five perf]$ tools/perf/trace/beauty/fsconfig.sh
static const char *fsconfig_cmds[] = {
	[0] = "SET_FLAG",
	[1] = "SET_STRING",
	[2] = "SET_BINARY",
	[3] = "SET_PATH",
	[4] = "SET_PATH_EMPTY",
	[5] = "SET_FD",
	[6] = "CMD_CREATE",
	[7] = "CMD_RECONFIGURE",
};
[acme@five perf]$

Cool, this is on f33, lemme see on some other distro:

perfbuilder@fd2d918f35e1:/git/perf$ sed --version | head -1
sed (GNU sed) 4.2.2
perfbuilder@fd2d918f35e1:/git/perf$ cat tools/perf/trace/beauty/fsconfig.sh 
#!/bin/sh
# SPDX-License-Identifier: LGPL-2.1

if [ $# -ne 1 ] ; then
	linux_header_dir=tools/include/uapi/linux
else
	linux_header_dir=$1
fi

linux_mount=${linux_header_dir}/mount.h

printf "static const char *fsconfig_cmds[] = {\n"
ms='[[:space:]]*'
sed -nr "s/^${ms}FSCONFIG_([[:alnum:]_]+)${ms}=${ms}([[:digit:]]+)${ms},.*/\t[\2] = \"\1\",/p" \
	${linux_mount}
printf "};\n"
perfbuilder@fd2d918f35e1:/git/perf$ tools/perf/trace/beauty/fsconfig.sh 
static const char *fsconfig_cmds[] = {
	[0] = "SET_FLAG",
	[1] = "SET_STRING",
	[2] = "SET_BINARY",
	[3] = "SET_PATH",
	[4] = "SET_PATH_EMPTY",
	[5] = "SET_FD",
	[6] = "CMD_CREATE",
	[7] = "CMD_RECONFIGURE",
};
perfbuilder@fd2d918f35e1:/git/perf$


[perfbuilder@five sisyphus]$ dsh alt:sisyphus
sh-4.4# bash
[root@6db6d5ad9661 /]# cat /etc/redhat-release
ALT Sisyphus Sisyphus (unstable) (sisyphus)
[root@6db6d5ad9661 /]# cd /git
[root@6db6d5ad9661 git]# cd perf
[root@6db6d5ad9661 perf]# cat tools/perf/trace/beauty/fsconfig.sh
#!/bin/sh
# SPDX-License-Identifier: LGPL-2.1

if [ $# -ne 1 ] ; then
	linux_header_dir=tools/include/uapi/linux
else
	linux_header_dir=$1
fi

linux_mount=${linux_header_dir}/mount.h

printf "static const char *fsconfig_cmds[] = {\n"
ms='[[:space:]]*'
sed -nr "s/^${ms}FSCONFIG_([[:alnum:]_]+)${ms}=${ms}([[:digit:]]+)${ms},.*/\t[\2] = \"\1\",/p" \
	${linux_mount}
printf "};\n"
[root@6db6d5ad9661 perf]# tools/perf/trace/beauty/fsconfig.sh
static const char *fsconfig_cmds[] = {
	[0] = "SET_FLAG",
	[1] = "SET_STRING",
	[2] = "SET_BINARY",
	[3] = "SET_PATH",
	[4] = "SET_PATH_EMPTY",
	[5] = "SET_FD",
	[6] = "CMD_CREATE",
	[7] = "CMD_RECONFIGURE",
};
[root@6db6d5ad9661 perf]#

So I guess we can sweep thru tools/perf/trace/beauty/*.sh and simplify
things in other table generators?

Please consider this.

Thanks, applied.

- Arnaldo