linux-kernel - Re: [PATCH] perf build: Specify that spellcheck should use the bash dialect.

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 for Android: free password hash cracker in your pocket

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

Message-ID: <CAP-5=fVoya-X+BBBuVwZQ527WKyGwC8tirk=gj37t2H_BBmwVg@mail.gmail.com>
Date: Mon, 23 Jun 2025 09:37:44 -0700
From: Ian Rogers <irogers@...gle.com>
To: James Clark <james.clark@...aro.org>
Cc: Collin Funk <collin.funk1@...il.com>, linux-perf-users@...r.kernel.org, 
	linux-kernel@...r.kernel.org, Peter Zijlstra <peterz@...radead.org>, 
	Ingo Molnar <mingo@...hat.com>, Arnaldo Carvalho de Melo <acme@...nel.org>, Namhyung Kim <namhyung@...nel.org>, 
	Alexander Shishkin <alexander.shishkin@...ux.intel.com>, Jiri Olsa <jolsa@...nel.org>, 
	Adrian Hunter <adrian.hunter@...el.com>, "Liang, Kan" <kan.liang@...ux.intel.com>
Subject: Re: [PATCH] perf build: Specify that spellcheck should use the bash dialect.

On Mon, Jun 23, 2025 at 1:10 AM James Clark <james.clark@...aro.org> wrote:
>
>
>
> On 20/06/2025 6:40 pm, Collin Funk wrote:
> > Hi James,
> >
> > James Clark <james.clark@...aro.org> writes:
> >
> >> If we're enforcing bash style with static analysis shouldn't we also
> >> change all the hashbangs to bash? Recently there have been changes to
> >> change sh to bash in some of the tests so presumably the hard rule for
> >> sh is no more?
> >>
> >> In the past I've had to replace bashisms that didn't work in sh but it
> >> would be nice to have only one language to write tests in. I doubt
> >> anyone running the tests today is running somewhere without bash, or
> >> that changing it will break anything. If anything it will fix more
> >> bashisms that have already been written.
> >>
> >> Just for reference there are 34 #!/bin/bash and 42 #!/bin/sh in
> >> tools/perf/tests
> >
> > That sounds reasonable to me. Writing portable shell is a hassle and if
> > we already assume a working /bin/bash in some places, I don't see a
> > reason not to use it for the others.
> >
> > Regarding this patch, shellcheck will use the file extension or shebang
> > only if it does not find a 'shell' directive in a .shellcheckrc. So that
> > change will still require this patch.
> >
> > I saw it was used in other places, so I assumed this patch was fine:
> >
> > $ find tools/perf -name Build | xargs grep bash
> > tools/perf/Build:     $(Q)$(call echo-cmd,test)shellcheck -s bash -a -S warning "$<" > $@ || (cat $@ && rm $@ && false)
> > tools/perf/trace/beauty/Build:        $(Q)$(call echo-cmd,test)shellcheck -s bash -a -S warning "$<" > $@ || (cat $@ && rm $@ && false)
> >
> > Collin
>
> In that case:
>
> Reviewed-by: James Clark <james.clark@...aro.org>
>
> And I'll send the bulk hashbang change separately.

I've no objection to switching to using bash globally. It seems
sub-optimal that we've copy-pasted the shellcheck command across many
different Build files and that this patch will cause the
tools/perf/tests/Build one to differ. My preference would be to have a
global definition probably in Makefile.perf, then use it consistently.
Alternative all shellcheck invocations can pass "-s bash" for the sake
of consistency.  Fwiw, I think the 'tools/arch/x86/tools/gen-insn-*'
which is to some extent taken from the kernel's 'arch/x86/tools' is
okay with the change too.

Thanks,
Ian