From nobody Fri Jun 28 10:38:45 2024 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4W9X3f1H71z5JjPX; Fri, 28 Jun 2024 10:38:54 +0000 (UTC) (envelope-from phk@critter.freebsd.dk) Received: from phk.freebsd.dk (phk.freebsd.dk [130.225.244.222]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 4W9X3d5v8Lz4rgD; Fri, 28 Jun 2024 10:38:53 +0000 (UTC) (envelope-from phk@critter.freebsd.dk) Authentication-Results: mx1.freebsd.org; none Received: from critter.freebsd.dk (unknown [192.168.55.3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by phk.freebsd.dk (Postfix) with ESMTPS id DA9BF892BF; Fri, 28 Jun 2024 10:38:45 +0000 (UTC) Received: from critter.freebsd.dk (localhost [127.0.0.1]) by critter.freebsd.dk (8.18.1/8.16.1) with ESMTPS id 45SAcjSD010521 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NO); Fri, 28 Jun 2024 10:38:45 GMT (envelope-from phk@critter.freebsd.dk) Received: (from phk@localhost) by critter.freebsd.dk (8.18.1/8.16.1/Submit) id 45SAcjNd010520; Fri, 28 Jun 2024 10:38:45 GMT (envelope-from phk) Message-Id: <202406281038.45SAcjNd010520@critter.freebsd.dk> To: Zhenlei Huang cc: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: Re: git: aa3860851b9f - main - net: Remove unneeded NULL check for the allocated ifnet In-reply-to: <202406281018.45SAILbb075980@gitrepo.freebsd.org> From: "Poul-Henning Kamp" References: <202406281018.45SAILbb075980@gitrepo.freebsd.org> List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-all@freebsd.org Sender: owner-dev-commits-src-all@FreeBSD.org MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-ID: <10518.1719571125.1@critter.freebsd.dk> Date: Fri, 28 Jun 2024 10:38:45 +0000 X-Spamd-Bar: ---- X-Rspamd-Pre-Result: action=no action; module=replies; Message is reply to one we originated X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[]; ASN(0.00)[asn:1835, ipnet:130.225.0.0/16, country:EU] X-Rspamd-Queue-Id: 4W9X3d5v8Lz4rgD -------- 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.