Re: git: aa3860851b9f - main - net: Remove unneeded NULL check for the allocated ifnet

--------
Zhenlei Huang writes:

> commit aa3860851b9f6a6002d135b1cac7736e0995eedc
>
>     net: Remove unneeded NULL check for the allocated ifnet
>     
>     Change 4787572d0580 made if_alloc_domain() never fail, then also do the
>     wrappers if_alloc(), if_alloc_dev(), and if_gethandle().
>     
>[...]
> diff --git a/sys/arm/allwinner/if_emac.c b/sys/arm/allwinner/if_emac.c
> index f581d361d3d9..1db43cbca26c 100644
> --- a/sys/arm/allwinner/if_emac.c
> +++ b/sys/arm/allwinner/if_emac.c
> @@ -940,11 +940,6 @@ emac_attach(device_t dev)
>  	emac_reset(sc);
>  
>  	ifp = sc->emac_ifp = if_alloc(IFT_ETHER);
> -	if (ifp == NULL) {
> -		device_printf(dev, "unable to allocate ifp\n");
> -		error = ENOSPC;
> -		goto fail;
> -	}
>  	if_setsoftc(ifp, sc);
>  
>  	/* Setup MII */

I would like to suggest that we use a KASSERT() to document the
expectations in cases like this.

It will costs us nothing, and it helps tell both human readers of
the source code, the compiler, and code analysis tools what to
expect.

Background:

One of the things I did with Varnish Cache was make asserts mandatory
and numerous, to the point where about one source line in ten is an
assert in our code base.

In Varnish asserts are not compiled out in production code, and so
far nobody has ever documented any performance difference by doing so.

This is because almost all the asserts are on data already present
in a CPU register at the time, and in many cases the compiler is
even able to determine that the assert is always false and eliminates
it before code generation.

But having the assert there still clearly states the expectation
for both humans and programs.

For basic "is(n't) NULL" we have two short-hand assert macros:

	AZ(expression)	// is zero
	AN(expression)	// is not zero/NULL

Maybe we should have something similar in sys/kassert.h ?

-- 
Poul-Henning Kamp       | UNIX since Zilog Zeus 3.20
phk@FreeBSD.ORG         | TCP/IP since RFC 956
FreeBSD committer       | BSD since 4.3-tahoe    
Never attribute to malice what can adequately be explained by incompetence.