1. Patchew
  2. linux
  3. [PATCH 0/5] perf jitdump: Fix PID namespace detection
  4. View patch

[PATCH 2/5] perf test java symbol: Get rid of shellcheck warnings

Ilya Leoshkevich posted 5 patches 1 month, 1 week ago
[PATCH 2/5] perf test java symbol: Get rid of shellcheck warnings
Posted by Ilya Leoshkevich 1 month, 1 week ago 
Add missing quotes and suppress the $? warnings.

Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
 tools/perf/tests/shell/test_java_symbol.sh | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/tools/perf/tests/shell/test_java_symbol.sh b/tools/perf/tests/shell/test_java_symbol.sh
index 499539d1c4794..b1d7cd43af01a 100755
--- a/tools/perf/tests/shell/test_java_symbol.sh
+++ b/tools/perf/tests/shell/test_java_symbol.sh
@@ -4,6 +4,9 @@
 # SPDX-License-Identifier: GPL-2.0
 # Leo Yan <leo.yan@linaro.org>, 2022
 
+# Allow [ $? -ne 0 ], because long commands look ugly in if statements.
+# shellcheck disable=SC2181
+
 # skip if there's no jshell
 if ! [ -x "$(command -v jshell)" ]; then
 	echo "skip: no jshell, install JDK"
@@ -13,11 +16,12 @@ fi
 PERF_DATA=$(mktemp /tmp/__perf_test.perf.data.XXXXX)
 PERF_INJ_DATA=$(mktemp /tmp/__perf_test.perf.data.inj.XXXXX)
 
