Date: Mon, 10 Dec 2012 23:34:34 -0700 From: Carl Delsey <carl.r.delsey@intel.com> To: Bruce Evans <brde@optusnet.com.au> Cc: svn-src-head@FreeBSD.org, Jim Harris <jimharris@FreeBSD.org>, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org, Niclas Zeising <zeising@FreeBSD.org> Subject: Re: svn commit: r243960 - in head/sys: amd64/include i386/include x86/include Message-ID: <50C6D3FA.6050308@intel.com> In-Reply-To: <20121211144118.L959@besplex.bde.org> References: <201212062233.qB6MXWpP046167@svn.freebsd.org> <50C66E2E.5040302@freebsd.org> <50C67129.6090704@intel.com> <50C672D0.9090908@freebsd.org> <50C6764E.4040804@intel.com> <50C67C29.4070408@freebsd.org> <20121211144118.L959@besplex.bde.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On 12/10/2012 9:06 PM, Bruce Evans wrote: > On Tue, 11 Dec 2012, Niclas Zeising wrote: > >> On 12/11/12 00:54, Carl Delsey wrote: >>> On 12/10/12 16:40, Niclas Zeising wrote: >>>> ... >>>> libpciaccess uses bus_space_[read,write]_[1,2,4], which are defined in >>>> x86/bus.h. It does not use the quad functions. >>>> Regards! >>> Ok. In that case I won't ifdef out the functions themselves, just the >>> KASSERT in case libpciaccess expands in the future to 8 byte accesses >>> :-) I had another related change in the works. I'll add this change >>> in. >> >> Sounds good to me, thank you very much! >> Don't forget to ifdef the include o <sys/systm.h> as well as the >> KASSERTs. > > Including it at all is namespace pollution. > > It is a bug for any kernel .c file to not include <sys/systm.h> always > and early in its includes, because other includes may have KASSERT()s > in them. Not all includes that uses KASSERT() have the namespace > pollution, so adding it to some just breaks detection of the bug in > some cases. > > The folowing headers in <sys> have the pollution: libkern.h, mbuf.h, > pmckern.h, refcount.h. refcount.h only has the pollution if _KERNEL is > defined. It handles the problem of polluting userland (though I think > userland should be rewarded by syntax errors if it uses refcount.h or > machine/bus.h) by supplying a dummy KASSERT() for the !_KERNEL case. > > The following headers in <sys> use KASSERT(): buf.h, eventhandler.h, > lock.h, mbuf.h, mount.h, mutex.h, osd.h, proc.h, random.h, refcount.h. > Most of them are missing the pollution. > > The pollution in libkern.h is circular: libkern.h includes systm.h, and > systm.h includes libkern.h. systm.h's include of libkern.h and of many > other headers like machine/atomic.h and machine/cpufunc.h is standard > pollution -- direct includes of these are style bugs, and not including > systm.h always and early is a bug because it may break more than > KASSERT() > (other headers may also use inlines in libkern.h etc.). The circular > pollution in libkern.h is used because most files in libkern don't > include > systm.h directly; they mostly only include libkern.h directly. > > This and other pollution in mbuf.h is especially disgusting since > mbuf.h is one of few headers that I managed to finish the #include > de-pollution of in FreeBSD-3 or 2. > > Bruce Thanks Bruce. I'll fix that in my change. It will make the bus.h file a bit cleaner. This information should probably be in style(9). It talks a bit about how to include header files but it doesn't mention any of these details that I noticed. I'll take a stab at adding some of this info if I get a chance in the next few weeks. Carl
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?50C6D3FA.6050308>