kern/47794: circular header dependencies force header inclusion order with <machine/byte_swap.h>

[rhansen%bbn.com@localhost]

>Number:         47794
>Category:       kern
>Synopsis:       circular header dependencies force header inclusion order with 
><machine/byte_swap.h>
>Confidential:   no
>Severity:       non-critical
>Priority:       medium
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Thu May 02 22:40:01 +0000 2013
>Originator:     Richard Hansen
>Release:        6.0.1
>Organization:
BBN
>Environment:
NetBSD netbsd.bbn.com 6.0.1 NetBSD 6.0.1 (GENERIC) amd64
>Description:
Due to circular header dependencies, '#include <machine/byte_swap.h>' without 
'#include <sys/bswap.h>' before it (either directly or indirectly, e.g., via 
<sys/endian.h>) causes macro redefined error messages.

 1. machine/byte_swap.h includes sys/types.h
    -> sys/types.h includes machine/endian.h
       -> machine/endian.h includes sys/endian.h
          -> sys/endian.h includes machine/bswap.h
             -> machine/bswap.h includes machine/byte_swap.h
                -> machine/byte_swap.h already included, do nothing
             -> machine/bswap.h includes sys/bswap.h
                -> sys/bswap.h defines __BYTE_SWAP_U??_VARIABLE
 2. machine/byte_swap.h defines __BYTE_SWAP_U??_VARIABLE

I understand that machine/*.h are arch-specific headers which may make this 
look like an amd64-specific bug, but there is at least this circular header 
dependency in all architectures:

  * sys/types.h includes machine/endian.h
  * machine/endian.h includes sys/endian.h (on all architectures)
  * sys/endian.h includes sys/types.h
>How-To-Repeat:
$ gcc -O2 -Werror -o /dev/null -x c /usr/include/machine/byte_swap.h
cc1: warnings being treated as errors
/usr/include/machine/byte_swap.h:45:0: error: "__BYTE_SWAP_U64_VARIABLE" 
redefined
/usr/include/sys/bswap.h:30:0: note: this is the location of the previous 
definition
/usr/include/machine/byte_swap.h:54:0: error: "__BYTE_SWAP_U32_VARIABLE" 
redefined
/usr/include/sys/bswap.h:34:0: note: this is the location of the previous 
definition
/usr/include/machine/byte_swap.h:63:0: error: "__BYTE_SWAP_U16_VARIABLE" 
redefined
/usr/include/sys/bswap.h:38:0: note: this is the location of the previous 
definition

>Fix:
Remove the line '#include <machine/endian.h>' from sys/types.h -- it doesn't 
need it.  It's unclear to me why it was ever included in the first place.

Unfortunately, sys/types.h has been including machine/endian.h for two decades 
now, so removing the inclusion breaks lots of (bad) code.  Much -- but not all 
-- of the problematic code can be addressed by including sys/endian.h from 
sys/param.h (which, based on the description in its man page, seems to be the 
header equivalent of a junk drawer).  In particular, the AC_C_BIGENDIAN 
autoconf test checks sys/param.h for the endianness macros provided by 
sys/endian.h.

Unfortunately, finding all of the bad code is not trivial -- some of the bad 
code simply does stuff like this:

    #if BYTE_ORDER == BIG_ENDIAN
      /* foo */
    #else
      /* bar */
    #endif

Code like this will still compile if machine/endian.h hasn't been included but 
may fail at runtime.