[PATCH v1 0/4] Add shellcheck beyond perf/tests

[PATCH v1 0/4] Add shellcheck beyond perf/tests

[Ian Rogers]

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

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

[Ian Rogers]

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

Signed-off-by: Ian Rogers <irogers@google.com>
---
 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

[PATCH v1 2/4] perf arch x86: Add shellcheck to build

[Ian Rogers]

Add shellcheck for:
tools/perf/arch/x86/tests/gen-insn-x86-dat.sh
tools/perf/arch/x86/entry/syscalls/syscalltbl.sh
Address a minor quoting issue.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/arch/x86/Build                     | 14 ++++++++++++++
 tools/perf/arch/x86/tests/Build               | 14 ++++++++++++++
 tools/perf/arch/x86/tests/gen-insn-x86-dat.sh |  2 +-
 3 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/tools/perf/arch/x86/Build b/tools/perf/arch/x86/Build
index a7dd46a5b678..ed37013b4289 100644
--- a/tools/perf/arch/x86/Build
+++ b/tools/perf/arch/x86/Build
@@ -1,2 +1,16 @@
 perf-y += util/
 perf-y += tests/
+
+ifdef SHELLCHECK
+  SHELL_TESTS := entry/syscalls/syscalltbl.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)
diff --git a/tools/perf/arch/x86/tests/Build b/tools/perf/arch/x86/tests/Build
index b87f46e5feea..c1e3b7d39554 100644
--- a/tools/perf/arch/x86/tests/Build
+++ b/tools/perf/arch/x86/tests/Build
@@ -10,3 +10,17 @@ perf-$(CONFIG_AUXTRACE) += insn-x86.o
 endif
 perf-$(CONFIG_X86_64) += bp-modify.o
 perf-y += amd-ibs-via-core-pmu.o
+
+ifdef SHELLCHECK
+  SHELL_TESTS := gen-insn-x86-dat.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)
diff --git a/tools/perf/arch/x86/tests/gen-insn-x86-dat.sh b/tools/perf/arch/x86/tests/gen-insn-x86-dat.sh
index 0d0a003a9c5e..89c46532cd5c 100755
--- a/tools/perf/arch/x86/tests/gen-insn-x86-dat.sh
+++ b/tools/perf/arch/x86/tests/gen-insn-x86-dat.sh
@@ -11,7 +11,7 @@ if [ "$(uname -m)" != "x86_64" ]; then
 	exit 1
 fi
 
-cd $(dirname $0)
+cd "$(dirname $0)"
 
 trap 'echo "Might need a more recent version of binutils"' EXIT
 
-- 
2.44.0.478.gd926399ef9-goog

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

[Ian Rogers]

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

Signed-off-by: Ian Rogers <irogers@google.com>
---
 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

[PATCH v1 4/4] perf trace beauty: Add shellcheck to scripts

[Ian Rogers]

Add shell check to scripts generating perf trace lookup tables. Fix
quoting issue in arch_errno_names.sh.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/trace/beauty/Build               | 14 ++++++++++++++
 tools/perf/trace/beauty/arch_errno_names.sh |  8 +++++---
 2 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/tools/perf/trace/beauty/Build b/tools/perf/trace/beauty/Build
index d8ce1b698983..cb3c1399ff40 100644
--- a/tools/perf/trace/beauty/Build
+++ b/tools/perf/trace/beauty/Build
@@ -20,3 +20,17 @@ perf-y += statx.o
 perf-y += sync_file_range.o
 perf-y += timespec.o
 perf-y += tracepoints/
+
+ifdef SHELLCHECK
+  SHELL_TESTS := $(wildcard trace/beauty/*.sh)
+  TEST_LOGS := $(SHELL_TESTS:trace/beauty/%=%.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/trace/beauty/arch_errno_names.sh b/tools/perf/trace/beauty/arch_errno_names.sh
index 7df4bf5b55a3..30d3889b2957 100755
--- a/tools/perf/trace/beauty/arch_errno_names.sh
+++ b/tools/perf/trace/beauty/arch_errno_names.sh
@@ -60,10 +60,12 @@ create_arch_errno_table_func()
 	printf 'arch_syscalls__strerrno_t *arch_syscalls__strerrno_function(const char *arch)\n'
 	printf '{\n'
 	for arch in $archlist; do
-		printf '\tif (!strcmp(arch, "%s"))\n' $(arch_string "$arch")
-		printf '\t\treturn errno_to_name__%s;\n' $(arch_string "$arch")
+		arch_str=$(arch_string "$arch")
+		printf '\tif (!strcmp(arch, "%s"))\n' "$arch_str"
+		printf '\t\treturn errno_to_name__%s;\n' "$arch_str"
 	done
-	printf '\treturn errno_to_name__%s;\n' $(arch_string "$default")
+	arch_str=$(arch_string "$default")
+	printf '\treturn errno_to_name__%s;\n' "$arch_str"
 	printf '}\n'
 }
 
-- 
2.44.0.478.gd926399ef9-goog

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

[James Clark]

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 <james.clark@arm.com>

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

[Arnaldo Carvalho de Melo]

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 <james.clark@arm.com>

Thanks, applied to perf-tools-next.

- Arnaldo