[PATCH 0/3] Fix for shellcheck issues with latest scripts in tests/shell

STALE997d

Revision v1 of 2 in this series.

21 messages, 7 authors, 2023-11-08 · open the first message on its own page

[PATCH 0/3] Fix for shellcheck issues with latest scripts in tests/shell

From: Athira Rajeev <hidden>
Date: 2023-09-29 04:12:43

shellcheck was run on perf tool shell scripts as a pre-requisite
to include a build option for shellcheck discussed here:
https://www.spinics.net/lists/linux-perf-users/msg25553.html

And fixes were added for the coding/formatting issues in
two patchsets:
https://lore.kernel.org/linux-perf-users/20230613164145.50488-1-atrajeev@linux.vnet.ibm.com/
https://lore.kernel.org/linux-perf-users/20230709182800.53002-1-atrajeev@linux.vnet.ibm.com/

Three additional issues were observed and fixes are part of:
https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/log/?h=perf-tools-next

With recent commits in perf, other three issues are observed.
shellcheck version: 0.6.0

With this patchset:

for F in $(find tests/shell/ -perm -o=x -name '*.sh'); do shellcheck -S warning $F; done
echo $?
0

The changes are with recent commits ( which is mentioned in each patch)
for ock_contention, record_sideband and test_arm_coresight testcases.
The changes are made on top of:
https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/log/?h=perf-tools-next

Athira Rajeev (3):
  perf tests test_arm_coresight: Fix the shellcheck warning in latest
    test_arm_coresight.sh
  tools/perf/tests Ignore the shellcheck SC2046 warning in
    lock_contentio
  tools/perf/tests: Fix shellcheck warning in record_sideband.sh test

 tools/perf/tests/shell/lock_contention.sh    | 1 +
 tools/perf/tests/shell/record_sideband.sh    | 2 +-
 tools/perf/tests/shell/test_arm_coresight.sh | 4 ++--
 3 files changed, 4 insertions(+), 3 deletions(-)

-- 
2.31.1

[PATCH 1/3] perf tests test_arm_coresight: Fix the shellcheck warning in latest test_arm_coresight.sh

From: Athira Rajeev <hidden>
Date: 2023-09-29 04:13:32

Running shellcheck on tests/shell/test_arm_coresight.sh
throws below warnings:

	In tests/shell/test_arm_coresight.sh line 15:
	cs_etm_path=$(find  /sys/bus/event_source/devices/cs_etm/ -name cpu* -print -quit)
                  ^--^ SC2061: Quote the parameter to -name so the shell won't interpret it.

	In tests/shell/test_arm_coresight.sh line 20:
		if [ $archhver -eq 5 -a "$(printf "0x%X\n" $archpart)" = "0xA13" ] ; then
                	             ^-- SC2166: Prefer [ p ] && [ q ] as [ p -a q ] is not well defined

This warning is observed after commit:
"commit bb350847965d ("perf test: Update cs_etm testcase for Arm ETE")"

Fixed this issue by using quoting 'cpu*' for SC2061 and
using "&&" in line number 20 for SC2166 warning

Fixes: bb350847965d ("perf test: Update cs_etm testcase for Arm ETE")
Signed-off-by: Athira Rajeev <redacted>
---
 tools/perf/tests/shell/test_arm_coresight.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/perf/tests/shell/test_arm_coresight.sh b/tools/perf/tests/shell/test_arm_coresight.sh
