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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <2d8e76d7dd3387c2a67e5cf1fa08a2874e910d78.camel@linux.ibm.com>
Date: Fri, 07 Nov 2025 08:57:37 +0100
From: Ilya Leoshkevich <iii@...ux.ibm.com>
To: Namhyung Kim <namhyung@...nel.org>
Cc: Arnaldo Carvalho de Melo <acme@...nel.org>, linux-kernel@...r.kernel.org,
        linux-perf-users@...r.kernel.org, Heiko
 Carstens <hca@...ux.ibm.com>,
        Vasily Gorbik <gor@...ux.ibm.com>,
        Alexander
 Gordeev <agordeev@...ux.ibm.com>
Subject: Re: [PATCH 2/5] perf test java symbol: Get rid of shellcheck
 warnings

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@...ux.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@...aro.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

[...]
>