style(9) rules for nested includes

[Warner Losh]

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 
file booted");
./sun4v/sun4v/machdep.c:        strlcpy(kernelname, env, 
sizeof(kernelname));
./sparc64/sparc64/machdep.c:        strlcpy(kernelname, env, 
sizeof(kernelname));
./powerpc/aim/machdep.c:        strlcpy(kernelname, env, 
sizeof(kernelname));
./amd64/amd64/machdep.c:        strlcpy(kernelname, env, 
sizeof(kernelname));
./ia64/ia64/machdep.c:        strncpy(kernelname, p, sizeof(kernelname) 
- 1);

> 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.

Warner