svn commit: r280346 - head/sys/sys

Sun Mar 22 22:53:41 UTC 2015

On Sun, 22 Mar 2015, Pedro F. Giffuni wrote:

> Log:
>  Small style(9) cleanup.
>
>  #define should always be followed by a tab not space.

Thanks, but not in comments.

> Modified: head/sys/sys/cdefs.h
> ==============================================================================
> --- head/sys/sys/cdefs.h	Sun Mar 22 13:11:56 2015	(r280345)
> +++ head/sys/sys/cdefs.h	Sun Mar 22 15:37:05 2015	(r280346)
> @@ -70,20 +70,20 @@
> #if defined(__GNUC__) || defined(__INTEL_COMPILER)
>
> #if __GNUC__ >= 3 || defined(__INTEL_COMPILER)
> -#define __GNUCLIKE_ASM 3
> -#define __GNUCLIKE_MATH_BUILTIN_CONSTANTS
> +#define	__GNUCLIKE_ASM 3
> +#define	__GNUCLIKE_MATH_BUILTIN_CONSTANTS

The commit that added the __INTEL_COMPILER stuff was well-intentioned
but not ready for commit.  The lines changed by it still have mounds
of style bugs.  They mostly still don't use tabs where they are more
needed -- to line up the #defined values.

The __INTEL_COMPILER stuff was only used in a few places for i386
originally (not even in the corresponding places for amd64), and
has rotted.  It is well intentioned, but mostly just makes the
code uglier where it is actually used.

> ...
> #ifndef __INTEL_COMPILER
> # define __GNUCLIKE_CTOR_SECTION_HANDLING 1
> #endif

Indenting 'define' after '#' is another style bug.  Apparently this
defeated your pattern matcher, so the space after 'define' is not fixed
here.

>
> -#define __GNUCLIKE_BUILTIN_CONSTANT_P 1
> +#define	__GNUCLIKE_BUILTIN_CONSTANT_P 1
> # if defined(__INTEL_COMPILER) && defined(__cplusplus) \
>     && __INTEL_COMPILER < 800

'&&' on a new line is another style bug.  It is gnu-style.

> @@ -111,19 +111,19 @@
> # define __GNUCLIKE_MATH_BUILTIN_RELOPS
> #endif
>
> -#define __GNUCLIKE_BUILTIN_MEMCPY 1
> +#define	__GNUCLIKE_BUILTIN_MEMCPY 1
>
> /* XXX: if __GNUC__ >= 2: not tested everywhere originally, where replaced */
> -#define __CC_SUPPORTS_INLINE 1
> -#define __CC_SUPPORTS___INLINE 1
> -#define __CC_SUPPORTS___INLINE__ 1
> +#define	__CC_SUPPORTS_INLINE 1
> +#define	__CC_SUPPORTS___INLINE 1
> +#define	__CC_SUPPORTS___INLINE__ 1

3 different macros to support the different spellings of 'inline' are
excessive, and also wrong:
- __inline__ is a gnu spelling.  Any use of it is a style and portability
   bug.  Using it defeats portability ifdefs in this file, since only
   the FreeBSD spelling __inline is really supported in this file.  The
   macro __CC_SUPPORTS___INLINE__ to support it shouldn't exist.
   Currently, there are 256 lines in /sys with this style bug (mostly
   in mips, fuse, xen, drm and contrib) :-(.  There are just 3 lines
   which reference the macro:
   - 1 in cdefs.h to define it
   - 1 in cx to do home made portability stuff (inline -> __inline__).
     The FreeBSD spelling of this portability stuff is __inline or just
     plain inline in sloppier code.  Even the sloppier code became
     portable to new compilers with C99.
   - 1 in iir.  iir uses __inline__ a lot, and just fails when the macro
     says that it is not supported.
- __inline is the FreeBSD spelling.  It is used a lot.  About 3500
   times in /sys.  The macro shouldn't exist, since portability ifdefs
   in cdefs.h are supposed to make __inline usable for most cases,
   by defining it to nothing if required.  These portability ifdefs
   are mostly for handling the difference between C90 and gcc in 1990,
   and aren't complicated enough to handle all 1990 model compilers,
   but most code didn't care about the difference even in 1990.  There
   are just 5 uses of the macro (not counting comments or its definition):
   - 2 in cpufunc.h (1 each for amd64 and i386)
   - 3 in in_cksum.h (1 each for amd64, mips, and powerpc.  i386 is
     gratuitously different from amd64 due to different bad fixes for
     bugs in the inline asm).
- inline is the C99 spelling.  It is used almost 6000 times (mostly
   in contrib'ed uglyware).  The macro for it made sense before C99
   existed, but was committed after 1999 and has never been used.

> -#define __CC_SUPPORTS___FUNC__ 1
> -#define __CC_SUPPORTS_WARNING 1
> +#define	__CC_SUPPORTS___FUNC__ 1
> +#define	__CC_SUPPORTS_WARNING 1

__func__ is like __inline, but has better compatibility support, so
the macro for it should not exist.  The macro is used just once
(in ichsmb, for just 1 of 12 instances of __func__ in this file).

>
> -#define __CC_SUPPORTS_VARADIC_XXX 1 /* see varargs.h */
> +#define	__CC_SUPPORTS_VARADIC_XXX 1 /* see varargs.h */
>
> -#define __CC_SUPPORTS_DYNAMIC_ARRAY_INIT 1
> +#define	__CC_SUPPORTS_DYNAMIC_ARRAY_INIT 1

The features corresponding to these macros are much more arcane than
__inline and __func__.  __CC_SUPPORTS_VARADIC_XXX is used just once
(in i386 varags.h.  Varargs is very compiler-dependent, but only the
implementation should know the details, and <machine/varargs> is a
better place to put them than here even if they only depend on the
compiler).  __CC_SUPPORTS_VARADIC_XXX is not used.  (All counts in
/sys).

> ...
> @@ -139,7 +139,7 @@
>
> /*
>  * The __CONCAT macro is used to concatenate parts of symbol names, e.g.
> - * with "#define OLD(foo) __CONCAT(old,foo)", OLD(foo) produces oldfoo.
> + * with "#define	OLD(foo) __CONCAT(old,foo)", OLD(foo) produces oldfoo.

Example of a comment mangled by the substitution.

> @@ -234,13 +234,13 @@
> #define	__section(x)	__attribute__((__section__(x)))
> #endif
> #if defined(__INTEL_COMPILER)
> -#define __dead2		__attribute__((__noreturn__))
> -#define __pure2		__attribute__((__const__))
> -#define __unused	__attribute__((__unused__))
> -#define __used		__attribute__((__used__))
> -#define __packed	__attribute__((__packed__))
> -#define __aligned(x)	__attribute__((__aligned__(x)))
> -#define __section(x)	__attribute__((__section__(x)))
> +#define	__dead2		__attribute__((__noreturn__))
> +#define	__pure2		__attribute__((__const__))
> +#define	__unused	__attribute__((__unused__))
> +#define	__used		__attribute__((__used__))
> +#define	__packed	__attribute__((__packed__))
> +#define	__aligned(x)	__attribute__((__aligned__(x)))
> +#define	__section(x)	__attribute__((__section__(x)))
> #endif
> #endif

__INTEL_COMPILER causes complications by not being fully gcc-compatible,
but here it is fully compatible and the duplication is spam.  Previously,
the spam wasn't even duplication since it had corrupt tabs.

Duplication defeats the point of complicated ifdefs elsewhere, where
the complications go in the direction of putting a single definition
under a complicated condition.

clang would require much messier ifdefs if it were not close to 100%
compatible, since it is actually used.

Bruce