On Fri, Jan 27, 2023 at 6:09 AM Andrei Gherzan
<andrei.gherzan@xxxxxxxxxxxxx> wrote:
>
> This change fixes the following shellcheck warnings and errors:
>
> * SC2155 (warning): Declare and assign separately to avoid masking return
> values.
> * SC2124 (warning): Assigning an array to a string! Assign as array, or use
> instead of @ to concatenate.
> * SC2034 (warning): ipv4_args appears unused. Verify use (or export if used
> externally).
> * SC2242 (error): Can only exit with status 0-255. Other data should be
> written to stdout/stderr.
> * SC2068 (error): Double quote array expansions to avoid re-splitting
> elements.
>
> Fixes: edae34a3ed92 ("selftests net: add UDP GRO fraglist + bpf self-tests")
> Signed-off-by: Andrei Gherzan <andrei.gherzan@xxxxxxxxxxxxx>
> ---
> tools/testing/selftests/net/udpgro_frglist.sh | 20 +++++++++----------
> 1 file changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/tools/testing/selftests/net/udpgro_frglist.sh b/tools/testing/selftests/net/udpgro_frglist.sh
> index e1ca49de2491..97bf20e9afd8 100755
> --- a/tools/testing/selftests/net/udpgro_frglist.sh
> +++ b/tools/testing/selftests/net/udpgro_frglist.sh
> @@ -3,7 +3,8 @@
> #
> # Run a series of udpgro benchmarks
>
> -readonly PEER_NS="ns-peer-$(mktemp -u XXXXXX)"
> +PEER_NS="ns-peer-$(mktemp -u XXXXXX)"
> +readonly PEER_NS
>
> BPF_FILE="../bpf/xdp_dummy.bpf.o"
> BPF_NAT6TO4_FILE="nat6to4.o"
> @@ -19,7 +20,7 @@ trap cleanup EXIT
>
> run_one() {
> # use 'rx' as separator between sender args and receiver args
> - local -r all="$@"
> + local -r all="$*"
this should technically use arrays, something like
local -a -r args=("$@")
but perhaps just get rid of args and just use "$@" directly below
> local -r tx_args=${all%rx*}
> local rx_args=${all#*rx}
>
> @@ -56,13 +57,13 @@ run_one() {
> }
>
> run_in_netns() {
> - local -r args=$@
> + local -r args="$*"
> echo ${args}
> ./in_netns.sh $0 __subprocess ${args}
ie. here could just use "$@" directly twice instead of defining args.
$0 should be doublequoted - though I guess it'll never be empty, and
is unlikely to include spaces.
> }
>
> run_udp() {
> - local -r args=$@
> + local -r args="$*"
>
> echo "udp gso - over veth touching data"
> run_in_netns ${args} -u -S 0 rx -4 -v
> @@ -72,7 +73,7 @@ run_udp() {
> }
>
> run_tcp() {
> - local -r args=$@
> + local -r args="$*"
>
> echo "tcp - over veth touching data"
> run_in_netns ${args} -t rx -4 -t
> @@ -80,7 +81,6 @@ run_tcp() {
>
> run_all() {
> local -r core_args="-l 4"
is this still useful? embed directly in ipv6_args
> - local -r ipv4_args="${core_args} -4 -D 192.168.1.1"
perhaps this should stay as a comment??
> local -r ipv6_args="${core_args} -6 -D 2001:db8::1"
>
> echo "ipv6"
> @@ -90,19 +90,19 @@ run_all() {
>
> if [ ! -f ${BPF_FILE} ]; then
double quote
"${BPF_FILE}"
in case space in file name
> echo "Missing ${BPF_FILE}. Build bpf selftest first"
> - exit -1
> + exit 1
> fi
>
> if [ ! -f "$BPF_NAT6TO4_FILE" ]; then
there seems to be inconsistency around [ vs [[, use [[ if relying on bash anyway
> echo "Missing nat6to4 helper. Build bpf nat6to4.o selftest first"
> - exit -1
> + exit 1
> fi
>
> if [[ $# -eq 0 ]]; then
> run_all
> elif [[ $1 == "__subprocess" ]]; then
while this does indeed work, imho $1 should be "$1" to be less confusing
> shift
> - run_one $@
> + run_one "$@"
> else
> - run_in_netns $@
> + run_in_netns "$@"
> fi
> --
> 2.34.1
