[PATCH 0/2] Script style improvements in lib/coresight.sh

[PATCH 0/2] Script style improvements in lib/coresight.sh

[Diederik de Haas]

These 2 patches improve the efficiency and quality of the
lib/coresight.sh script.

The first one uses grep's `-c` parameter to count the results instead of
piping it to `wc -l`.
The second one replaces the use of backticks (`...`) with $(...) as the
former has several potential issues while the latter does not.

Diederik de Haas (2):
  perf test: Replace 'grep | wc -l' with 'grep -c'
  perf test: Replace legacy `...` with $(...)

 tools/perf/tests/shell/lib/coresight.sh | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

-- 
2.39.1

[PATCH 1/2] perf test: Replace 'grep | wc -l' with 'grep -c'

[Diederik de Haas]

To count the number of results from grep, use the '-c' parameter
instead of piping it to 'wc'.
See also https://www.shellcheck.net/wiki/SC2126

Signed-off-by: Diederik de Haas <didi.debian@cknow.org>
---
 tools/perf/tests/shell/lib/coresight.sh | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/perf/tests/shell/lib/coresight.sh b/tools/perf/tests/shell/lib/coresight.sh
index 45a1477256b6..7e27e5c5bc9c 100644
--- a/tools/perf/tests/shell/lib/coresight.sh
+++ b/tools/perf/tests/shell/lib/coresight.sh
@@ -58,9 +58,9 @@ perf_dump_aux_verify() {
 	# compiler may produce different code depending on the compiler and
 	# optimization options, so this is rough just to see if we're
 	# either missing almost all the data or all of it
-	ATOM_FX_NUM=`grep I_ATOM_F "$DUMP" | wc -l`
-	ASYNC_NUM=`grep I_ASYNC "$DUMP" | wc -l`
-	TRACE_INFO_NUM=`grep I_TRACE_INFO "$DUMP" | wc -l`
+	ATOM_FX_NUM=`grep -c I_ATOM_F "$DUMP"`
+	ASYNC_NUM=`grep -c I_ASYNC "$DUMP"`
+	TRACE_INFO_NUM=`grep -c I_TRACE_INFO "$DUMP"`
 	rm -f "$DUMP"
 
 	# Arguments provide minimums for a pass
-- 
2.39.1

[PATCH 2/2] perf test: Replace legacy `...` with $(...)

[Diederik de Haas]

As detailed in https://www.shellcheck.net/wiki/SC2006:

The use of `...` is legacy syntax with several issues:
1. It has a series of undefined behaviors related to quoting in POSIX.
2. It imposes a custom escaping mode with surprising results.
3. It's exceptionally hard to nest.

$(...) command substitution has none of these problems,
and is therefore strongly encouraged.

Signed-off-by: Diederik de Haas <didi.debian@cknow.org>
---
 tools/perf/tests/shell/lib/coresight.sh | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/tools/perf/tests/shell/lib/coresight.sh b/tools/perf/tests/shell/lib/coresight.sh
index 7e27e5c5bc9c..6c3d34ec64d8 100644
--- a/tools/perf/tests/shell/lib/coresight.sh
+++ b/tools/perf/tests/shell/lib/coresight.sh
@@ -58,9 +58,9 @@ perf_dump_aux_verify() {
 	# compiler may produce different code depending on the compiler and
 	# optimization options, so this is rough just to see if we're
 	# either missing almost all the data or all of it
-	ATOM_FX_NUM=`grep -c I_ATOM_F "$DUMP"`
-	ASYNC_NUM=`grep -c I_ASYNC "$DUMP"`
-	TRACE_INFO_NUM=`grep -c I_TRACE_INFO "$DUMP"`
+	ATOM_FX_NUM=$(grep -c I_ATOM_F "$DUMP")
+	ASYNC_NUM=$(grep -c I_ASYNC "$DUMP")
+	TRACE_INFO_NUM=$(grep -c I_TRACE_INFO "$DUMP")
 	rm -f "$DUMP"
 
 	# Arguments provide minimums for a pass
@@ -96,18 +96,18 @@ perf_dump_aux_tid_verify() {
 
 	# The TID test tools will print a TID per stdout line that are being
 	# tested
-	TIDS=`cat "$2"`
+	TIDS=$(cat "$2")
 	# Scan the perf report to find the TIDs that are actually CID in hex
 	# and build a list of the ones found
-	FOUND_TIDS=`perf report --stdio --dump -i "$1" | \
+	FOUND_TIDS=$(perf report --stdio --dump -i "$1" | \
 			grep -o "CID=0x[0-9a-z]\+" | sed 's/CID=//g' | \
-			uniq | sort | uniq`
+			uniq | sort | uniq)
 	# No CID=xxx found - maybe your kernel is reporting these as
 	# VMID=xxx so look there
 	if test -z "$FOUND_TIDS"; then
-		FOUND_TIDS=`perf report --stdio --dump -i "$1" | \
+		FOUND_TIDS=$(perf report --stdio --dump -i "$1" | \
 				grep -o "VMID=0x[0-9a-z]\+" | sed 's/VMID=//g' | \
-				uniq | sort | uniq`
+				uniq | sort | uniq)
 	fi
 
 	# Iterate over the list of TIDs that the test says it has and find
@@ -116,7 +116,7 @@ perf_dump_aux_tid_verify() {
 	for TID2 in $TIDS; do
 		FOUND=""
 		for TIDHEX in $FOUND_TIDS; do
-			TID=`printf "%i" $TIDHEX`
+			TID=$(printf "%i" $TIDHEX)
 			if test "$TID" -eq "$TID2"; then
 				FOUND="y"
 				break
-- 
2.39.1

Re: [PATCH 0/2] Script style improvements in lib/coresight.sh

[Arnaldo Carvalho de Melo]

Em Wed, Feb 01, 2023 at 10:49:43PM +0100, Diederik de Haas escreveu:
> These 2 patches improve the efficiency and quality of the
> lib/coresight.sh script.
> 
> The first one uses grep's `-c` parameter to count the results instead of
> piping it to `wc -l`.
> The second one replaces the use of backticks (`...`) with $(...) as the
> former has several potential issues while the latter does not.
> 
> Diederik de Haas (2):
>   perf test: Replace 'grep | wc -l' with 'grep -c'
>   perf test: Replace legacy `...` with $(...)



Thanks, applied.

- Arnaldo

Re: [PATCH 0/2] Script style improvements in lib/coresight.sh

[Carsten Haitzler]

On 2/1/23 21:49, Diederik de Haas wrote:
> These 2 patches improve the efficiency and quality of the
> lib/coresight.sh script.
>
> The first one uses grep's `-c` parameter to count the results instead of
> piping it to `wc -l`.
> The second one replaces the use of backticks (`...`) with $(...) as the
> former has several potential issues while the latter does not.
>
> Diederik de Haas (2):
>    perf test: Replace 'grep | wc -l' with 'grep -c'
>    perf test: Replace legacy `...` with $(...)
>
>   tools/perf/tests/shell/lib/coresight.sh | 18 +++++++++---------
>   1 file changed, 9 insertions(+), 9 deletions(-)
>
ACK to all of this series.

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

Re: [PATCH 0/2] Script style improvements in lib/coresight.sh

[Arnaldo Carvalho de Melo]

Em Thu, Feb 02, 2023 at 08:38:46AM +0000, Carsten Haitzler escreveu:
> On 2/1/23 21:49, Diederik de Haas wrote:
> > These 2 patches improve the efficiency and quality of the
> > lib/coresight.sh script.
> >
> > The first one uses grep's `-c` parameter to count the results instead of
> > piping it to `wc -l`.
> > The second one replaces the use of backticks (`...`) with $(...) as the
> > former has several potential issues while the latter does not.
> >
> > Diederik de Haas (2):
> >    perf test: Replace 'grep | wc -l' with 'grep -c'
> >    perf test: Replace legacy `...` with $(...)
> >
> >   tools/perf/tests/shell/lib/coresight.sh | 18 +++++++++---------
> >   1 file changed, 9 insertions(+), 9 deletions(-)
> >
> ACK to all of this series.

Thanks, added to the two patches, that I processed yesterday, still in
tmp.perf/core.

- Arnaldo