2024-04-09 02:34:57

by Ian Rogers

[permalink] [raw]
Subject: [PATCH v1 0/4] Add shellcheck beyond perf/tests

Add shellcheck to other locations of shell scripts, not just
tools/perf/tests. Fix issues detected by shellcheck.

Ian Rogers (4):
perf build: Add shellcheck to tools/perf scripts
perf arch x86: Add shellcheck to build
perf util: Add shellcheck to generate-cmdlist.sh
perf trace beauty: Add shellcheck to scripts

tools/perf/Build | 14 +++++++++++
tools/perf/arch/x86/Build | 14 +++++++++++
tools/perf/arch/x86/tests/Build | 14 +++++++++++
tools/perf/arch/x86/tests/gen-insn-x86-dat.sh | 2 +-
tools/perf/perf-archive.sh | 2 +-
tools/perf/perf-completion.sh | 23 +++++++++++++------
tools/perf/trace/beauty/Build | 14 +++++++++++
tools/perf/trace/beauty/arch_errno_names.sh | 8 ++++---
tools/perf/util/Build | 14 +++++++++++
9 files changed, 93 insertions(+), 12 deletions(-)

--
2.44.0.478.gd926399ef9-goog



2024-04-09 02:35:25

by Ian Rogers

[permalink] [raw]
Subject: [PATCH v1 1/4] perf build: Add shellcheck to tools/perf scripts

Address shell check errors/warnings in perf-archive.sh and
perf-completion.sh.

Signed-off-by: Ian Rogers <[email protected]>
---
tools/perf/Build | 14 ++++++++++++++
tools/perf/perf-archive.sh | 2 +-
tools/perf/perf-completion.sh | 23 ++++++++++++++++-------
3 files changed, 31 insertions(+), 8 deletions(-)

diff --git a/tools/perf/Build b/tools/perf/Build
index aa7623622834..b0cb7ad8e6ac 100644
--- a/tools/perf/Build
+++ b/tools/perf/Build
@@ -59,3 +59,17 @@ perf-y += ui/
perf-y += scripts/

gtk-y += ui/gtk/
+
+ifdef SHELLCHECK
+ SHELL_TESTS := $(wildcard *.sh)
+ TEST_LOGS := $(SHELL_TESTS:%=%.shellcheck_log)
+else
+ SHELL_TESTS :=
+ TEST_LOGS :=
+endif
+
+$(OUTPUT)%.shellcheck_log: %
+ $(call rule_mkdir)
+ $(Q)$(call echo-cmd,test)shellcheck -s bash -a -S warning "$<" > $@ || (cat $@ && rm $@ && false)
+
+perf-y += $(TEST_LOGS)
diff --git a/tools/perf/perf-archive.sh b/tools/perf/perf-archive.sh
index f94795794b36..6ed7e52ab881 100755
--- a/tools/perf/perf-archive.sh
+++ b/tools/perf/perf-archive.sh
@@ -34,7 +34,7 @@ if [ $UNPACK -eq 1 ]; then
TARGET=`find . -regex "\./perf.*\.tar\.bz2"`
TARGET_NUM=`echo -n "$TARGET" | grep -c '^'`

- if [ -z "$TARGET" -o $TARGET_NUM -gt 1 ]; then
+ if [ -z "$TARGET" ] || [ $TARGET_NUM -gt 1 ]; then
echo -e "Error: $TARGET_NUM files found for unpacking:\n$TARGET"
echo "Provide the requested file as an argument"
exit 1
diff --git a/tools/perf/perf-completion.sh b/tools/perf/perf-completion.sh
index f224d79b89e6..69cba3c170d5 100644
--- a/tools/perf/perf-completion.sh
+++ b/tools/perf/perf-completion.sh
@@ -108,6 +108,8 @@ __perf__ltrim_colon_completions()

__perfcomp ()
{
+ # Expansion of spaces to array is deliberate.
+ # shellcheck disable=SC2207
COMPREPLY=( $( compgen -W "$1" -- "$2" ) )
}

@@ -127,13 +129,13 @@ __perf_prev_skip_opts ()

let i=cword-1
cmds_=$($cmd $1 --list-cmds)
- prev_skip_opts=()
+ prev_skip_opts=""
while [ $i -ge 0 ]; do
- if [[ ${words[i]} == $1 ]]; then
+ if [[ ${words[i]} == "$1" ]]; then
return
fi
for cmd_ in $cmds_; do
- if [[ ${words[i]} == $cmd_ ]]; then
+ if [[ ${words[i]} == "$cmd_" ]]; then
prev_skip_opts=${words[i]}
return
fi
@@ -164,9 +166,10 @@ __perf_main ()
$prev_skip_opts == @(record|stat|top) ]]; then

local cur1=${COMP_WORDS[COMP_CWORD]}
- local raw_evts=$($cmd list --raw-dump hw sw cache tracepoint pmu sdt)
+ local raw_evts
local arr s tmp result cpu_evts

+ raw_evts=$($cmd list --raw-dump hw sw cache tracepoint pmu sdt)
# aarch64 doesn't have /sys/bus/event_source/devices/cpu/events
if [[ `uname -m` != aarch64 ]]; then
cpu_evts=$(ls /sys/bus/event_source/devices/cpu/events)
@@ -175,10 +178,12 @@ __perf_main ()
if [[ "$cur1" == */* && ${cur1#*/} =~ ^[A-Z] ]]; then
OLD_IFS="$IFS"
IFS=" "
+ # Expansion of spaces to array is deliberate.
+ # shellcheck disable=SC2206
arr=($raw_evts)
IFS="$OLD_IFS"