index fe78c4626e45..f2115dfa24a5 100755
--- a/tools/perf/tests/shell/test_arm_coresight.sh
+++ b/tools/perf/tests/shell/test_arm_coresight.sh
@@ -12,12 +12,12 @@
 glb_err=0
 
 cs_etm_dev_name() {
-	cs_etm_path=$(find  /sys/bus/event_source/devices/cs_etm/ -name cpu* -print -quit)
+	cs_etm_path=$(find  /sys/bus/event_source/devices/cs_etm/ -name 'cpu*' -print -quit)
 	trcdevarch=$(cat ${cs_etm_path}/mgmt/trcdevarch)
 	archhver=$((($trcdevarch >> 12) & 0xf))
 	archpart=$(($trcdevarch & 0xfff))
 
-	if [ $archhver -eq 5 -a "$(printf "0x%X\n" $archpart)" = "0xA13" ] ; then
+	if [ $archhver -eq 5 ] && [ "$(printf "0x%X\n" $archpart)" = "0xA13" ] ; then
 		echo "ete"
 	else
 		echo "etm"
-- 
2.31.1

Re: [PATCH 1/3] perf tests test_arm_coresight: Fix the shellcheck warning in latest test_arm_coresight.sh

From: Namhyung Kim <namhyung@kernel.org>
Date: 2023-10-05 05:03:05

On Thu, Sep 28, 2023 at 9:11 PM Athira Rajeev
[off-list ref] wrote:
Running shellcheck on tests/shell/test_arm_coresight.sh
throws below warnings:

        In tests/shell/test_arm_coresight.sh line 15:
        cs_etm_path=$(find  /sys/bus/event_source/devices/cs_etm/ -name cpu* -print -quit)
                  ^--^ SC2061: Quote the parameter to -name so the shell won't interpret it.

        In tests/shell/test_arm_coresight.sh line 20:
                if [ $archhver -eq 5 -a "$(printf "0x%X\n" $archpart)" = "0xA13" ] ; then
                                     ^-- SC2166: Prefer [ p ] && [ q ] as [ p -a q ] is not well defined

This warning is observed after commit:
"commit bb350847965d ("perf test: Update cs_etm testcase for Arm ETE")"

Fixed this issue by using quoting 'cpu*' for SC2061 and
using "&&" in line number 20 for SC2166 warning

Fixes: bb350847965d ("perf test: Update cs_etm testcase for Arm ETE")
Signed-off-by: Athira Rajeev <redacted>
You'd better add Coresight folks on this.
Maybe this file was missing in the MAINTAINERS file.

Thanks,
Namhyung

quoted hunk
---
 tools/perf/tests/shell/test_arm_coresight.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/perf/tests/shell/test_arm_coresight.sh b/tools/perf/tests/shell/test_arm_coresight.sh
index fe78c4626e45..f2115dfa24a5 100755
--- a/tools/perf/tests/shell/test_arm_coresight.sh
+++ b/tools/perf/tests/shell/test_arm_coresight.sh
@@ -12,12 +12,12 @@
 glb_err=0

 cs_etm_dev_name() {
-       cs_etm_path=$(find  /sys/bus/event_source/devices/cs_etm/ -name cpu* -print -quit)
+       cs_etm_path=$(find  /sys/bus/event_source/devices/cs_etm/ -name 'cpu*' -print -quit)
        trcdevarch=$(cat ${cs_etm_path}/mgmt/trcdevarch)
        archhver=$((($trcdevarch >> 12) & 0xf))
        archpart=$(($trcdevarch & 0xfff))

-       if [ $archhver -eq 5 -a "$(printf "0x%X\n" $archpart)" = "0xA13" ] ; then
+       if [ $archhver -eq 5 ] && [ "$(printf "0x%X\n" $archpart)" = "0xA13" ] ; then
                echo "ete"
        else
                echo "etm"
--
2.31.1

Re: [PATCH 1/3] perf tests test_arm_coresight: Fix the shellcheck warning in latest test_arm_coresight.sh

From: James Clark <hidden>
Date: 2023-10-05 08:29:21


On 05/10/2023 06:02, Namhyung Kim wrote:
On Thu, Sep 28, 2023 at 9:11 PM Athira Rajeev
[off-list ref] wrote:
quoted
Running shellcheck on tests/shell/test_arm_coresight.sh
throws below warnings:

        In tests/shell/test_arm_coresight.sh line 15:
        cs_etm_path=$(find  /sys/bus/event_source/devices/cs_etm/ -name cpu* -print -quit)
                  ^--^ SC2061: Quote the parameter to -name so the shell won't interpret it.

        In tests/shell/test_arm_coresight.sh line 20:
                if [ $archhver -eq 5 -a "$(printf "0x%X\n" $archpart)" = "0xA13" ] ; then
                                     ^-- SC2166: Prefer [ p ] && [ q ] as [ p -a q ] is not well defined

This warning is observed after commit:
"commit bb350847965d ("perf test: Update cs_etm testcase for Arm ETE")"

Fixed this issue by using quoting 'cpu*' for SC2061 and
using "&&" in line number 20 for SC2166 warning

Fixes: bb350847965d ("perf test: Update cs_etm testcase for Arm ETE")
Signed-off-by: Athira Rajeev <redacted>
You'd better add Coresight folks on this.
Maybe this file was missing in the MAINTAINERS file.

Thanks,
Namhyung
get_maintainer already does a pretty good job with the defaults, but
maybe I can try to add all the Coresight Perf stuff in the maintainers file.
quoted
---
 tools/perf/tests/shell/test_arm_coresight.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/perf/tests/shell/test_arm_coresight.sh b/tools/perf/tests/shell/test_arm_coresight.sh
index fe78c4626e45..f2115dfa24a5 100755
--- a/tools/perf/tests/shell/test_arm_coresight.sh
+++ b/tools/perf/tests/shell/test_arm_coresight.sh
@@ -12,12 +12,12 @@
 glb_err=0

 cs_etm_dev_name() {
-       cs_etm_path=$(find  /sys/bus/event_source/devices/cs_etm/ -name cpu* -print -quit)
+       cs_etm_path=$(find  /sys/bus/event_source/devices/cs_etm/ -name 'cpu*' -print -quit)
        trcdevarch=$(cat ${cs_etm_path}/mgmt/trcdevarch)
        archhver=$((($trcdevarch >> 12) & 0xf))
        archpart=$(($trcdevarch & 0xfff))

-       if [ $archhver -eq 5 -a "$(printf "0x%X\n" $archpart)" = "0xA13" ] ; then
+       if [ $archhver -eq 5 ] && [ "$(printf "0x%X\n" $archpart)" = "0xA13" ] ; then
                echo "ete"
        else
                echo "etm"
--
2.31.1

Re: [PATCH 1/3] perf tests test_arm_coresight: Fix the shellcheck warning in latest test_arm_coresight.sh

From: Suzuki K Poulose <suzuki.poulose@arm.com>
Date: 2023-10-05 09:44:19

On 05/10/2023 06:02, Namhyung Kim wrote:
On Thu, Sep 28, 2023 at 9:11 PM Athira Rajeev
[off-list ref] wrote:
quoted
Running shellcheck on tests/shell/test_arm_coresight.sh
throws below warnings:

         In tests/shell/test_arm_coresight.sh line 15:
         cs_etm_path=$(find  /sys/bus/event_source/devices/cs_etm/ -name cpu* -print -quit)
                   ^--^ SC2061: Quote the parameter to -name so the shell won't interpret it.

         In tests/shell/test_arm_coresight.sh line 20:
                 if [ $archhver -eq 5 -a "$(printf "0x%X\n" $archpart)" = "0xA13" ] ; then
                                      ^-- SC2166: Prefer [ p ] && [ q ] as [ p -a q ] is not well defined

This warning is observed after commit:
"commit bb350847965d ("perf test: Update cs_etm testcase for Arm ETE")"

Fixed this issue by using quoting 'cpu*' for SC2061 and
using "&&" in line number 20 for SC2166 warning

Fixes: bb350847965d ("perf test: Update cs_etm testcase for Arm ETE")
Signed-off-by: Athira Rajeev <redacted>
Thanks for the fix.

Nothing to do with this patch, but I am wondering if the original patch
is over engineered and may not be future proof.

e.g.,

cs_etm_dev_name() {
+	cs_etm_path=$(find  /sys/bus/event_source/devices/cs_etm/ -name cpu* 
-print -quit)

Right there you got the device name and we can easily deduce the name of
the "ETM" node.

e.g,:
	etm=$(basename $(readlink cs_etm_path) | sed "s/[0-9]\+$//")

And practically, nobody prevents an ETE mixed with an ETM on a "hybrid"
system (hopefully, no one builds it ;-))

Also, instead of hardcoding "ete" and "etm" prefixes from the arch part,
we should simply use the cpu nodes from :

/sys/bus/event_source/devices/cs_etm/

e.g.,

arm_cs_etm_traverse_path_test() {
	# Iterate for every ETM device
	for c in /sys/bus/event_source/devices/cs_etm/cpu*; do
		# Read the link to be on the safer side
		dev=`readlink $c`
		
		# Find the ETM device belonging to which CPU
		cpu=`cat $dev/cpu`

		# Use depth-first search (DFS) to iterate outputs
		arm_cs_iterate_devices $dev $cpu
	done;
}


You'd better add Coresight folks on this.
Maybe this file was missing in the MAINTAINERS file.
And the original author of the commit, that introduced the issue too.

Suzuki
Thanks,
Namhyung

quoted
---
  tools/perf/tests/shell/test_arm_coresight.sh | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/perf/tests/shell/test_arm_coresight.sh b/tools/perf/tests/shell/test_arm_coresight.sh
index fe78c4626e45..f2115dfa24a5 100755
--- a/tools/perf/tests/shell/test_arm_coresight.sh
+++ b/tools/perf/tests/shell/test_arm_coresight.sh
@@ -12,12 +12,12 @@
  glb_err=0

  cs_etm_dev_name() {
-       cs_etm_path=$(find  /sys/bus/event_source/devices/cs_etm/ -name cpu* -print -quit)
+       cs_etm_path=$(find  /sys/bus/event_source/devices/cs_etm/ -name 'cpu*' -print -quit)
         trcdevarch=$(cat ${cs_etm_path}/mgmt/trcdevarch)
         archhver=$((($trcdevarch >> 12) & 0xf))
         archpart=$(($trcdevarch & 0xfff))

-       if [ $archhver -eq 5 -a "$(printf "0x%X\n" $archpart)" = "0xA13" ] ; then
+       if [ $archhver -eq 5 ] && [ "$(printf "0x%X\n" $archpart)" = "0xA13" ] ; then
                 echo "ete"
         else
                 echo "etm"
--
2.31.1

Re: [PATCH 1/3] perf tests test_arm_coresight: Fix the shellcheck warning in latest test_arm_coresight.sh

From: Athira Rajeev <hidden>
Date: 2023-10-12 15:58:11

On 05-Oct-2023, at 3:06 PM, Suzuki K Poulose [off-list ref] wrote:

On 05/10/2023 06:02, Namhyung Kim wrote:
quoted
On Thu, Sep 28, 2023 at 9:11 PM Athira Rajeev
[off-list ref] wrote:
quoted
Running shellcheck on tests/shell/test_arm_coresight.sh
throws below warnings:

        In tests/shell/test_arm_coresight.sh line 15:
        cs_etm_path=$(find  /sys/bus/event_source/devices/cs_etm/ -name cpu* -print -quit)
                  ^--^ SC2061: Quote the parameter to -name so the shell won't interpret it.

        In tests/shell/test_arm_coresight.sh line 20:
                if [ $archhver -eq 5 -a "$(printf "0x%X\n" $archpart)" = "0xA13" ] ; then
                                     ^-- SC2166: Prefer [ p ] && [ q ] as [ p -a q ] is not well defined

This warning is observed after commit:
"commit bb350847965d ("perf test: Update cs_etm testcase for Arm ETE")"

Fixed this issue by using quoting 'cpu*' for SC2061 and
using "&&" in line number 20 for SC2166 warning

Fixes: bb350847965d ("perf test: Update cs_etm testcase for Arm ETE")
Signed-off-by: Athira Rajeev <redacted>
Thanks for the fix.

Nothing to do with this patch, but I am wondering if the original patch
is over engineered and may not be future proof.

e.g.,

cs_etm_dev_name() {
+ cs_etm_path=$(find  /sys/bus/event_source/devices/cs_etm/ -name cpu* -print -quit)

Right there you got the device name and we can easily deduce the name of
the "ETM" node.

e.g,:
etm=$(basename $(readlink cs_etm_path) | sed "s/[0-9]\+$//")

And practically, nobody prevents an ETE mixed with an ETM on a "hybrid"
system (hopefully, no one builds it ;-))

Also, instead of hardcoding "ete" and "etm" prefixes from the arch part,
we should simply use the cpu nodes from :

/sys/bus/event_source/devices/cs_etm/

e.g.,

arm_cs_etm_traverse_path_test() {
# Iterate for every ETM device
for c in /sys/bus/event_source/devices/cs_etm/cpu*; do
# Read the link to be on the safer side
dev=`readlink $c`

# Find the ETM device belonging to which CPU
cpu=`cat $dev/cpu`

# Use depth-first search (DFS) to iterate outputs
arm_cs_iterate_devices $dev $cpu
done;
}


quoted
You'd better add Coresight folks on this.
Maybe this file was missing in the MAINTAINERS file.
And the original author of the commit, that introduced the issue too.

Suzuki
Hi All,
Thanks for the discussion and feedbacks.

This patch fixes the shellcheck warning introduced in function "cs_etm_dev_name". But with the changes that Suzuki suggested, we won't need the function "cs_etm_dev_name" since the code will use "/sys/bus/event_source/devices/cs_etm/" .  In that case, can I drop this patch for now from this series ?

Thanks
Athira
quoted
Thanks,
Namhyung
quoted
---
 tools/perf/tests/shell/test_arm_coresight.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/perf/tests/shell/test_arm_coresight.sh b/tools/perf/tests/shell/test_arm_coresight.sh
index fe78c4626e45..f2115dfa24a5 100755
--- a/tools/perf/tests/shell/test_arm_coresight.sh
+++ b/tools/perf/tests/shell/test_arm_coresight.sh
@@ -12,12 +12,12 @@
 glb_err=0

 cs_etm_dev_name() {
-       cs_etm_path=$(find  /sys/bus/event_source/devices/cs_etm/ -name cpu* -print -quit)
+       cs_etm_path=$(find  /sys/bus/event_source/devices/cs_etm/ -name 'cpu*' -print -quit)
        trcdevarch=$(cat ${cs_etm_path}/mgmt/trcdevarch)
        archhver=$((($trcdevarch >> 12) & 0xf))
        archpart=$(($trcdevarch & 0xfff))

-       if [ $archhver -eq 5 -a "$(printf "0x%X\n" $archpart)" = "0xA13" ] ; then
+       if [ $archhver -eq 5 ] && [ "$(printf "0x%X\n" $archpart)" = "0xA13" ] ; then
                echo "ete"
        else
                echo "etm"
--
2.31.1

Re: [PATCH 1/3] perf tests test_arm_coresight: Fix the shellcheck warning in latest test_arm_coresight.sh

From: Suzuki K Poulose <suzuki.poulose@arm.com>
Date: 2023-10-12 16:08:48

Hi,

On 12/10/2023 16:56, Athira Rajeev wrote:
quoted
On 05-Oct-2023, at 3:06 PM, Suzuki K Poulose [off-list ref] wrote:

On 05/10/2023 06:02, Namhyung Kim wrote:
quoted
On Thu, Sep 28, 2023 at 9:11 PM Athira Rajeev
[off-list ref] wrote:
quoted
...
quoted
Thanks for the fix.

Nothing to do with this patch, but I am wondering if the original patch
is over engineered and may not be future proof.

e.g.,

cs_etm_dev_name() {
+ cs_etm_path=$(find  /sys/bus/event_source/devices/cs_etm/ -name cpu* -print -quit)

Right there you got the device name and we can easily deduce the name of
the "ETM" node.

e.g,:
etm=$(basename $(readlink cs_etm_path) | sed "s/[0-9]\+$//")

And practically, nobody prevents an ETE mixed with an ETM on a "hybrid"
system (hopefully, no one builds it ;-))

Also, instead of hardcoding "ete" and "etm" prefixes from the arch part,
we should simply use the cpu nodes from :

/sys/bus/event_source/devices/cs_etm/

e.g.,

arm_cs_etm_traverse_path_test() {
# Iterate for every ETM device
for c in /sys/bus/event_source/devices/cs_etm/cpu*; do
# Read the link to be on the safer side
dev=`readlink $c`

# Find the ETM device belonging to which CPU
cpu=`cat $dev/cpu`

# Use depth-first search (DFS) to iterate outputs
arm_cs_iterate_devices $dev $cpu
done;
}


quoted
You'd better add Coresight folks on this.
Maybe this file was missing in the MAINTAINERS file.
And the original author of the commit, that introduced the issue too.

Suzuki
Hi All,
Thanks for the discussion and feedbacks.

This patch fixes the shellcheck warning introduced in function "cs_etm_dev_name". But with the changes that Suzuki suggested, we won't need the function "cs_etm_dev_name" since the code will use "/sys/bus/event_source/devices/cs_etm/" .  In that case, can I drop this patch for now from this series ?
Yes, please. James will send out the proposed patch

Suzuki

Re: [PATCH 1/3] perf tests test_arm_coresight: Fix the shellcheck warning in latest test_arm_coresight.sh

From: Athira Rajeev <hidden>
Date: 2023-10-12 16:22:01

On 12-Oct-2023, at 9:37 PM, Suzuki K Poulose [off-list ref] wrote:

Hi,

On 12/10/2023 16:56, Athira Rajeev wrote:
quoted
quoted
On 05-Oct-2023, at 3:06 PM, Suzuki K Poulose [off-list ref] wrote:

On 05/10/2023 06:02, Namhyung Kim wrote:
quoted
On Thu, Sep 28, 2023 at 9:11 PM Athira Rajeev
[off-list ref] wrote:
quoted
...
quoted
quoted
Thanks for the fix.

Nothing to do with this patch, but I am wondering if the original patch
is over engineered and may not be future proof.

e.g.,

cs_etm_dev_name() {
+ cs_etm_path=$(find  /sys/bus/event_source/devices/cs_etm/ -name cpu* -print -quit)

Right there you got the device name and we can easily deduce the name of
the "ETM" node.

e.g,:
etm=$(basename $(readlink cs_etm_path) | sed "s/[0-9]\+$//")

And practically, nobody prevents an ETE mixed with an ETM on a "hybrid"
system (hopefully, no one builds it ;-))

Also, instead of hardcoding "ete" and "etm" prefixes from the arch part,
we should simply use the cpu nodes from :

/sys/bus/event_source/devices/cs_etm/

e.g.,

arm_cs_etm_traverse_path_test() {
# Iterate for every ETM device
for c in /sys/bus/event_source/devices/cs_etm/cpu*; do
# Read the link to be on the safer side
dev=`readlink $c`

# Find the ETM device belonging to which CPU
cpu=`cat $dev/cpu`

# Use depth-first search (DFS) to iterate outputs
arm_cs_iterate_devices $dev $cpu
done;
}


quoted
You'd better add Coresight folks on this.
Maybe this file was missing in the MAINTAINERS file.
And the original author of the commit, that introduced the issue too.

Suzuki
Hi All,
Thanks for the discussion and feedbacks.
This patch fixes the shellcheck warning introduced in function "cs_etm_dev_name". But with the changes that Suzuki suggested, we won't need the function "cs_etm_dev_name" since the code will use "/sys/bus/event_source/devices/cs_etm/" .  In that case, can I drop this patch for now from this series ?
Yes, please. James will send out the proposed patch
Hi Suzuki,

Sure. Thanks! 

Athira
Suzuki

Re: [PATCH 1/3] perf tests test_arm_coresight: Fix the shellcheck warning in latest test_arm_coresight.sh

From: James Clark <hidden>
Date: 2023-10-05 08:34:18


On 29/09/2023 05:11, Athira Rajeev wrote:
quoted hunk
Running shellcheck on tests/shell/test_arm_coresight.sh
throws below warnings:

	In tests/shell/test_arm_coresight.sh line 15:
	cs_etm_path=$(find  /sys/bus/event_source/devices/cs_etm/ -name cpu* -print -quit)
                  ^--^ SC2061: Quote the parameter to -name so the shell won't interpret it.

	In tests/shell/test_arm_coresight.sh line 20:
		if [ $archhver -eq 5 -a "$(printf "0x%X\n" $archpart)" = "0xA13" ] ; then
                	             ^-- SC2166: Prefer [ p ] && [ q ] as [ p -a q ] is not well defined

This warning is observed after commit:
"commit bb350847965d ("perf test: Update cs_etm testcase for Arm ETE")"

Fixed this issue by using quoting 'cpu*' for SC2061 and
using "&&" in line number 20 for SC2166 warning

Fixes: bb350847965d ("perf test: Update cs_etm testcase for Arm ETE")
Signed-off-by: Athira Rajeev <redacted>
---
 tools/perf/tests/shell/test_arm_coresight.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/perf/tests/shell/test_arm_coresight.sh b/tools/perf/tests/shell/test_arm_coresight.sh
index fe78c4626e45..f2115dfa24a5 100755
--- a/tools/perf/tests/shell/test_arm_coresight.sh
+++ b/tools/perf/tests/shell/test_arm_coresight.sh
@@ -12,12 +12,12 @@
 glb_err=0
 
 cs_etm_dev_name() {
-	cs_etm_path=$(find  /sys/bus/event_source/devices/cs_etm/ -name cpu* -print -quit)
+	cs_etm_path=$(find  /sys/bus/event_source/devices/cs_etm/ -name 'cpu*' -print -quit)
 	trcdevarch=$(cat ${cs_etm_path}/mgmt/trcdevarch)
 	archhver=$((($trcdevarch >> 12) & 0xf))
 	archpart=$(($trcdevarch & 0xfff))
 
-	if [ $archhver -eq 5 -a "$(printf "0x%X\n" $archpart)" = "0xA13" ] ; then
+	if [ $archhver -eq 5 ] && [ "$(printf "0x%X\n" $archpart)" = "0xA13" ] ; then
 		echo "ete"
 	else
 		echo "etm"

Reviewed-by: James Clark <redacted>

Re: [PATCH 1/3] perf tests test_arm_coresight: Fix the shellcheck warning in latest test_arm_coresight.sh

From: Athira Rajeev <hidden>
Date: 2023-10-05 12:38:14

On 05-Oct-2023, at 1:50 PM, James Clark [off-list ref] wrote:



On 29/09/2023 05:11, Athira Rajeev wrote:
quoted
Running shellcheck on tests/shell/test_arm_coresight.sh
throws below warnings:

In tests/shell/test_arm_coresight.sh line 15:
cs_etm_path=$(find  /sys/bus/event_source/devices/cs_etm/ -name cpu* -print -quit)
                 ^--^ SC2061: Quote the parameter to -name so the shell won't interpret it.

In tests/shell/test_arm_coresight.sh line 20:
if [ $archhver -eq 5 -a "$(printf "0x%X\n" $archpart)" = "0xA13" ] ; then
                             ^-- SC2166: Prefer [ p ] && [ q ] as [ p -a q ] is not well defined

This warning is observed after commit:
"commit bb350847965d ("perf test: Update cs_etm testcase for Arm ETE")"

Fixed this issue by using quoting 'cpu*' for SC2061 and
using "&&" in line number 20 for SC2166 warning

Fixes: bb350847965d ("perf test: Update cs_etm testcase for Arm ETE")
Signed-off-by: Athira Rajeev <redacted>
---
tools/perf/tests/shell/test_arm_coresight.sh | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/perf/tests/shell/test_arm_coresight.sh b/tools/perf/tests/shell/test_arm_coresight.sh
index fe78c4626e45..f2115dfa24a5 100755
--- a/tools/perf/tests/shell/test_arm_coresight.sh
+++ b/tools/perf/tests/shell/test_arm_coresight.sh
@@ -12,12 +12,12 @@
glb_err=0

cs_etm_dev_name() {
- cs_etm_path=$(find  /sys/bus/event_source/devices/cs_etm/ -name cpu* -print -quit)
+ cs_etm_path=$(find  /sys/bus/event_source/devices/cs_etm/ -name 'cpu*' -print -quit)
trcdevarch=$(cat ${cs_etm_path}/mgmt/trcdevarch)
archhver=$((($trcdevarch >> 12) & 0xf))
archpart=$(($trcdevarch & 0xfff))

- if [ $archhver -eq 5 -a "$(printf "0x%X\n" $archpart)" = "0xA13" ] ; then
+ if [ $archhver -eq 5 ] && [ "$(printf "0x%X\n" $archpart)" = "0xA13" ] ; then
echo "ete"
else
echo "etm"

Reviewed-by: James Clark <redacted>
Thanks James for checking

Athira

Re: [PATCH 1/3] perf tests test_arm_coresight: Fix the shellcheck warning in latest test_arm_coresight.sh

From: Arnaldo Carvalho de Melo <acme@kernel.org>
Date: 2023-11-06 21:45:11

Em Thu, Oct 05, 2023 at 02:24:15PM +0530, Athira Rajeev escreveu:
quoted
On 05-Oct-2023, at 1:50 PM, James Clark [off-list ref] wrote:
On 29/09/2023 05:11, Athira Rajeev wrote:
quoted
Running shellcheck on tests/shell/test_arm_coresight.sh
throws below warnings:

In tests/shell/test_arm_coresight.sh line 15:
cs_etm_path=$(find  /sys/bus/event_source/devices/cs_etm/ -name cpu* -print -quit)
                 ^--^ SC2061: Quote the parameter to -name so the shell won't interpret it.

In tests/shell/test_arm_coresight.sh line 20:
if [ $archhver -eq 5 -a "$(printf "0x%X\n" $archpart)" = "0xA13" ] ; then
                             ^-- SC2166: Prefer [ p ] && [ q ] as [ p -a q ] is not well defined

This warning is observed after commit:
"commit bb350847965d ("perf test: Update cs_etm testcase for Arm ETE")"

Fixed this issue by using quoting 'cpu*' for SC2061 and
using "&&" in line number 20 for SC2166 warning

Fixes: bb350847965d ("perf test: Update cs_etm testcase for Arm ETE")
Signed-off-by: Athira Rajeev <redacted>
---
tools/perf/tests/shell/test_arm_coresight.sh | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/perf/tests/shell/test_arm_coresight.sh b/tools/perf/tests/shell/test_arm_coresight.sh
index fe78c4626e45..f2115dfa24a5 100755
--- a/tools/perf/tests/shell/test_arm_coresight.sh
+++ b/tools/perf/tests/shell/test_arm_coresight.sh
@@ -12,12 +12,12 @@
glb_err=0

cs_etm_dev_name() {
- cs_etm_path=$(find  /sys/bus/event_source/devices/cs_etm/ -name cpu* -print -quit)
+ cs_etm_path=$(find  /sys/bus/event_source/devices/cs_etm/ -name 'cpu*' -print -quit)
trcdevarch=$(cat ${cs_etm_path}/mgmt/trcdevarch)
archhver=$((($trcdevarch >> 12) & 0xf))
archpart=$(($trcdevarch & 0xfff))

- if [ $archhver -eq 5 -a "$(printf "0x%X\n" $archpart)" = "0xA13" ] ; then
+ if [ $archhver -eq 5 ] && [ "$(printf "0x%X\n" $archpart)" = "0xA13" ] ; then
echo "ete"
else
echo "etm"

Reviewed-by: James Clark <redacted>
Some are not applying, even after b4 picking up v2

Total patches: 3
---
Cover: ./v2_20231013_atrajeev_fix_for_shellcheck_issues_with_latest_scripts_in_tests_shell.cover
 Link: https://lore.kernel.org/r/20231013073021.99794-1-atrajeev@linux.vnet.ibm.com
 Base: not specified
       git am ./v2_20231013_atrajeev_fix_for_shellcheck_issues_with_latest_scripts_in_tests_shell.mbx
⬢[acme@toolbox perf-tools-next]$        git am ./v2_20231013_atrajeev_fix_for_shellcheck_issues_with_latest_scripts_in_tests_shell.mbx
Applying: tools/perf/tests Ignore the shellcheck SC2046 warning in lock_contention
error: patch failed: tools/perf/tests/shell/lock_contention.sh:33
error: tools/perf/tests/shell/lock_contention.sh: patch does not apply
Patch failed at 0001 tools/perf/tests Ignore the shellcheck SC2046 warning in lock_contention
hint: Use 'git am --show-current-patch=diff' to see the failed patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
⬢[acme@toolbox perf-tools-next]$ git am --abort
⬢[acme@toolbox perf-tools-next]$

Re: [PATCH 1/3] perf tests test_arm_coresight: Fix the shellcheck warning in latest test_arm_coresight.sh

From: Athira Rajeev <hidden>
Date: 2023-11-07 06:39:49

On 07-Nov-2023, at 3:14 AM, Arnaldo Carvalho de Melo [off-list ref] wrote:

Em Thu, Oct 05, 2023 at 02:24:15PM +0530, Athira Rajeev escreveu:
quoted
quoted
On 05-Oct-2023, at 1:50 PM, James Clark [off-list ref] wrote:
On 29/09/2023 05:11, Athira Rajeev wrote:
quoted
Running shellcheck on tests/shell/test_arm_coresight.sh
throws below warnings:

In tests/shell/test_arm_coresight.sh line 15:
cs_etm_path=$(find  /sys/bus/event_source/devices/cs_etm/ -name cpu* -print -quit)
                ^--^ SC2061: Quote the parameter to -name so the shell won't interpret it.

In tests/shell/test_arm_coresight.sh line 20:
if [ $archhver -eq 5 -a "$(printf "0x%X\n" $archpart)" = "0xA13" ] ; then
                            ^-- SC2166: Prefer [ p ] && [ q ] as [ p -a q ] is not well defined

This warning is observed after commit:
"commit bb350847965d ("perf test: Update cs_etm testcase for Arm ETE")"

Fixed this issue by using quoting 'cpu*' for SC2061 and
using "&&" in line number 20 for SC2166 warning

Fixes: bb350847965d ("perf test: Update cs_etm testcase for Arm ETE")
Signed-off-by: Athira Rajeev <redacted>
---
tools/perf/tests/shell/test_arm_coresight.sh | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/perf/tests/shell/test_arm_coresight.sh b/tools/perf/tests/shell/test_arm_coresight.sh
index fe78c4626e45..f2115dfa24a5 100755
--- a/tools/perf/tests/shell/test_arm_coresight.sh
+++ b/tools/perf/tests/shell/test_arm_coresight.sh
@@ -12,12 +12,12 @@
glb_err=0

cs_etm_dev_name() {
- cs_etm_path=$(find  /sys/bus/event_source/devices/cs_etm/ -name cpu* -print -quit)
+ cs_etm_path=$(find  /sys/bus/event_source/devices/cs_etm/ -name 'cpu*' -print -quit)
trcdevarch=$(cat ${cs_etm_path}/mgmt/trcdevarch)
archhver=$((($trcdevarch >> 12) & 0xf))
archpart=$(($trcdevarch & 0xfff))

- if [ $archhver -eq 5 -a "$(printf "0x%X\n" $archpart)" = "0xA13" ] ; then
+ if [ $archhver -eq 5 ] && [ "$(printf "0x%X\n" $archpart)" = "0xA13" ] ; then
echo "ete"
else
echo "etm"

Reviewed-by: James Clark <redacted>
Some are not applying, even after b4 picking up v2

Total patches: 3
---
Cover: ./v2_20231013_atrajeev_fix_for_shellcheck_issues_with_latest_scripts_in_tests_shell.cover
Link: https://lore.kernel.org/r/20231013073021.99794-1-atrajeev@linux.vnet.ibm.com
Base: not specified
      git am ./v2_20231013_atrajeev_fix_for_shellcheck_issues_with_latest_scripts_in_tests_shell.mbx
⬢[acme@toolbox perf-tools-next]$        git am ./v2_20231013_atrajeev_fix_for_shellcheck_issues_with_latest_scripts_in_tests_shell.mbx
Applying: tools/perf/tests Ignore the shellcheck SC2046 warning in lock_contention
error: patch failed: tools/perf/tests/shell/lock_contention.sh:33
error: tools/perf/tests/shell/lock_contention.sh: patch does not apply
Patch failed at 0001 tools/perf/tests Ignore the shellcheck SC2046 warning in lock_contention
hint: Use 'git am --show-current-patch=diff' to see the failed patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
⬢[acme@toolbox perf-tools-next]$ git am --abort
⬢[acme@toolbox perf-tools-next]$
Hi Arnaldo

The patch is picked up : https://lore.kernel.org/all/169757198796.167943.10552920255799914362.b4-ty@kernel.org/ .
Thanks for looking into.

Athira

Re: [PATCH 1/3] perf tests test_arm_coresight: Fix the shellcheck warning in latest test_arm_coresight.sh

From: Arnaldo Carvalho de Melo <acme@kernel.org>
Date: 2023-11-08 20:05:40

Em Tue, Nov 07, 2023 at 12:08:25PM +0530, Athira Rajeev escreveu:
quoted
On 07-Nov-2023, at 3:14 AM, Arnaldo Carvalho de Melo [off-list ref] wrote:
quoted
quoted
Reviewed-by: James Clark <redacted>
quoted
Some are not applying, even after b4 picking up v2
quoted
Total patches: 3
quoted
Cover: ./v2_20231013_atrajeev_fix_for_shellcheck_issues_with_latest_scripts_in_tests_shell.cover
Link: https://lore.kernel.org/r/20231013073021.99794-1-atrajeev@linux.vnet.ibm.com
Base: not specified
      git am ./v2_20231013_atrajeev_fix_for_shellcheck_issues_with_latest_scripts_in_tests_shell.mbx
⬢[acme@toolbox perf-tools-next]$        git am ./v2_20231013_atrajeev_fix_for_shellcheck_issues_with_latest_scripts_in_tests_shell.mbx
Applying: tools/perf/tests Ignore the shellcheck SC2046 warning in lock_contention
error: patch failed: tools/perf/tests/shell/lock_contention.sh:33
error: tools/perf/tests/shell/lock_contention.sh: patch does not apply
Patch failed at 0001 tools/perf/tests Ignore the shellcheck SC2046 warning in lock_contention
hint: Use 'git am --show-current-patch=diff' to see the failed patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
⬢[acme@toolbox perf-tools-next]$ git am --abort
⬢[acme@toolbox perf-tools-next]$
 
The patch is picked up : https://lore.kernel.org/all/169757198796.167943.10552920255799914362.b4-ty@kernel.org/ .
Thanks for looking into.
Thanks for checking, my mistake then,

- Arnaldo

RE: [PATCH 1/3] perf tests test_arm_coresight: Fix the shellcheck warning in latest test_arm_coresight.sh

From: David Laight <hidden>
Date: 2023-10-05 10:16:18

From: Athira Rajeev
quoted hunk
Sent: 29 September 2023 05:12

Running shellcheck on tests/shell/test_arm_coresight.sh
throws below warnings:

	In tests/shell/test_arm_coresight.sh line 15:
	cs_etm_path=$(find  /sys/bus/event_source/devices/cs_etm/ -name cpu* -print -quit)
                  ^--^ SC2061: Quote the parameter to -name so the shell won't interpret it.

	In tests/shell/test_arm_coresight.sh line 20:
		if [ $archhver -eq 5 -a "$(printf "0x%X\n" $archpart)" = "0xA13" ] ; then
                	             ^-- SC2166: Prefer [ p ] && [ q ] as [ p -a q ] is not well defined

This warning is observed after commit:
"commit bb350847965d ("perf test: Update cs_etm testcase for Arm ETE")"

Fixed this issue by using quoting 'cpu*' for SC2061 and
using "&&" in line number 20 for SC2166 warning

Fixes: bb350847965d ("perf test: Update cs_etm testcase for Arm ETE")
Signed-off-by: Athira Rajeev <redacted>
---
 tools/perf/tests/shell/test_arm_coresight.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/perf/tests/shell/test_arm_coresight.sh
b/tools/perf/tests/shell/test_arm_coresight.sh
index fe78c4626e45..f2115dfa24a5 100755
--- a/tools/perf/tests/shell/test_arm_coresight.sh
+++ b/tools/perf/tests/shell/test_arm_coresight.sh
@@ -12,12 +12,12 @@
 glb_err=0

 cs_etm_dev_name() {
-	cs_etm_path=$(find  /sys/bus/event_source/devices/cs_etm/ -name cpu* -print -quit)
+	cs_etm_path=$(find  /sys/bus/event_source/devices/cs_etm/ -name 'cpu*' -print -quit)
Isn't the intention to get the shell to expand "cpu* ?
So quoting it completely breaks the script.
 	trcdevarch=$(cat ${cs_etm_path}/mgmt/trcdevarch)
 	archhver=$((($trcdevarch >> 12) & 0xf))
 	archpart=$(($trcdevarch & 0xfff))

-	if [ $archhver -eq 5 -a "$(printf "0x%X\n" $archpart)" = "0xA13" ] ; then
+	if [ $archhver -eq 5 ] && [ "$(printf "0x%X\n" $archpart)" = "0xA13" ] ; then
The only issue I see there is that $archhver could be in "".
IIRC test is required to parse the 5 part expression "a op b -a c op d"
in the 'expected' manner even if any of a/b/c/d look like operators.
In any case '(' and ')' can be used to force the ordering.
Or, more usually, prepend an x as in:

if [ "x$archhver" = x5 -a "$(printf "0x%X\n" $archpart)" = "0xA13" ] ; then

	David
 		echo "ete"
 	else
 		echo "etm"
--
2.31.1
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

RE: [PATCH 1/3] perf tests test_arm_coresight: Fix the shellcheck warning in latest test_arm_coresight.sh

From: David Laight <hidden>
Date: 2023-10-05 15:46:16

From: David Laight
Sent: 05 October 2023 11:16
...
quoted
-	cs_etm_path=$(find  /sys/bus/event_source/devices/cs_etm/ -name cpu* -print -quit)
+	cs_etm_path=$(find  /sys/bus/event_source/devices/cs_etm/ -name 'cpu*' -print -quit)
Isn't the intention to get the shell to expand "cpu* ?
So quoting it completely breaks the script.
Complete brain-fade :-(

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

Re: [PATCH 1/3] perf tests test_arm_coresight: Fix the shellcheck warning in latest test_arm_coresight.sh

From: Athira Rajeev <hidden>
Date: 2023-10-12 15:49:08

On 05-Oct-2023, at 9:15 PM, David Laight [off-list ref] wrote:

From: David Laight
quoted
Sent: 05 October 2023 11:16
...
quoted
quoted
- cs_etm_path=$(find  /sys/bus/event_source/devices/cs_etm/ -name cpu* -print -quit)
+ cs_etm_path=$(find  /sys/bus/event_source/devices/cs_etm/ -name 'cpu*' -print -quit)
Isn't the intention to get the shell to expand "cpu* ?
So quoting it completely breaks the script.
Complete brain-fade :-(
Hi David,

Yeah, quoting it also will expand

Thanks
Athira
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

[PATCH 2/3] tools/perf/tests Ignore the shellcheck SC2046 warning in lock_contentio

From: Athira Rajeev <hidden>
Date: 2023-09-29 04:14:23

Running shellcheck on lock_contention.sh generates below
warning

	In tests/shell/lock_contention.sh line 36:
	   if [ `nproc` -lt 4 ]; then
		  ^-----^ SC2046: Quote this to prevent word splitting.

Here since nproc will generate a single word output
and there is no possibility of word splitting, this
warning can be ignored. Use exception for this with
"disable" option in shellcheck. This warning is observed
after commit:
"commit 29441ab3a30a ("perf test lock_contention.sh: Skip test
if not enough CPUs")"

Fixes: 29441ab3a30a ("perf test lock_contention.sh: Skip test if not enough CPUs")
Signed-off-by: Athira Rajeev <redacted>
---
 tools/perf/tests/shell/lock_contention.sh | 1 +
 1 file changed, 1 insertion(+)
diff --git a/tools/perf/tests/shell/lock_contention.sh b/tools/perf/tests/shell/lock_contention.sh
index d5a191d3d090..c1ec5762215b 100755
--- a/tools/perf/tests/shell/lock_contention.sh
+++ b/tools/perf/tests/shell/lock_contention.sh
@@ -33,6 +33,7 @@ check() {
 		exit
 	fi
 
+	# shellcheck disable=SC2046
 	if [ `nproc` -lt 4 ]; then
 		echo "[Skip] Low number of CPUs (`nproc`), lock event cannot be triggered certainly"
 		err=2
-- 
2.31.1

[PATCH 3/3] tools/perf/tests: Fix shellcheck warning in record_sideband.sh test

From: Athira Rajeev <hidden>
Date: 2023-09-29 04:15:20

Running shellcheck on record_sideband.sh throws below
warning:

	In tests/shell/record_sideband.sh line 25:
	  if ! perf record -o ${perfdata} -BN --no-bpf-event -C $1 true 2>&1 >/dev/null
	    ^--^ SC2069: To redirect stdout+stderr, 2>&1 must be last (or use '{ cmd > file; } 2>&1' to clarify).

This shows shellcheck warning SC2069 where the redirection
order needs to be fixed. Use { cmd > file; } 2>&1 to fix the
redirection of perf record output

Fixes: 23b97c7ee963 ("perf test: Add test case for record sideband events")
Signed-off-by: Athira Rajeev <redacted>
---
 tools/perf/tests/shell/record_sideband.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/perf/tests/shell/record_sideband.sh b/tools/perf/tests/shell/record_sideband.sh
index 5024a7ce0c51..7e036763a43c 100755
--- a/tools/perf/tests/shell/record_sideband.sh
+++ b/tools/perf/tests/shell/record_sideband.sh
@@ -22,7 +22,7 @@ trap trap_cleanup EXIT TERM INT
 
 can_cpu_wide()
 {
-    if ! perf record -o ${perfdata} -BN --no-bpf-event -C $1 true 2>&1 >/dev/null
+    if ! { perf record -o ${perfdata} -BN --no-bpf-event -C $1 true > /dev/null; } 2>&1
     then
         echo "record sideband test [Skipped cannot record cpu$1]"
         err=2
-- 
2.31.1

Re: [PATCH 3/3] tools/perf/tests: Fix shellcheck warning in record_sideband.sh test

From: Namhyung Kim <namhyung@kernel.org>
Date: 2023-10-05 05:05:22

On Thu, Sep 28, 2023 at 9:11 PM Athira Rajeev
[off-list ref] wrote:
quoted hunk
Running shellcheck on record_sideband.sh throws below
warning:

        In tests/shell/record_sideband.sh line 25:
          if ! perf record -o ${perfdata} -BN --no-bpf-event -C $1 true 2>&1 >/dev/null
            ^--^ SC2069: To redirect stdout+stderr, 2>&1 must be last (or use '{ cmd > file; } 2>&1' to clarify).

This shows shellcheck warning SC2069 where the redirection
order needs to be fixed. Use { cmd > file; } 2>&1 to fix the
redirection of perf record output

Fixes: 23b97c7ee963 ("perf test: Add test case for record sideband events")
Signed-off-by: Athira Rajeev <redacted>
---
 tools/perf/tests/shell/record_sideband.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/perf/tests/shell/record_sideband.sh b/tools/perf/tests/shell/record_sideband.sh
index 5024a7ce0c51..7e036763a43c 100755
--- a/tools/perf/tests/shell/record_sideband.sh
+++ b/tools/perf/tests/shell/record_sideband.sh
@@ -22,7 +22,7 @@ trap trap_cleanup EXIT TERM INT

 can_cpu_wide()
 {
-    if ! perf record -o ${perfdata} -BN --no-bpf-event -C $1 true 2>&1 >/dev/null
+    if ! { perf record -o ${perfdata} -BN --no-bpf-event -C $1 true > /dev/null; } 2>&1
I think we usually go without braces.

Thanks,
Namhyung

     then
         echo "record sideband test [Skipped cannot record cpu$1]"
         err=2
--
2.31.1

Re: [PATCH 3/3] tools/perf/tests: Fix shellcheck warning in record_sideband.sh test

From: Athira Rajeev <hidden>
Date: 2023-10-05 09:08:29

On 05-Oct-2023, at 10:34 AM, Namhyung Kim [off-list ref] wrote:

On Thu, Sep 28, 2023 at 9:11 PM Athira Rajeev
[off-list ref] wrote:
quoted
Running shellcheck on record_sideband.sh throws below
warning:

       In tests/shell/record_sideband.sh line 25:
         if ! perf record -o ${perfdata} -BN --no-bpf-event -C $1 true 2>&1 >/dev/null
           ^--^ SC2069: To redirect stdout+stderr, 2>&1 must be last (or use '{ cmd > file; } 2>&1' to clarify).

This shows shellcheck warning SC2069 where the redirection
order needs to be fixed. Use { cmd > file; } 2>&1 to fix the
redirection of perf record output

Fixes: 23b97c7ee963 ("perf test: Add test case for record sideband events")
Signed-off-by: Athira Rajeev <redacted>
---
tools/perf/tests/shell/record_sideband.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/perf/tests/shell/record_sideband.sh b/tools/perf/tests/shell/record_sideband.sh
index 5024a7ce0c51..7e036763a43c 100755
--- a/tools/perf/tests/shell/record_sideband.sh
+++ b/tools/perf/tests/shell/record_sideband.sh
@@ -22,7 +22,7 @@ trap trap_cleanup EXIT TERM INT
can_cpu_wide()
{
-    if ! perf record -o ${perfdata} -BN --no-bpf-event -C $1 true 2>&1 >/dev/null
+    if ! { perf record -o ${perfdata} -BN --no-bpf-event -C $1 true > /dev/null; } 2>&1
I think we usually go without braces.
Hi Namhyung

Thanks for reviving.I will fix this in V2

Thanks
Athira
Thanks,
Namhyung

quoted
    then
        echo "record sideband test [Skipped cannot record cpu$1]"
        err=2
--
2.31.1

Re: [PATCH 0/3] Fix for shellcheck issues with latest scripts in tests/shell

From: kajoljain <hidden>
Date: 2023-10-03 07:48:49

Patchset looks fine to me.

Reviewed-by: Kajol Jain <redacted>

thanks,
Kajol Jain

On 9/29/23 09:41, Athira Rajeev wrote:
shellcheck was run on perf tool shell scripts as a pre-requisite
to include a build option for shellcheck discussed here:
https://www.spinics.net/lists/linux-perf-users/msg25553.html

And fixes were added for the coding/formatting issues in
two patchsets:
https://lore.kernel.org/linux-perf-users/20230613164145.50488-1-atrajeev@linux.vnet.ibm.com/
https://lore.kernel.org/linux-perf-users/20230709182800.53002-1-atrajeev@linux.vnet.ibm.com/

Three additional issues were observed and fixes are part of:
https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/log/?h=perf-tools-next

With recent commits in perf, other three issues are observed.
shellcheck version: 0.6.0

With this patchset:

for F in $(find tests/shell/ -perm -o=x -name '*.sh'); do shellcheck -S warning $F; done
echo $?
0

The changes are with recent commits ( which is mentioned in each patch)
for ock_contention, record_sideband and test_arm_coresight testcases.
The changes are made on top of:
https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/log/?h=perf-tools-next

Athira Rajeev (3):
  perf tests test_arm_coresight: Fix the shellcheck warning in latest
    test_arm_coresight.sh
  tools/perf/tests Ignore the shellcheck SC2046 warning in
    lock_contentio
  tools/perf/tests: Fix shellcheck warning in record_sideband.sh test

 tools/perf/tests/shell/lock_contention.sh    | 1 +
 tools/perf/tests/shell/record_sideband.sh    | 2 +-
 tools/perf/tests/shell/test_arm_coresight.sh | 4 ++--
 3 files changed, 4 insertions(+), 3 deletions(-)
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help