Date: Thu, 10 Mar 2011 11:24:09 -0700 From: Warner Losh <imp@bsdimp.com> To: freebsd-arch@freebsd.org Subject: Re: style(9) rules for nested includes Message-ID: <4D791749.9020803@bsdimp.com> In-Reply-To: <AANLkTikqBJON46-EJFPPktT82L8dgX6dwwDrxWwFqumU@mail.gmail.com> References: <AANLkTikqBJON46-EJFPPktT82L8dgX6dwwDrxWwFqumU@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On 03/10/2011 10:17, mdf@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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4D791749.9020803>