Re: [PATCH 06/17] tools/perf/tests: Fix shellcheck warnings for trace+probe_vfs_getname.sh

[Arnaldo Carvalho de Melo <acme@kernel.org>]

Em Tue, Jun 13, 2023 at 10:11:34PM +0530, Athira Rajeev escreveu:
> From: Akanksha J N <akanksha@linux.ibm.com>
> 
> Fix the shellcheck warnings on powerpc and x86 for testcase
> trace+probe_vfs_getname.sh. Add quotes to prevent word splitting
> which are caused by unquoted command expansions.
> 
> Before fix:
> 
> $ shellcheck -S warning trace+probe_vfs_getname.sh
> 
> 	In trace+probe_vfs_getname.sh line 13:
> 	. $(dirname $0)/lib/probe.sh
> 	  ^-----------^ SC2046 (warning): Quote this to prevent word splitting.
> 
> 	In trace+probe_vfs_getname.sh line 18:
> 	. $(dirname $0)/lib/probe_vfs_getname.sh
> 	  ^-----------^ SC2046 (warning): Quote this to prevent word splitting.
> 
> 	In trace+probe_vfs_getname.sh line 21:
> 		evts=$(echo $(perf list syscalls:sys_enter_open* 2>/dev/null | grep -E 'open(at)? ' | sed -r 's/.*sys_enter_([a-z]+) +\[.*$/\1/') | sed 's/ /,/')
> 		            ^-- SC2046 (warning): Quote this to prevent word splitting.
> 
> 	100: Check open filename arg using perf trace + vfs_getname          : Ok
> 
> After the fix:
> 
> $ shellcheck -S warning trace+probe_vfs_getname.sh
> 
> 	100: Check open filename arg using perf trace + vfs_getname          : Ok

So, I tried this on x86_64, fedora and get:

[root@quaco ~]# perf test "trace + vfs"
115: Check open filename arg using perf trace + vfs_getname          : FAILED!
[root@quaco ~]# 

Then, looking at the change:


> -	evts=$(echo $(perf list syscalls:sys_enter_open* 2>/dev/null | grep -E 'open(at)? ' | sed -r 's/.*sys_enter_([a-z]+) +\[.*$/\1/') | sed 's/ /,/')
> +	evts=$(echo "$(perf list syscalls:sys_enter_open* 2>/dev/null | grep -E 'open(at)? ' | sed -r 's/.*sys_enter_([a-z]+) +\[.*$/\1/')" | sed 's/ /,/')

So, before:

[root@quaco ~]# evts=$(echo $(perf list syscalls:sys_enter_open* 2>/dev/null | grep -E 'open(at)? ' | sed -r 's/.*sys_enter_([a-z]+) +\[.*$/\1/') | sed 's/ /,/')
[root@quaco ~]# echo $evts
open,openat
[root@quaco ~]#

Then after:

[root@quaco ~]# evts=$(echo "$(perf list syscalls:sys_enter_open* 2>/dev/null | grep -E 'open(at)? ' | sed -r 's/.*sys_enter_([a-z]+) +\[.*$/\1/')" | sed 's/ /,/')
[root@quaco ~]# echo $evts
open openat
[root@quaco ~]#

Not equivalent, so I'm removing this patch, please check and resubmit,
please.

- Arnaldo

> 
> Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
> Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
> Signed-off-by: Akanksha J N <akanksha@linux.ibm.com>
> Signed-off-by: Abhishek Singh Tomar <abhishek@linux.ibm.com>
> Signed-off-by: Saket <skb99@linux.vnet.ibm.com>
> Signed-off-by: Avnish Chouhan <avnish@linux.vnet.ibm.com>
> ---
>  tools/perf/tests/shell/trace+probe_vfs_getname.sh | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/perf/tests/shell/trace+probe_vfs_getname.sh b/tools/perf/tests/shell/trace+probe_vfs_getname.sh
> index 0a4bac3dd77e..935eac7efa47 100755
> --- a/tools/perf/tests/shell/trace+probe_vfs_getname.sh
> +++ b/tools/perf/tests/shell/trace+probe_vfs_getname.sh
> @@ -10,15 +10,15 @@
>  # SPDX-License-Identifier: GPL-2.0
>  # Arnaldo Carvalho de Melo <acme@kernel.org>, 2017
>  
> -. $(dirname $0)/lib/probe.sh
> +. "$(dirname $0)"/lib/probe.sh
>  
>  skip_if_no_perf_probe || exit 2
>  skip_if_no_perf_trace || exit 2
>  
> -. $(dirname $0)/lib/probe_vfs_getname.sh
> +. "$(dirname $0)"/lib/probe_vfs_getname.sh
>  
>  trace_open_vfs_getname() {
> -	evts=$(echo $(perf list syscalls:sys_enter_open* 2>/dev/null | grep -E 'open(at)? ' | sed -r 's/.*sys_enter_([a-z]+) +\[.*$/\1/') | sed 's/ /,/')
> +	evts=$(echo "$(perf list syscalls:sys_enter_open* 2>/dev/null | grep -E 'open(at)? ' | sed -r 's/.*sys_enter_([a-z]+) +\[.*$/\1/')" | sed 's/ /,/')
>  	perf trace -e $evts touch $file 2>&1 | \
>  	grep -E " +[0-9]+\.[0-9]+ +\( +[0-9]+\.[0-9]+ ms\): +touch\/[0-9]+ open(at)?\((dfd: +CWD, +)?filename: +${file}, +flags: CREAT\|NOCTTY\|NONBLOCK\|WRONLY, +mode: +IRUGO\|IWUGO\) += +[0-9]+$"
>  }
> -- 
> 2.39.1
> 

-- 

- Arnaldo