netdev - Re: [PATCH net-next, v3] selftest: Add selftest for multicast address notifications

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] [day] [month] [year] [list]

Message-ID: <CADXeF1HJ7dyw5gp7sKZvRgf_WLuEJatqfKmfxzpWtLibB=e9rg@mail.gmail.com>
Date: Sat, 14 Jun 2025 12:23:03 +0900
From: Yuyang Huang <yuyanghuang@...gle.com>
To: Jakub Kicinski <kuba@...nel.org>
Cc: "David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, 
	Paolo Abeni <pabeni@...hat.com>, Simon Horman <horms@...nel.org>, Shuah Khan <shuah@...nel.org>, 
	netdev@...r.kernel.org, linux-kselftest@...r.kernel.org, 
	linux-kernel@...r.kernel.org, Maciej Żenczykowski <maze@...gle.com>, 
	Lorenzo Colitti <lorenzo@...gle.com>
Subject: Re: [PATCH net-next, v3] selftest: Add selftest for multicast address notifications

Thanks for the suggestion.

>Perhaps move these to lib.sh as a separate commit?
>They seem generic.

I am looking at the existing test cases, and it seems that each case
is doing its own way of handling the end_test()/run_cmd(). It's
non-trivial to unify everything into lib.sh, and it seems to be a huge
refactor if we want to do it this way. I can also imagine each test
case might want to customize the behavior a little bit differently.

On the other hand, it seems some of the helper functions I copied over
can be simplified. I will refactor the code a little bit to reduce the
duplication.

Thanks,
Yuyang


On Sat, Jun 14, 2025 at 9:29 AM Jakub Kicinski <kuba@...nel.org> wrote:
>
> On Thu, 12 Jun 2025 11:05:14 +0900 Yuyang Huang wrote:
> > +VERBOSE=0
> > +PAUSE=no
> > +PAUSE_ON_FAIL=no
> > +
> > +source lib.sh
> > +
> > +# set global exit status, but never reset nonzero one.
> > +check_err()
> > +{
> > +     if [ $ret -eq 0 ]; then
> > +             ret=$1
> > +     fi
> > +     [ -n "$2" ] && echo "$2"
> > +}
> > +
> > +run_cmd_common()
> > +{
> > +     local cmd="$*"
> > +     local out
> > +     if [ "$VERBOSE" = "1" ]; then
> > +             echo "COMMAND: ${cmd}"
> > +     fi
> > +     out=$($cmd 2>&1)
> > +     rc=$?
> > +     if [ "$VERBOSE" = "1" -a -n "$out" ]; then
> > +             echo "    $out"
> > +     fi
> > +     return $rc
> > +}
> > +
> > +run_cmd() {
> > +     run_cmd_common "$@"
> > +     rc=$?
> > +     check_err $rc
> > +     return $rc
> > +}
> > +
> > +end_test()
> > +{
> > +     echo "$*"
> > +     [ "${VERBOSE}" = "1" ] && echo
> > +
> > +     if [[ $ret -ne 0 ]] && [[ "${PAUSE_ON_FAIL}" = "yes" ]]; then
> > +             echo "Hit enter to continue"
> > +             read a
> > +     fi;
> > +
> > +     if [ "${PAUSE}" = "yes" ]; then
> > +             echo "Hit enter to continue"
> > +             read a
> > +     fi
> > +
> > +}
>
> Perhaps move these to lib.sh as a separate commit?
> They seem generic.
>
> Please fix the shellcheck warnings. The "info"-level messages
> are up you, SC2317 can be ignored.
> --
> pw-bot: cr