- for s in ${arr[@]}
+ for s in "${arr[@]}"
do
if [[ "$s" == *cpu/* ]]; then
tmp=${s#*cpu/}
@@ -200,11 +205,13 @@ __perf_main ()
fi
elif [[ $prev == @("--pfm-events") &&
$prev_skip_opts == @(record|stat|top) ]]; then
- local evts=$($cmd list --raw-dump pfm)
+ local evts
+ evts=$($cmd list --raw-dump pfm)
__perfcomp "$evts" "$cur"
elif [[ $prev == @("-M"|"--metrics") &&
$prev_skip_opts == @(stat) ]]; then
- local metrics=$($cmd list --raw-dump metric metricgroup)
+ local metrics
+ metrics=$($cmd list --raw-dump metric metricgroup)
__perfcomp "$metrics" "$cur"
else
# List subcommands for perf commands
@@ -278,6 +285,8 @@ if [[ -n ${ZSH_VERSION-} ]]; then
let cword=CURRENT-1
emulate ksh -c __perf_main
let _ret && _default && _ret=0
+ # _ret is only assigned 0 or 1, disable inaccurate analysis.
+ # shellcheck disable=SC2152
return _ret
}

--
2.44.0.478.gd926399ef9-goog


2024-04-09 02:36:20

by Ian Rogers

[permalink] [raw]
Subject: [PATCH v1 3/4] perf util: Add shellcheck to generate-cmdlist.sh

Add shellcheck to generate-cmdlist.sh to avoid basic shell script
mistakes.

Signed-off-by: Ian Rogers <[email protected]>
---
tools/perf/util/Build | 14 ++++++++++++++
1 file changed, 14 insertions(+)

diff --git a/tools/perf/util/Build b/tools/perf/util/Build
index aec5a590e349..292170a99ab6 100644
--- a/tools/perf/util/Build
+++ b/tools/perf/util/Build
@@ -389,3 +389,17 @@ $(OUTPUT)util/vsprintf.o: ../lib/vsprintf.c FORCE
$(OUTPUT)util/list_sort.o: ../lib/list_sort.c FORCE
$(call rule_mkdir)
$(call if_changed_dep,cc_o_c)
+
+ifdef SHELLCHECK
+ SHELL_TESTS := generate-cmdlist.sh
+ TEST_LOGS := $(SHELL_TESTS:%=%.shellcheck_log)
+else
+ SHELL_TESTS :=
+ TEST_LOGS :=
+endif
+
+$(OUTPUT)%.shellcheck_log: %
+ $(call rule_mkdir)
+ $(Q)$(call echo-cmd,test)shellcheck -a -S warning "$<" > $@ || (cat $@ && rm $@ && false)
+
+perf-y += $(TEST_LOGS)
--
2.44.0.478.gd926399ef9-goog


2024-04-09 10:03:11

by James Clark

[permalink] [raw]
Subject: Re: [PATCH v1 0/4] Add shellcheck beyond perf/tests



On 09/04/2024 03:32, Ian Rogers wrote:
> Add shellcheck to other locations of shell scripts, not just
> tools/perf/tests. Fix issues detected by shellcheck.
>
> Ian Rogers (4):
> perf build: Add shellcheck to tools/perf scripts
> perf arch x86: Add shellcheck to build
> perf util: Add shellcheck to generate-cmdlist.sh
> perf trace beauty: Add shellcheck to scripts
>
> tools/perf/Build | 14 +++++++++++
> tools/perf/arch/x86/Build | 14 +++++++++++
> tools/perf/arch/x86/tests/Build | 14 +++++++++++
> tools/perf/arch/x86/tests/gen-insn-x86-dat.sh | 2 +-
> tools/perf/perf-archive.sh | 2 +-
> tools/perf/perf-completion.sh | 23 +++++++++++++------
> tools/perf/trace/beauty/Build | 14 +++++++++++
> tools/perf/trace/beauty/arch_errno_names.sh | 8 ++++---
> tools/perf/util/Build | 14 +++++++++++
> 9 files changed, 93 insertions(+), 12 deletions(-)
>

Reviewed-by: James Clark <[email protected]>

2024-04-12 19:39:06

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH v1 0/4] Add shellcheck beyond perf/tests

On Tue, Apr 09, 2024 at 11:02:58AM +0100, James Clark wrote:
>
>
> On 09/04/2024 03:32, Ian Rogers wrote:
> > Add shellcheck to other locations of shell scripts, not just
> > tools/perf/tests. Fix issues detected by shellcheck.
> >
> > Ian Rogers (4):
> > perf build: Add shellcheck to tools/perf scripts
> > perf arch x86: Add shellcheck to build
> > perf util: Add shellcheck to generate-cmdlist.sh
> > perf trace beauty: Add shellcheck to scripts
> >
> > tools/perf/Build | 14 +++++++++++
> > tools/perf/arch/x86/Build | 14 +++++++++++
> > tools/perf/arch/x86/tests/Build | 14 +++++++++++
> > tools/perf/arch/x86/tests/gen-insn-x86-dat.sh | 2 +-
> > tools/perf/perf-archive.sh | 2 +-
> > tools/perf/perf-completion.sh | 23 +++++++++++++------
> > tools/perf/trace/beauty/Build | 14 +++++++++++
> > tools/perf/trace/beauty/arch_errno_names.sh | 8 ++++---
> > tools/perf/util/Build | 14 +++++++++++
> > 9 files changed, 93 insertions(+), 12 deletions(-)
> >
>
> Reviewed-by: James Clark <[email protected]>

Thanks, applied to perf-tools-next.

- Arnaldo