Date: Fri, 28 Jun 2019 01:47:51 +1000 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Andriy Gapon <avg@freebsd.org> Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r349459 - head/sys/sys Message-ID: <20190628012059.D2091@besplex.bde.org> In-Reply-To: <201906271507.x5RF775Q070616@repo.freebsd.org> References: <201906271507.x5RF775Q070616@repo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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 <machine/_bus.h>
> #include <sys/_bus_dma.h>
> #include <sys/ioccom.h>
> +#include <sys/systm.h>
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 <sys/systm.h>
- fail.h:#include <sys/systm.h>
- fbio.h:#include <sys/systm.h>
- intr.h:#include <sys/systm.h>
- libkern.h:#include <sys/systm.h>
- malloc.h:#include <sys/systm.h>
- mbuf.h:#include <sys/systm.h>
- pmckern.h:#include <sys/systm.h>
- proc.h:#include <sys/systm.h>
- refcount.h:#include <sys/systm.h>
- seqc.h:#include <sys/systm.h>
- sf_buf.h:#include <sys/systm.h>
- zutil.h:#include <sys/systm.h>
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20190628012059.D2091>