Hi Petr,
On 16/06/2025 17:06, Petr Machata wrote:
Matthieu Baerts matttbe@kernel.org writes:
Hi Jakub, Petr,
On 13/06/2025 18:57, Jakub Kicinski wrote:
On Thu, 12 Jun 2025 22:10:48 +0200 Petr Machata wrote:
Add tests for MC-routing underlay VXLAN traffic.
Signed-off-by: Petr Machata petrm@nvidia.com
Notes: v2: - Adjust as per shellcheck citations
Noob question - would we also be able to squash the unreachable code warnings if we declared ALL_TESTS as an array instead of a string? IDK if there's any trick we could use to make shellcheck stop complaining. Not blocking the series, obviously.
CC Matthieu, I presume you may have already investigated this :)
Thank you for the Cc. Yes indeed, I already had this case.
I don't think declaring ALL_TESTS as an array would help for this case -- even if it looks clearer than a long string -- because I think shellcheck will simply check if all the different functions are called directly. As mentioned in Shellcheck wiki [1]: "ShellCheck may incorrectly believe that code is unreachable if it's invoked by variable name or in a trap. In such a case, please Ignore the message".
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?
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.
Cheers, Matt