Re: [PATCH net 5/5] selftests: net: add very basic test for netdev names and namespaces

[Jakub Kicinski <kuba@kernel.org>]

On Tue, 17 Oct 2023 13:25:10 +0200 Przemek Kitszel wrote:
> On 10/16/23 22:16, Jakub Kicinski wrote:
> > +fail() {
> > +    if [ ! -z "$1" ]; then  
> 
> s/! -z/-n/

I find ! -z easier to read.
! -z == "not zero", -n == "not??"

> > +	echo "ERROR: $1"
> > +    else
> > +	echo "ERROR: unexpected return code"  
> 
> I see that in some cases unexpected rc is 0, but it's worth printing.
> 
> At the expense of requiring reader to know about default values syntax,
> whole if could become:
>    echo "ERROR: ${1-unexpected return code} (rc=$_)"

SG!

> I didn't do my homework (of checking expectations of selftests
> "framework"), but perhaps errors/diag messages should go to stderr? >&2

Hm, I've never done that but won't hurt!

> > +    fi
> > +    RET_CODE=1
> > +}
> > +
> > +ip netns add $NS
> > +
> > +#
> > +# Test basic move without a rename
> > +#
> > +ip -netns $NS link add name $DEV type dummy || fail
> > +ip -netns $NS link set dev $DEV netns 1 || \  
> 
> \ after ||, |, && is redundant, not sure if it improves readability or not

Roger.

> I like this patch (and the rest of the series), especially for the fact
> how easy it is to test (compared to internals of HW drivers ;) )

Or notification fixes without something more flexible like YNL :(