On 16/06/2025 21:53, Petr Machata wrote:
Matthieu Baerts matttbe@kernel.org writes:
Hi Petr,
On 16/06/2025 17:06, Petr Machata wrote:
Matthieu Baerts matttbe@kernel.org writes:
That what we did with MPTCP, see the top of the mptcp_join.sh file for example [2], where we have:
# ShellCheck incorrectly believes that most of the code here is unreachable # because it's invoked by variable name, see how the "tests" array is used #shellcheck disable=SC2317
If you add this at the top of your new file, followed by an empty line, shellcheck will ignore this issue for the whole file.
The ALL_TESTS issue is not the end of it either. We use helpers that call stuff indirectly all over the place. defer, in_ns... It would make sense to me to just disable SC2317 in NIPA runs. Or maybe even put it in net/forwarding/.shellcheckrc. Pretty much all those tests are written in a style that will hit these issues.
In this case, I think it would be better to add this .shellcheckrc file in net/forwarding. If you modify NIPA, I don't think people will know what is allowed or not, or what command line to use, no?
Good point. The question then is whether to put it to forwarding/ or directly net/, which is has seen more use of lib.sh and therefore the same sort of coding style. I'll experiment with it and should be able to send it later in the week. I don't want to add it to the MC patchset.
Since Jakub disabled SC2317 in NIPA [1], then I guess we can put this .shellcheckrc file in net/, no?
Note that NIPA's shellcheck reports are currently ignoring all "info" and "style" severities -- so including SC2317 -- except for new files or the ones that were previously shellcheck compliant.
Yeah, I know, but the result is still very noisy, if you want to verify it prior to sending the patchset. It's a bit annoying to have to scroll through the report trying to find relevant stuff. I could add .shellcheckrc in my own clone, but everybody is going to hit these.
When Shellcheck got introduced, there were some discussions about SC2317 and SC2086 [2]. I don't think they are useless -- maybe a function is not used by mistake, maybe double quotes are really needed for some cases, etc. -- but I agree with you: they create a lot of noise.
[1] https://github.com/linux-netdev/nipa/commit/23b74dd [2] https://github.com/linux-netdev/nipa/pull/51#issuecomment-2939556291
Cheers, Matt