Re: [PATCH net-next] selftests: forwarding: lib: rewrite processing of command line arguments

[Simon Horman]

On Fri, Apr 03, 2026 at 05:19:12PM +0300, Ioana Ciornei wrote:
> The piece of code which processes the command line arguments and
> populates NETIFS based on them is really unobvious. Rewrite it so that
> the intention is clear and the code is easy to follow.
> 
> Suggested-by: Petr Machata <petrm@xxxxxxxxxx>
> Signed-off-by: Ioana Ciornei <ioana.ciornei@xxxxxxx>
> ---
>  tools/testing/selftests/net/forwarding/lib.sh | 22 +++++++++----------
>  1 file changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh
> index d8cc4c64148d..922cdaf2ceb9 100644
> --- a/tools/testing/selftests/net/forwarding/lib.sh
> +++ b/tools/testing/selftests/net/forwarding/lib.sh
> @@ -466,18 +466,18 @@ if [ "${DRIVER_TEST_CONFORMANT}" = "yes" ]; then
>  	NETIFS[p2]="$remote_netif"
>  	TARGETS[$remote_netif]="$REMOTE_TYPE:$REMOTE_ARGS"
>  else
> -	count=0

AI generated review on Sashiko.dev suggests that initialising count should
be retained to ensure correct behaviour in the (unlikely) case that count
is already set (to a non-zero value) in the environment.

And this seems like a good idea to me.

> +	# Prime NETIFS from the command line, but retain if none given.
> +	if [[ $# -gt 0 ]]; then
> +		unset NETIFS
> +		declare -A NETIFS
>  
> -	while [[ $# -gt 0 ]]; do
> -		if [[ "$count" -eq "0" ]]; then
> -			unset NETIFS
> -			declare -A NETIFS
> -		fi
> -		count=$((count + 1))
> -		NETIFS[p$count]="$1"
> -		TARGETS[$1]="local:"
> -		shift
> -	done
> +		while [[ $# -gt 0 ]]; do
> +			count=$((count + 1))
> +			NETIFS[p$count]="$1"
> +			TARGETS[$1]="local:"
> +			shift
> +		done
> +	fi
>  fi
>  
>  ##############################################################################
> -- 
> 2.25.1
>