linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
From: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
To: acme@kernel.org, jolsa@kernel.org, irogers@google.com,
	namhyung@kernel.org, ravi.bangoria@amd.com,
	john.g.garry@oracle.com
Cc: atrajeev@linux.vnet.ibm.com, Shirisha G <shirisha@linux.ibm.com>,
	kjain@linux.ibm.com, linux-perf-users@vger.kernel.org,
	maddy@linux.ibm.com, disgoel@linux.vnet.ibm.com,
	linuxppc-dev@lists.ozlabs.org
Subject: [PATCH 04/17] tools/perf/tests: fix shellcheck warnings for daemon.sh
Date: Tue, 13 Jun 2023 22:11:32 +0530	[thread overview]
Message-ID: <20230613164145.50488-5-atrajeev@linux.vnet.ibm.com> (raw)
In-Reply-To: <20230613164145.50488-1-atrajeev@linux.vnet.ibm.com>

From: Shirisha G <shirisha@linux.ibm.com>

Running shellcheck -S on daemon.sh throws below warnings:

Result from shellcheck:
     # shellcheck -S warning daemon.sh
     local line_name=`echo "${line}" | awk 'BEGIN { FS = ":" } ; { print $2 }'`
           ^-------^ SC2155: Declare and assign separately to avoid masking return values.

     trap "echo 'FAILED: Signal caught'; daemon_exit ${config}; exit 1" SIGINT SIGTERM
                                                     ^-------^ SC2064: Use single quotes, otherwise this expands now rather than when signalled.

     count=`ls ${base}/session-test/ | grep perf.data | wc -l`
            ^-- SC2010: Don't use ls | grep. Use a glob or a for loop with a condition to allow non-alphanumeric filenames.

     if [ ${size} != "OK" -o ${type} != "OK" ]; then
                          ^-- SC2166: Prefer [ p ] || [ q ] as [ p -o q ] is not well defined.

Fixed above warnings by:
   - declaring and assigning local variables separately
   - To fix SC2010, instead of using "ls | grep", used glob to allow non-alphanumeric filenames
   - Used single quotes to prevent expanding.

Result from shellcheck after patch changes:
     $ shellcheck -S warning daemon.sh
     $ echo $?
       0

Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
Signed-off-by: Shirisha G <shirisha@linux.ibm.com>
---
 tools/perf/tests/shell/daemon.sh | 113 ++++++++++++++++++++-----------
 1 file changed, 75 insertions(+), 38 deletions(-)

diff --git a/tools/perf/tests/shell/daemon.sh b/tools/perf/tests/shell/daemon.sh
index 45fc24af5b07..aaf3849353e8 100755
--- a/tools/perf/tests/shell/daemon.sh
+++ b/tools/perf/tests/shell/daemon.sh
@@ -11,11 +11,16 @@ check_line_first()
 	local lock=$5
 	local up=$6
 
-	local line_name=`echo "${line}" | awk 'BEGIN { FS = ":" } ; { print $2 }'`
-	local line_base=`echo "${line}" | awk 'BEGIN { FS = ":" } ; { print $3 }'`
-	local line_output=`echo "${line}" | awk 'BEGIN { FS = ":" } ; { print $4 }'`
-	local line_lock=`echo "${line}" | awk 'BEGIN { FS = ":" } ; { print $5 }'`
-	local line_up=`echo "${line}" | awk 'BEGIN { FS = ":" } ; { print $6 }'`
+	local line_name
+	line_name=`echo "${line}" | awk 'BEGIN { FS = ":" } ; { print $2 }'`
+	local line_base
+	line_base=`echo "${line}" | awk 'BEGIN { FS = ":" } ; { print $3 }'`
+	local line_output
+	line_output=`echo "${line}" | awk 'BEGIN { FS = ":" } ; { print $4 }'`
+	local line_lock
+	line_lock=`echo "${line}" | awk 'BEGIN { FS = ":" } ; { print $5 }'`
+	local line_up
+	line_up=`echo "${line}" | awk 'BEGIN { FS = ":" } ; { print $6 }'`
 
 	if [ "${name}" != "${line_name}" ]; then
 		echo "FAILED: wrong name"
