From owner-svn-src-head@freebsd.org Thu Jun 27 15:48:03 2019 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 3155415C65E0; Thu, 27 Jun 2019 15:48:03 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail106.syd.optusnet.com.au (mail106.syd.optusnet.com.au [211.29.132.42]) by mx1.freebsd.org (Postfix) with ESMTP id 946058BCD7; Thu, 27 Jun 2019 15:48:02 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from [192.168.0.102] (c110-21-101-228.carlnfd1.nsw.optusnet.com.au [110.21.101.228]) by mail106.syd.optusnet.com.au (Postfix) with ESMTPS id 353823DCAC5; Fri, 28 Jun 2019 01:47:53 +1000 (AEST) Date: Fri, 28 Jun 2019 01:47:51 +1000 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Andriy Gapon cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r349459 - head/sys/sys In-Reply-To: <201906271507.x5RF775Q070616@repo.freebsd.org> Message-ID: <20190628012059.D2091@besplex.bde.org> References: <201906271507.x5RF775Q070616@repo.freebsd.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.2 cv=P6RKvmIu c=1 sm=1 tr=0 cx=a_idp_d a=PalzARQSbocsUSjMRkwAPg==:117 a=PalzARQSbocsUSjMRkwAPg==:17 a=jpOVt7BSZ2e4Z31A5e1TngXxSK0=:19 a=kj9zAlcOel0A:10 a=3aoYDsRNsgoN-TxrW0EA:9 a=CjuIK1q_8ugA:10 X-Rspamd-Queue-Id: 946058BCD7 X-Spamd-Bar: ------ Authentication-Results: mx1.freebsd.org X-Spamd-Result: default: False [-6.96 / 15.00]; NEURAL_HAM_MEDIUM(-1.00)[-1.000,0]; NEURAL_HAM_LONG(-1.00)[-1.000,0]; REPLY(-4.00)[]; NEURAL_HAM_SHORT(-0.96)[-0.955,0] X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 27 Jun 2019 15:48:03 -0000 On Thu, 27 Jun 2019, Andriy Gapon wrote: > Log: > upgrade the warning printf-s in bus accessors to KASSERT-s > > After this change sys/bus.h includes sys/systm.h. This is further namespace pollution. sys/systm.h is a prerequiste for all kernel headers except sys/param.h and the headers that that includes, since any kernel header (except sys/param.hm etc.) may have inlines which use features in systm.h, e.g., KASSERT() or an inline function. > Modified: head/sys/sys/bus.h > ============================================================================== > --- head/sys/sys/bus.h Thu Jun 27 14:26:57 2019 (r349458) > +++ head/sys/sys/bus.h Thu Jun 27 15:07:06 2019 (r349459) > @@ -35,6 +35,7 @@ > #include > #include > #include > +#include This header used to be relatively clean, with no polluting includes. The following low-quality headers in sys already include sys/systm.h: - capsicum.h:#include - fail.h:#include - fbio.h:#include - intr.h:#include - libkern.h:#include - malloc.h:#include - mbuf.h:#include - pmckern.h:#include - proc.h:#include - refcount.h:#include - seqc.h:#include - sf_buf.h:#include - zutil.h:#include This is especially bad in: - libkern.h. systm.h includes libkern.h, so this recurses. The recursion is bounded by the reinclude guard. libkern.h was broken by adding KASSERT()s to it. It is a leaf header, so must be written carefully. - malloc.h and mbuf.h. I fixed all the include pollution in them. They are now much more polluted than before I cleaned them - proc.h. Almost everything includes this. Including sys/systm.h in it hdes the bug of not including sys/systm.h in the correct place in .c files. > /** > * @defgroup NEWBUS newbus - a generic framework for managing devices > @@ -813,12 +814,9 @@ static __inline type varp ## _get_ ## var(device_t dev > int e; \ > e = BUS_READ_IVAR(device_get_parent(dev), dev, \ > ivarp ## _IVAR_ ## ivar, &v); \ > - if (e != 0) { \ > - device_printf(dev, "failed to read ivar " \ > - __XSTRING(ivarp ## _IVAR_ ## ivar) " on bus %s, " \ > - "error = %d\n", \ > - device_get_nameunit(device_get_parent(dev)), e); \ > - } \ > + KASSERT(e == 0, ("%s failed for %s on bus %s, error = %d", \ > + __func__, device_get_nameunit(dev), \ > + device_get_nameunit(device_get_parent(dev)), e)); \ > return ((type) v); \ > } \ > \ Inline functions in headers generally cause namespace pollution, like here. Some headers use macros which don't have that problem. mbuf.h used to be like that. The cleaned version of it in FreeBSD-4 includes only sys/queue.h. It has rotted to include sys/systm.h, sys/refcount.h, vm/uma.h. sys/lock.h and sys/sdt.h. Bruce