style(9) rules for nested includes
imp at bsdimp.com
Thu Mar 10 18:35:21 UTC 2011
On 03/10/2011 10:17, mdf at freebsd.org wrote:
> I recall a recent discussion/PR about nested includes in the context
> of<sys/linker_set.h> and<sys/queue.h> being a few of the only ones
> allowed. However, I don't see anything in style(9) about this.
> One rule for nested includes I've heard (that FreeBSD doesn't use, but
> just mentioning it for variety) is that every header file should
> compile on its own; that is, a .c file with nothing but #include
> <header.h> should compile without errors. This rule also makes some
> sense with the FreeBSD style convention of alphabetizing includes;
> otherwise it's kinda just happenstance that e.g.<sys/lock.h> comes
> alphabetically before<sys/sx.h>,<sys/mutex.h>,<sys/rwlock.h> and
> <sys/rmlock.h>, and barely before<sys/lockmgr.h>.
We've explicitly not done that to date. In the past, this would cause
extra I/O and processing that would slow down compile times by a
measurable amount. These days, I think you'd be hard pressed to even
measure the effect. However, if we adopted this rule, you tend to get
n^2 behavior because each layer of the onion redundantly would be
including the inner onion...
A survey of the include files shows uneven application of rules. Some
require additional includes before them (the most common being
sys/param.h or sys/types.h), some include some prereques, etc. There's
no hard and fast rule that will describe the current state of affairs.
> Now we come to the reason I ask. I'm working on a patch to change the
> static sysctl code to use the standard SYSININT/SYSUNINIT code rather
> than have special treatment in kern_linker.c, but to do this I need to
> either change quite a few places that include<sys/sysctl.h>, or
> include<sys/kernel.h> instead of<sys/linker_set.h> in sysctl.h, as
> the SI_SUB_SYSCTLS value isn't visible otherwise.
> This, though, shows a bug in<sys/kernel.h>, where it uses the
> constant MAXPATHLEN in the extern declaration of kernelname (which
> itself I think is not standard C, IIRC, but there are several uses of
> sizeof(kernelname) that don't work otherwise). So then<sys/kernel.h>
> needs<sys/param.h> to build.
It is cascade problems like this which both militate for the change and
against it. For the change because it causes less churn (in theory).
Against the change because you tend to accumulate lots of things that
used to be required, but are no longer, but which consumers come to
bogusly depend on, making it hard to detangle the thicket.
An interesting number of include files assume that param.h has already
been included, and avoids the extra parsing overhead that skipping
param.h N times would cause.
It likely is better to fix the 6 places in the kernel where sizeof is
used than to redundantly include param.h. Also, while you're there, fix
the strncpy to a strlcpy:
./kern/kern_mib.c: kernelname, sizeof kernelname, "Name of kernel
./sun4v/sun4v/machdep.c: strlcpy(kernelname, env,
./sparc64/sparc64/machdep.c: strlcpy(kernelname, env,
./powerpc/aim/machdep.c: strlcpy(kernelname, env,
./amd64/amd64/machdep.c: strlcpy(kernelname, env,
./ia64/ia64/machdep.c: strncpy(kernelname, p, sizeof(kernelname)
> So, which files are okay to include in an include file? Does someone
> (I'll do it if we can agree on a list) want to put this in style(9)?
I doubt it would be wise to make a blanket statement like all headers
must be self-contained. There's name-space contamination issues to
worry about, etc.
More information about the freebsd-arch