@@ -54,13 +59,20 @@ check_line_other()
 	local ack=$7
 	local up=$8
 
-	local line_name=`echo "${line}" | awk 'BEGIN { FS = ":" } ; { print $2 }'`
-	local line_run=`echo "${line}" | awk 'BEGIN { FS = ":" } ; { print $3 }'`
-	local line_base=`echo "${line}" | awk 'BEGIN { FS = ":" } ; { print $4 }'`
-	local line_output=`echo "${line}" | awk 'BEGIN { FS = ":" } ; { print $5 }'`
-	local line_control=`echo "${line}" | awk 'BEGIN { FS = ":" } ; { print $6 }'`
-	local line_ack=`echo "${line}" | awk 'BEGIN { FS = ":" } ; { print $7 }'`
-	local line_up=`echo "${line}" | awk 'BEGIN { FS = ":" } ; { print $8 }'`
+	local line_name
+	line_name=`echo "${line}" | awk 'BEGIN { FS = ":" } ; { print $2 }'`
+	local line_run
+	line_run=`echo "${line}" | awk 'BEGIN { FS = ":" } ; { print $3 }'`
+	local line_base
+	line_base=`echo "${line}" | awk 'BEGIN { FS = ":" } ; { print $4 }'`
+	local line_output
+	line_output=`echo "${line}" | awk 'BEGIN { FS = ":" } ; { print $5 }'`
+	local line_control
+	line_control=`echo "${line}" | awk 'BEGIN { FS = ":" } ; { print $6 }'`
+	local line_ack
+	line_ack=`echo "${line}" | awk 'BEGIN { FS = ":" } ; { print $7 }'`
+	local line_up
+	line_up=`echo "${line}" | awk 'BEGIN { FS = ":" } ; { print $8 }'`
 
 	if [ "${name}" != "${line_name}" ]; then
 		echo "FAILED: wrong name"
