[PATCH] perf tests: Fix test_arm_callgraph_fp variable expansion

[PATCH] perf tests: Fix test_arm_callgraph_fp variable expansion

[James Clark]

$TEST_PROGRAM is a command with spaces so it's supposed to be word
split. The referenced fix to fix the shellcheck warnings incorrectly
quoted this string so unquote it to fix the test.

At the same time silence the shellcheck warning for that line and fix
two more shellcheck errors at the end of the script.

Fixes: 1bb17b4c6c91 ("perf tests arm_callgraph_fp: Address shellcheck warnings about signal names and adding double quotes for expression")
Signed-off-by: James Clark <james.clark@arm.com>
---
 tools/perf/tests/shell/test_arm_callgraph_fp.sh | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/tools/perf/tests/shell/test_arm_callgraph_fp.sh b/tools/perf/tests/shell/test_arm_callgraph_fp.sh
index 1380e0d12dce..66dfdfdad553 100755
--- a/tools/perf/tests/shell/test_arm_callgraph_fp.sh
+++ b/tools/perf/tests/shell/test_arm_callgraph_fp.sh
@@ -15,7 +15,8 @@ cleanup_files()
 trap cleanup_files EXIT TERM INT
 
 # Add a 1 second delay to skip samples that are not in the leaf() function
-perf record -o "$PERF_DATA" --call-graph fp -e cycles//u -D 1000 --user-callchains -- "$TEST_PROGRAM" 2> /dev/null &
+# shellcheck disable=SC2086
+perf record -o "$PERF_DATA" --call-graph fp -e cycles//u -D 1000 --user-callchains -- $TEST_PROGRAM 2> /dev/null &
 PID=$!
 
 echo " + Recording (PID=$PID)..."
@@ -33,8 +34,8 @@ wait $PID
 # 	76c leafloop
 # ...
 
-perf script -i $PERF_DATA -F comm,ip,sym | head -n4
-perf script -i $PERF_DATA -F comm,ip,sym | head -n4 | \
+perf script -i "$PERF_DATA" -F comm,ip,sym | head -n4
+perf script -i "$PERF_DATA" -F comm,ip,sym | head -n4 | \
 	awk '{ if ($2 != "") sym[i++] = $2 } END { if (sym[0] != "leaf" ||
 						       sym[1] != "parent" ||
 						       sym[2] != "leafloop") exit 1 }'
-- 
2.34.1

Re: [PATCH] perf tests: Fix test_arm_callgraph_fp variable expansion

[Namhyung Kim]

Hi James,

On Thu, Jun 22, 2023 at 3:18 AM James Clark <james.clark@arm.com> wrote:
>
> $TEST_PROGRAM is a command with spaces so it's supposed to be word
> split. The referenced fix to fix the shellcheck warnings incorrectly
> quoted this string so unquote it to fix the test.
>
> At the same time silence the shellcheck warning for that line and fix
> two more shellcheck errors at the end of the script.
>
> Fixes: 1bb17b4c6c91 ("perf tests arm_callgraph_fp: Address shellcheck warnings about signal names and adding double quotes for expression")
> Signed-off-by: James Clark <james.clark@arm.com>

Acked-by: Namhyung Kim <namhyung@kernel.org>

Thanks,
Namhyung

> ---
>  tools/perf/tests/shell/test_arm_callgraph_fp.sh | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/tools/perf/tests/shell/test_arm_callgraph_fp.sh b/tools/perf/tests/shell/test_arm_callgraph_fp.sh
> index 1380e0d12dce..66dfdfdad553 100755
> --- a/tools/perf/tests/shell/test_arm_callgraph_fp.sh
> +++ b/tools/perf/tests/shell/test_arm_callgraph_fp.sh
> @@ -15,7 +15,8 @@ cleanup_files()
>  trap cleanup_files EXIT TERM INT
>
>  # Add a 1 second delay to skip samples that are not in the leaf() function
> -perf record -o "$PERF_DATA" --call-graph fp -e cycles//u -D 1000 --user-callchains -- "$TEST_PROGRAM" 2> /dev/null &
> +# shellcheck disable=SC2086
> +perf record -o "$PERF_DATA" --call-graph fp -e cycles//u -D 1000 --user-callchains -- $TEST_PROGRAM 2> /dev/null &
>  PID=$!
>
>  echo " + Recording (PID=$PID)..."
> @@ -33,8 +34,8 @@ wait $PID
>  #      76c leafloop
>  # ...
>
> -perf script -i $PERF_DATA -F comm,ip,sym | head -n4
> -perf script -i $PERF_DATA -F comm,ip,sym | head -n4 | \
> +perf script -i "$PERF_DATA" -F comm,ip,sym | head -n4
> +perf script -i "$PERF_DATA" -F comm,ip,sym | head -n4 | \
>         awk '{ if ($2 != "") sym[i++] = $2 } END { if (sym[0] != "leaf" ||
>                                                        sym[1] != "parent" ||
>                                                        sym[2] != "leafloop") exit 1 }'
> --
> 2.34.1
>

Re: [PATCH] perf tests: Fix test_arm_callgraph_fp variable expansion

[Namhyung Kim]

On Fri, Jun 23, 2023 at 9:56 AM Markus Elfring <Markus.Elfring@web.de> wrote:
>
> …
> > At the same time silence the shellcheck warning for that line and fix
> > two more shellcheck errors at the end of the script.
>
> Does such a wording really fit to the known requirement “Solve only one problem per patch.”?

Maybe not, but I think it still falls into the shellcheck category.
I don't mind having those trivial fixes together.

Applied to perf-tools-next, thanks!

>
> See also:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.4-rc7#n81

Re: [PATCH] perf tests: Fix test_arm_callgraph_fp variable expansion

[James Clark]

On 23/06/2023 17:56, Markus Elfring wrote:
> …
>> At the same time silence the shellcheck warning for that line and fix
>> two more shellcheck errors at the end of the script.
> 
> Does such a wording really fit to the known requirement “Solve only one problem per patch.”?
> 
> See also:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.4-rc7#n81
> 
> 
> Regards,
> Markus

I think so, it fixes all the shellcheck errors that were claimed to be
fixed and introduced by the referenced fix.

To be honest I would have rather just reverted the original change
completely because it was obviously never tested.