-cleanup_files()
-{
+# Shellcheck does not understand that this function is used by a trap.
+# shellcheck disable=SC2317
+cleanup_files() {
 	echo "Cleaning up files..."
-	rm -f ${PERF_DATA}
-	rm -f ${PERF_INJ_DATA}
+	rm -f "$PERF_DATA"
+	rm -f "$PERF_INJ_DATA"
 }
 
 trap cleanup_files exit term int
@@ -38,7 +42,7 @@ else
 	exit 2
 fi
 
-cat <<EOF | perf record -k 1 -o $PERF_DATA jshell -s -J-agentpath:$LIBJVMTI
+cat <<EOF | perf record -k 1 -o "$PERF_DATA" jshell -s -J"-agentpath:$LIBJVMTI"
 int fib(int x) {
 	return x > 1 ? fib(x - 2) + fib(x - 1) : 1;
 }
@@ -56,7 +60,7 @@ if [ $? -ne 0 ]; then
 	exit 1
 fi
 
-if ! DEBUGINFOD_URLS='' perf inject -i $PERF_DATA -o $PERF_INJ_DATA -j; then
+if ! DEBUGINFOD_URLS='' perf inject -i "$PERF_DATA" -o "$PERF_INJ_DATA" -j; then
 	echo "Fail to inject samples"
 	exit 1
 fi
@@ -64,8 +68,8 @@ fi
 # Below is an example of the instruction samples reporting:
 #   8.18%  jshell           jitted-50116-29.so    [.] Interpreter
 #   0.75%  Thread-1         jitted-83602-1670.so  [.] jdk.internal.jimage.BasicImageReader.getString(int)
-perf report --stdio -i ${PERF_INJ_DATA} 2>&1 | \
-	grep -E " +[0-9]+\.[0-9]+% .* (Interpreter|jdk\.internal).*" > /dev/null 2>&1
+perf report --stdio -i "$PERF_INJ_DATA" 2>&1 |
+	grep -E " +[0-9]+\.[0-9]+% .* (Interpreter|jdk\.internal).*" >/dev/null 2>&1
 
 if [ $? -ne 0 ]; then
 	echo "Fail to find java symbols"
-- 
2.51.1
Re: [PATCH 2/5] perf test java symbol: Get rid of shellcheck warnings
Posted by Namhyung Kim 1 month, 1 week ago 
Hello,

On Wed, Nov 05, 2025 at 08:10:25PM +0100, Ilya Leoshkevich wrote:
> Add missing quotes and suppress the $? warnings.

Can you please share the actual shellcheck warnings you see?

> 
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
>  tools/perf/tests/shell/test_java_symbol.sh | 20 ++++++++++++--------
>  1 file changed, 12 insertions(+), 8 deletions(-)
> 
> diff --git a/tools/perf/tests/shell/test_java_symbol.sh b/tools/perf/tests/shell/test_java_symbol.sh
> index 499539d1c4794..b1d7cd43af01a 100755
> --- a/tools/perf/tests/shell/test_java_symbol.sh
> +++ b/tools/perf/tests/shell/test_java_symbol.sh
> @@ -4,6 +4,9 @@
>  # SPDX-License-Identifier: GPL-2.0
>  # Leo Yan <leo.yan@linaro.org>, 2022
>  
> +# Allow [ $? -ne 0 ], because long commands look ugly in if statements.
> +# shellcheck disable=SC2181
> +
>  # skip if there's no jshell
>  if ! [ -x "$(command -v jshell)" ]; then
>  	echo "skip: no jshell, install JDK"
> @@ -13,11 +16,12 @@ fi
>  PERF_DATA=$(mktemp /tmp/__perf_test.perf.data.XXXXX)
>  PERF_INJ_DATA=$(mktemp /tmp/__perf_test.perf.data.inj.XXXXX)
>  
> -cleanup_files()
> -{
> +# Shellcheck does not understand that this function is used by a trap.
> +# shellcheck disable=SC2317
> +cleanup_files() {

Please minimize unnecessary changes.  It seems you don't need to change
the function declaration.

Thanks,
Namhyung


>  	echo "Cleaning up files..."
> -	rm -f ${PERF_DATA}
> -	rm -f ${PERF_INJ_DATA}
> +	rm -f "$PERF_DATA"
> +	rm -f "$PERF_INJ_DATA"
>  }
>  
>  trap cleanup_files exit term int
> @@ -38,7 +42,7 @@ else
>  	exit 2
>  fi
>  
> -cat <<EOF | perf record -k 1 -o $PERF_DATA jshell -s -J-agentpath:$LIBJVMTI
> +cat <<EOF | perf record -k 1 -o "$PERF_DATA" jshell -s -J"-agentpath:$LIBJVMTI"
>  int fib(int x) {
>  	return x > 1 ? fib(x - 2) + fib(x - 1) : 1;
>  }
> @@ -56,7 +60,7 @@ if [ $? -ne 0 ]; then
>  	exit 1
>  fi
>  
> -if ! DEBUGINFOD_URLS='' perf inject -i $PERF_DATA -o $PERF_INJ_DATA -j; then
> +if ! DEBUGINFOD_URLS='' perf inject -i "$PERF_DATA" -o "$PERF_INJ_DATA" -j; then
>  	echo "Fail to inject samples"
>  	exit 1
>  fi
> @@ -64,8 +68,8 @@ fi
>  # Below is an example of the instruction samples reporting:
>  #   8.18%  jshell           jitted-50116-29.so    [.] Interpreter
>  #   0.75%  Thread-1         jitted-83602-1670.so  [.] jdk.internal.jimage.BasicImageReader.getString(int)
> -perf report --stdio -i ${PERF_INJ_DATA} 2>&1 | \
> -	grep -E " +[0-9]+\.[0-9]+% .* (Interpreter|jdk\.internal).*" > /dev/null 2>&1
> +perf report --stdio -i "$PERF_INJ_DATA" 2>&1 |
> +	grep -E " +[0-9]+\.[0-9]+% .* (Interpreter|jdk\.internal).*" >/dev/null 2>&1
>  
>  if [ $? -ne 0 ]; then
>  	echo "Fail to find java symbols"
> -- 
> 2.51.1
>
Re: [PATCH 2/5] perf test java symbol: Get rid of shellcheck warnings
Posted by Ilya Leoshkevich 1 month, 1 week ago 
On Thu, 2025-11-06 at 18:07 -0800, Namhyung Kim wrote:
> Hello,
> 
> On Wed, Nov 05, 2025 at 08:10:25PM +0100, Ilya Leoshkevich wrote:
> > Add missing quotes and suppress the $? warnings.
> 
> Can you please share the actual shellcheck warnings you see?

There are a lot of them, I have cut similar ones from the output:

linux/tools/perf/tests/shell$ shellcheck -x test_java_symbol.sh

[...]

In test_java_symbol.sh line 19:
        rm -f ${PERF_DATA}
        ^----------------^ SC2317 (info): Command appears to be
unreachable. Check usage (or ignore if invoked indirectly).
              ^----------^ SC2086 (info): Double quote to prevent
globbing and word splitting.

[...]

In test_java_symbol.sh line 54:
if [ $? -ne 0 ]; then
     ^-- SC2181 (style): Check exit code directly with e.g. 'if !
mycmd;', not indirectly with $?.

[...]

> > Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> > ---
> >  tools/perf/tests/shell/test_java_symbol.sh | 20 ++++++++++++------
> > --
> >  1 file changed, 12 insertions(+), 8 deletions(-)
> > 
> > diff --git a/tools/perf/tests/shell/test_java_symbol.sh
> > b/tools/perf/tests/shell/test_java_symbol.sh
> > index 499539d1c4794..b1d7cd43af01a 100755
> > --- a/tools/perf/tests/shell/test_java_symbol.sh
> > +++ b/tools/perf/tests/shell/test_java_symbol.sh
> > @@ -4,6 +4,9 @@
> >  # SPDX-License-Identifier: GPL-2.0
> >  # Leo Yan <leo.yan@linaro.org>, 2022
> >  
> > +# Allow [ $? -ne 0 ], because long commands look ugly in if
> > statements.
> > +# shellcheck disable=SC2181
> > +
> >  # skip if there's no jshell
> >  if ! [ -x "$(command -v jshell)" ]; then
> >  	echo "skip: no jshell, install JDK"
> > @@ -13,11 +16,12 @@ fi
> >  PERF_DATA=$(mktemp /tmp/__perf_test.perf.data.XXXXX)
> >  PERF_INJ_DATA=$(mktemp /tmp/__perf_test.perf.data.inj.XXXXX)
> >  
> > -cleanup_files()
> > -{
> > +# Shellcheck does not understand that this function is used by a
> > trap.
> > +# shellcheck disable=SC2317
> > +cleanup_files() {
> 
> Please minimize unnecessary changes.  It seems you don't need to
> change
> the function declaration.

My bad, I ran shfmt as well and forgot about this. Interestingly
enough, there appears to be no consensus regarding style, but { on
the same line is a bit more popular:

linux/tools/perf/tests/shell$ grep -R '^[a-zA-Z0-9_]*()$' | wc -l
128

linux/tools/perf/tests/shell$ grep -R '^[a-zA-Z0-9_]*() {$' | wc -l
185

[...]
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.