svn commit: r272574 - head/sys/sys

[Bruce Evans]

On Sun, 5 Oct 2014, Mateusz Guzik wrote:

> Log:
>  seq_t needs to be visible to userspace
>
>  Pointy hat to:	mjg
>  Reported by: bz
>  X-MFC:	with r272567

The namespace pollution is more disgusting than before.

> Modified: head/sys/sys/seq.h
> ==============================================================================
> --- head/sys/sys/seq.h	Sun Oct  5 21:34:56 2014	(r272573)
> +++ head/sys/sys/seq.h	Sun Oct  5 21:39:50 2014	(r272574)
> @@ -29,6 +29,16 @@
> #define _SYS_SEQ_H_
>
> #ifdef _KERNEL
> +#include <sys/systm.h>
> +#endif

Namespace pollution.  In the kernel, it is the responsibility of .c
files to include <sys/systm.h> before any other header except
<sys/param.h>, in case bad headers like this one use inline functions
with KASSERT() either now or later.

> +#include <sys/types.h>

Namespace pollution.  In the kernel, it is the responsibility of .c
files to include <sys/param.h> before any other header, since it has
standard namspace pollution that other headers may depend on now or
later.  This pollution includes <sys/types.h>, even outside the
kernel.  It is a documented style bug to include both <sys/types.h>
and <sys/param.h>.  It is a bug to include <sys/types.h> and not
<sys/param.h> in the kernel, except in very special cases where only
headers with a stable API are included after it (otherwise, the others
might grow a need for <sys/param.h> or <sys/systm.h>.

Outside of the kernel, the necessary types and no others should be
declared in headers.


> +/*
> + * seq_t may be included in structs visible to userspace
> + */
> +typedef uint32_t seq_t;
> +
> +#ifdef _KERNEL
>
> /*
>  * Typical usage:
> @@ -54,10 +64,7 @@
>  * 	foo(lobj);
>  */
>
> -typedef uint32_t seq_t;
> -
> /* A hack to get MPASS macro */
> -#include <sys/systm.h>
> #include <sys/lock.h>

Now the comment doesn't match the code.  The hack is to include both
<sys/systm.h> and <sys/lock.h>.  MPASS is defined in <sys/lock.h>.
<sys/lock.h> has minimal pollution.  In particular, it doesn't
include <sys/systm.h>.  It uses MPASS() with a KASSERT(), but this
is in a macro so files that actually use the macro need to include
<sys/param.h>.  <sys/filedesc.h>'s includers are now such files, but
<sys/filedesc.h> polluted instead.  This commit moves half of the
hack earlier than needed and far from the comment that documents it.

>
> #include <machine/cpu.h>

If you remove the <sys/systm.h> part of the hack, some .c files might
break.  This is a feature.  It detects 2 bugs:
- nested pollution.  IIRC, <sys/filedesc.h> is now polluted with an
   include of <sys/seq.h>, so not only files that include <sys/seq.h>
   are effected
- the brokenness of the files that break.  <sys/filedesc.h> is included
   much more, and any .c file that is missing the include of
   <sys/systm.h>, or includes it miss-sorted in alphabetical or totally
   disordered after <sys/filedesc.h>.  Part of the feature is that you
   get to fix all such broken .c files, or better implement new headers
   without namespace pollution.  The only easy way to avoid pollution
   from KASSERT() in headers is to only use it in other macros.  Some
   inline functions are so small that this is clean enough.  But ones
   that use KASSERT() are not so small.  Just using it makes them too
   large to inline (when it is enabled).

Bruce