@@ -102,8 +114,10 @@ daemon_exit()
 {
 	local config=$1
 
-	local line=`perf daemon --config ${config} -x: | head -1`
-	local pid=`echo "${line}" | awk 'BEGIN { FS = ":" } ; { print $1 }'`
+	local line
+	line=`perf daemon --config ${config} -x: | head -1`
+	local pid
+	pid=`echo "${line}" | awk 'BEGIN { FS = ":" } ; { print $1 }'`
 
 	# Reset trap handler.
 	trap - SIGINT SIGTERM
@@ -123,7 +137,7 @@ daemon_start()
 	perf daemon start --config ${config}
 
 	# Clean up daemon if interrupted.
-	trap "echo 'FAILED: Signal caught'; daemon_exit ${config}; exit 1" SIGINT SIGTERM
+	trap 'echo "FAILED: Signal caught"; daemon_exit "${config}"; exit 1' SIGINT SIGTERM
 
 	# wait for the session to ping
 	local state="FAIL"
@@ -144,8 +158,10 @@ test_list()
 {
 	echo "test daemon list"
 
-	local config=$(mktemp /tmp/perf.daemon.config.XXX)
-	local base=$(mktemp -d /tmp/perf.daemon.base.XXX)
+	local config
+	config=$(mktemp /tmp/perf.daemon.config.XXX)
+	local base
+	base=$(mktemp -d /tmp/perf.daemon.base.XXX)
 
 	cat <<EOF > ${config}
 [daemon]
@@ -165,19 +181,22 @@ EOF
 
 	# check first line
 	# pid:daemon:base:base/output:base/lock
-	local line=`perf daemon --config ${config} -x: | head -1`
+	local line
+	line=`perf daemon --config ${config} -x: | head -1`
 	check_line_first ${line} daemon ${base} ${base}/output ${base}/lock "0"
 
 	# check 1st session
 	# pid:size:-e cpu-clock:base/size:base/size/output:base/size/control:base/size/ack:0
-	local line=`perf daemon --config ${config} -x: | head -2 | tail -1`
+	local line
+	line=`perf daemon --config ${config} -x: | head -2 | tail -1`
 	check_line_other "${line}" size "-e cpu-clock -m 1 sleep 10" ${base}/session-size \
 			 ${base}/session-size/output ${base}/session-size/control \
 			 ${base}/session-size/ack "0"
 
 	# check 2nd session
 	# pid:time:-e task-clock:base/time:base/time/output:base/time/control:base/time/ack:0
-	local line=`perf daemon --config ${config} -x: | head -3 | tail -1`
+	local line
+	line=`perf daemon --config ${config} -x: | head -3 | tail -1`
 	check_line_other "${line}" time "-e task-clock -m 1 sleep 10" ${base}/session-time \
 			 ${base}/session-time/output ${base}/session-time/control \
 			 ${base}/session-time/ack "0"
@@ -193,8 +212,10 @@ test_reconfig()
 {
 	echo "test daemon reconfig"
 
-	local config=$(mktemp /tmp/perf.daemon.config.XXX)
-	local base=$(mktemp -d /tmp/perf.daemon.base.XXX)
+	local config
+	config=$(mktemp /tmp/perf.daemon.config.XXX)
+	local base
+	base=$(mktemp -d /tmp/perf.daemon.base.XXX)
 
 	# prepare config
 	cat <<EOF > ${config}
@@ -215,10 +236,12 @@ EOF
 
 	# check 2nd session
 	# pid:time:-e task-clock:base/time:base/time/output:base/time/control:base/time/ack:0
-	local line=`perf daemon --config ${config} -x: | head -3 | tail -1`
+	local line
+	line=`perf daemon --config ${config} -x: | head -3 | tail -1`
 	check_line_other "${line}" time "-e task-clock -m 1 sleep 10" ${base}/session-time \
 			 ${base}/session-time/output ${base}/session-time/control ${base}/session-time/ack "0"
-	local pid=`echo "${line}" | awk 'BEGIN { FS = ":" } ; { print $1 }'`
+	local pid
+	pid=`echo "${line}" | awk 'BEGIN { FS = ":" } ; { print $1 }'`
 
 	# prepare new config
 	local config_new=${config}.new
@@ -249,7 +272,8 @@ EOF
 
 	# check reconfigured 2nd session
 	# pid:time:-e task-clock:base/time:base/time/output:base/time/control:base/time/ack:0
-	local line=`perf daemon --config ${config} -x: | head -3 | tail -1`
+	local line
+	line=`perf daemon --config ${config} -x: | head -3 | tail -1`
 	check_line_other "${line}" time "-e cpu-clock -m 1 sleep 10" ${base}/session-time \
 			 ${base}/session-time/output ${base}/session-time/control ${base}/session-time/ack "0"
 
@@ -276,7 +300,8 @@ EOF
 		state=`perf daemon ping --config ${config} --session size | awk '{ print $1 }'`
 	done
 
-	local one=`perf daemon --config ${config} -x: | wc -l`
+	local one
+	one=`perf daemon --config ${config} -x: | wc -l`
 
 	if [ ${one} -ne "1" ]; then
 		echo "FAILED: wrong list output"
@@ -312,8 +337,10 @@ test_stop()
 {
 	echo "test daemon stop"
 
-	local config=$(mktemp /tmp/perf.daemon.config.XXX)
-	local base=$(mktemp -d /tmp/perf.daemon.base.XXX)
+	local config
+	config=$(mktemp /tmp/perf.daemon.config.XXX)
+	local base
+	base=$(mktemp -d /tmp/perf.daemon.base.XXX)
 
 	# prepare config
 	cat <<EOF > ${config}
@@ -332,8 +359,12 @@ EOF
 	# start daemon
 	daemon_start ${config} size
 
-	local pid_size=`perf daemon --config ${config} -x: | head -2 | tail -1 | awk 'BEGIN { FS = ":" } ; { print $1 }'`
-	local pid_time=`perf daemon --config ${config} -x: | head -3 | tail -1 | awk 'BEGIN { FS = ":" } ; { print $1 }'`
+	local pid_size
+	pid_size=`perf daemon --config ${config} -x: | head -2 | tail -1 |
+		  awk 'BEGIN { FS = ":" } ; { print $1 }'`
+	local pid_time
+	pid_time=`perf daemon --config ${config} -x: | head -3 | tail -1 |
+		  awk 'BEGIN { FS = ":" } ; { print $1 }'`
 
 	# check that sessions are running
 	if [ ! -d "/proc/${pid_size}" ]; then
@@ -364,8 +395,10 @@ test_signal()
 {
 	echo "test daemon signal"
 
-	local config=$(mktemp /tmp/perf.daemon.config.XXX)
-	local base=$(mktemp -d /tmp/perf.daemon.base.XXX)
+	local config
+	config=$(mktemp /tmp/perf.daemon.config.XXX)
+	local base
+	base=$(mktemp -d /tmp/perf.daemon.base.XXX)
 
 	# prepare config
 	cat <<EOF > ${config}
@@ -389,7 +422,7 @@ EOF
 	daemon_exit ${config}
 
 	# count is 2 perf.data for signals and 1 for perf record finished
-	count=`ls ${base}/session-test/ | grep perf.data | wc -l`
+	count=`ls ${base}/session-test/*perf.data* | wc -l`
 	if [ ${count} -ne 3 ]; then
 		error=1
 		echo "FAILED: perf data no generated"
@@ -403,8 +436,10 @@ test_ping()
 {
 	echo "test daemon ping"
 
-	local config=$(mktemp /tmp/perf.daemon.config.XXX)
-	local base=$(mktemp -d /tmp/perf.daemon.base.XXX)
+	local config
+	config=$(mktemp /tmp/perf.daemon.config.XXX)
+	local base
+	base=$(mktemp -d /tmp/perf.daemon.base.XXX)
 
 	# prepare config
 	cat <<EOF > ${config}
@@ -426,7 +461,7 @@ EOF
 	size=`perf daemon ping --config ${config} --session size | awk '{ print $1 }'`
 	type=`perf daemon ping --config ${config} --session time | awk '{ print $1 }'`
 
-	if [ ${size} != "OK" -o ${type} != "OK" ]; then
+	if [ ${size} != "OK" ] || [ ${type} != "OK" ]; then
 		error=1
 		echo "FAILED: daemon ping failed"
 	fi
@@ -442,8 +477,10 @@ test_lock()
 {
 	echo "test daemon lock"
 
-	local config=$(mktemp /tmp/perf.daemon.config.XXX)
-	local base=$(mktemp -d /tmp/perf.daemon.base.XXX)
+	local config
+	config=$(mktemp /tmp/perf.daemon.config.XXX)
+	local base
+	base=$(mktemp -d /tmp/perf.daemon.base.XXX)
 
 	# prepare config
 	cat <<EOF > ${config}
-- 
2.39.1


  parent reply	other threads:[~2023-06-13 17:24 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-13 16:41 [PATCH 00/17] tool/perf/test: Fix shellcheck coding/formatting issues of test shell scripts Athira Rajeev
2023-06-13 16:41 ` [PATCH 01/17] perf: get rid of unused import Athira Rajeev
2023-06-13 19:54   ` Arnaldo Carvalho de Melo
2023-06-14  1:59     ` Leo Yan
2023-06-14  2:34       ` Arnaldo Carvalho de Melo
2023-06-14  2:48         ` Leo Yan
2023-06-13 16:41 ` [PATCH 02/17] tools/perf/tests: fix shellcheck warning for stat+json_output Athira Rajeev
2023-06-13 16:41 ` [PATCH 03/17] shellcheck : fixing signal names and adding double quotes for expression in test_arm_callgraph_fp Athira Rajeev
2023-06-13 16:41 ` Athira Rajeev [this message]
2023-06-13 16:41 ` [PATCH 05/17] tools/perf/tests: Fix shellcheck warnings for stat+csv_output Athira Rajeev
2023-06-13 16:41 ` [PATCH 06/17] tools/perf/tests: Fix shellcheck warnings for trace+probe_vfs_getname.sh Athira Rajeev
2023-06-14  2:21   ` Arnaldo Carvalho de Melo
2023-06-13 16:41 ` [PATCH 07/17] perf/tests/shell : Shellcheck fixes for perf test "test_arm_coresight.sh" Athira Rajeev
2023-06-13 16:41 ` [PATCH 08/17] tools/perf/tests/shell/stat_all_metrics: Fix shellcheck warning SC2076 in stat_all_metrics.sh Athira Rajeev
2023-06-13 16:41 ` [PATCH 09/17] tools/perf/tests: Fix shellcheck issues in test_task_analyzer.sh file Athira Rajeev
2023-06-13 16:41 ` [PATCH 10/17] tools/perf/tests: fix test_arm_spe.sh signal case issues Athira Rajeev
2023-06-13 16:41 ` [PATCH 11/17] perf/tests/shell: fix shellscript errors for lock_contention.sh Athira Rajeev
2023-06-13 16:41 ` [PATCH 12/17] tools/perf/tests: fixed shellcheck warnings for perf shell scripts Athira Rajeev
2023-06-13 16:41 ` [PATCH 13/17] tools/perf/tests: Fix all POSIX sh warnings in perf shell test test_brstack.sh Athira Rajeev
2023-06-13 16:41 ` [PATCH 14/17] tools/perf/tests: Fix all POSIX sh warnings in stat+shadow_stat.sh Athira Rajeev
2023-06-13 16:41 ` [PATCH 15/17] perf tests task_analyzer: fix bad substitution ${$1} Athira Rajeev
2023-06-13 20:06   ` Arnaldo Carvalho de Melo
2023-06-21  0:48     ` Namhyung Kim
2023-06-21 10:03       ` Aditya Gupta
2023-06-21 15:23         ` Namhyung Kim
2023-06-21 18:43           ` Aditya Gupta
2023-06-22 21:33             ` Namhyung Kim
2023-06-22 23:25               ` Namhyung Kim
2023-06-23 18:04                 ` Aditya Gupta
2023-06-23 18:08                 ` Aditya Gupta
2023-06-23 18:19                 ` Aditya Gupta
2023-06-23 23:11                   ` Namhyung Kim
2023-06-13 16:41 ` [PATCH 16/17] perf tests task_analyzer: print command on failure Athira Rajeev
2023-06-13 20:07   ` Arnaldo Carvalho de Melo
2023-06-13 20:43     ` Hagen Paul Pfeifer
2023-06-13 16:41 ` [PATCH 17/17] perf tests task_analyzer: skip tests if no libtraceevent support Athira Rajeev
2023-07-13  1:16   ` Ian Rogers
2023-07-13  8:35     ` Aditya Gupta
2023-06-13 20:34 ` [PATCH 00/17] tool/perf/test: Fix shellcheck coding/formatting issues of test shell scripts Arnaldo Carvalho de Melo
2023-06-14  9:31   ` Athira Rajeev

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230613164145.50488-5-atrajeev@linux.vnet.ibm.com \
    --to=atrajeev@linux.vnet.ibm.com \
    --cc=acme@kernel.org \
    --cc=disgoel@linux.vnet.ibm.com \
    --cc=irogers@google.com \
    --cc=john.g.garry@oracle.com \
    --cc=jolsa@kernel.org \
    --cc=kjain@linux.ibm.com \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=maddy@linux.ibm.com \
    --cc=namhyung@kernel.org \
    --cc=ravi.bangoria@amd.com \
    --cc=shirisha@linux.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).