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

[Ilya Leoshkevich <iii@linux.ibm.com>]

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

[...]
>