svn commit: r197638 - projects/tcp_ffcaia2008_8.x/sys/kern

[Lawrence Stewart]

Bruce Evans wrote:
> On Wed, 30 Sep 2009, Lawrence Stewart wrote:
> 
>> Log:
>>  Alphabetically order includes.
>> ...
>> Modified: projects/tcp_ffcaia2008_8.x/sys/kern/kern_alq.c
>> ============================================================================== 
>>
>> --- projects/tcp_ffcaia2008_8.x/sys/kern/kern_alq.c    Wed Sep 30 
>> 08:48:59 2009    (r197637)
>> +++ projects/tcp_ffcaia2008_8.x/sys/kern/kern_alq.c    Wed Sep 30 
>> 09:53:03 2009    (r197638)
>> @@ -36,7 +36,9 @@ __FBSDID("$FreeBSD$");
>> #include "opt_mac.h"
>>
>> #include <sys/param.h>
>> -#include <sys/systm.h>
>> +#include <sys/alq.h>
>> +#include <sys/eventhandler.h>
>> +#include <sys/fcntl.h>
>> #include <sys/kernel.h>
>> #include <sys/kthread.h>
>> #include <sys/lock.h>
>> @@ -44,12 +46,9 @@ __FBSDID("$FreeBSD$");
>> #include <sys/mutex.h>
>> #include <sys/namei.h>
>> #include <sys/proc.h>
>> -#include <sys/vnode.h>
>> -#include <sys/alq.h>
>> -#include <sys/malloc.h>
>> +#include <sys/systm.h>
>> ...
> 
> <sys/systm.h> should not be sorted alphabeticaly, since it declares
> things like KASSERT() and (by nested includes) hundreds of inline
> functions (especially ones in <machine/atomic.h>, <machine/cpufunc.h>
> and <sys/libkern.h>) which are used in many other headers.  It should
> be sorted immediately after <sys/param.h> where it was.

Ok got it, thanks for the pointer. Everything compiled fine which is why 
I made the change but as you say there must be some "pollution" 
somewhere. Does this requirement to keep the systm.h include up top 
warrant a mention in style(9) or is it just something to remember?

> 
> Mis-sorting it is sometimes masked by namespace pollution in other
> headers.  <sys/mbuf.h> has the grossest namespace pollution despite
> me once completely cleaning it up :-(.  It now includes <sys/systm.h>
> and <vm/uma.h>.  (<vm/uma.h> is even more disgusting.  It starts by
> including <sys/param.h> and says that this is "/* for NULL */", despite
> there being a whole header <sys/_null.h> for the purpose of defining
> NULL and thus avoiding namespace which would be caused by defining
> NULL in a more central header, and despite it using _much_ more of
> <sys/param.h> than the definition of NULL.)  But most headers aren't
> as bad.  Thus sorting <sys/systm.h> alphabetically (near the end)
> rarely works, and even when it works it is fragile and will break when
> a header sorted before it starts using KASSERT().

Random, likely naive thought: is there some way to detect if a header 
has been included before or after another header? Could be done fairly 
easily with an external script I guess, but something explicit in the 
files (some sort of preprocessor and macro magic?) would be more useful 
I suspect. Perhaps there could be a warning emitted if ordering is 
violated or an include is detected that shouldn't be there. If done 
right, it would likely do the job of educating people about correct 
ordering/inclusion and go a long way to reducing "pollution creep".

Cheers,
Lawrence