[Libcdio-devel] autoheader in the global namespace

[Robert William Fuller]

Recently, I re-tooled my project (cued) to use autotools.  I started out 
by doing the naive thing when it came to including my project's 
config.h.  Here is an excerpt from rip.c:

    #include "config.h" // HAVE_CDIO_MMC_LL_CMDS_H
    #include "unix.h"
    #include "util.h"

    #define DO_NOT_WANT_PARANOIA_COMPATIBILITY
    #include <cdio/cdio.h>
    #include <cdio/mmc.h> // CDIO_MMC_READ_TYPE_ANY
    #ifdef HAVE_CDIO_MMC_LL_CMDS_H
    #include <cdio/mmc_ll_cmds.h>
    #endif

This seemed to work fine.  I proceeded to make my project compile under 
Linux, Open Indiana (Solaris), and FreeBSD.  So far so good.  Then, I 
tried to port to MacOS, not because I am particularly concerned about 
supporting it, but because I figured it was different enough from the 
other 3 platforms that something might break.

Consequently, when trying to build under MacOS, I was rewarded with this 
nastiness:

    gcc -DHAVE_CONFIG_H -I. -I../..  -I/opt/local/include 
-I/opt/local/include   -I/opt/local/include   -I/opt/local/include 
-I../../lib/cued   -std=gnu99 -Wall -Wstrict-aliasing=3 -Wformat=2  -g 
-O2 -MT rip.o -MD -MP -MF .deps/rip.Tpo -c -o rip.o rip.c
    In file included from /opt/local/include/cdio/types.h:34,
                     from /opt/local/include/cdio/cdio.h:35,
                     from rip.c:25:
    /opt/local/include/cdio/cdio_config.h:306:1: warning: "PACKAGE" 
redefined
    In file included from rip.c:20:
    ../../config.h:91:1: warning: this is the location of the previous 
definition
    In file included from /opt/local/include/cdio/types.h:34,
                     from /opt/local/include/cdio/cdio.h:35,
                     from rip.c:25:
    /opt/local/include/cdio/cdio_config.h:309:1: warning: 
"PACKAGE_BUGREPORT" redefined

Add a half dozen more of these warnings, and you get the gist. 
Apparently, there was a conflict between my project's config.h and 
libcdio's config.h.  Yet, the warning only showed up on MacOS, not any 
of the other platforms.

The reason it showed up on MacOS is that MacPorts installs header files 
in /opt/local rather than /usr/include.  GCC treats system headers 
differently than other headers.  It ignores redefined macros if they are 
redefined by system headers (i.e. in /usr/include).  This is documented 
under the GCC option "-isystem".

So PACKAGE_NAME was defined to be "libcdio" rather than "cued", the name 
of my package.  VERSION was defined to be "0.83" instead of "1.20". 
This is a somewhat insidious problem because I got NO warning on ANY 
platform other than MacOS.

Next, I searched this mailing list to see what I could learn.  I think I 
fixed my problem by switching to the following code (but maybe not, read 
on:)

    #ifdef HAVE_CONFIG_H
    #include "config.h" // HAVE_CDIO_MMC_LL_CMDS_H
    #define __CDIO_CONFIG_H__ // avoid conflicts with libcdio
    #endif
    #include "unix.h"
    #include "util.h"

    #define DO_NOT_WANT_PARANOIA_COMPATIBILITY
    #include <cdio/cdio.h>
    #include <cdio/mmc.h> // CDIO_MMC_READ_TYPE_ANY
    #ifdef HAVE_CDIO_MMC_LL_CMDS_H
    #include <cdio/mmc_ll_cmds.h>
    #endif

I am not totally satisfied with this solution.  If every library that I 
used included its config.h in the global namespace, I might end up with 
something like this (assuming that other projects create config.h 
include guards which they generally do not:)

    #ifdef HAVE_CONFIG_H
    #include "config.h"
    #define __CDIO_CONFIG_H__
    #define __SNDFILE_CONFIG_H__
    #define __CDDB_CONFIG_H__
    etc.

MOREOVER, I am not sure that <cdio/types.h> will always do the right 
thing when I define __CDIO_CONFIG_H__ before including it.  You might 
argue I should have used the other proposed solution for these sorts of 
conflicts which would look something like this:

    #include <cdio/cdio.h>
    #include <cdio/cdio_unconfig.h> # remove *all* symbols libcdio defines

    #include "config.h"
    #include "unix.h"
    #include "util.h"

    #define DO_NOT_WANT_PARANOIA_COMPATIBILITY
    #include <cdio/mmc.h> // CDIO_MMC_READ_TYPE_ANY
    #ifdef HAVE_CDIO_MMC_LL_CMDS_H
    #include <cdio/mmc_ll_cmds.h>
    #endif

Note that I need to include my "config.h" before <cdio/mmc_ll_cmds.h>, 
BUT I am supposed to include the cdio headers before my "config.h". 
Also I am not convinced that cdio_unconfig.h won't remove some 
definitions that are included in the GCC specs for some platform or in 
the system header files for that platform.  For example, cdio_unconfig.h 
undefines "const".

Now the general autotools lore seems to be that you should never include 
config.h in a header file, but only in a .c file.  This is generally 
attributed to the lack of include guard.  I think that is missing the 
point.  Here, I will argue that you should never include config.h in a 
header file because of the global namespace pollution and the potential 
for silent conflicts that show up as bugs.

One path out of this conflict is to remove code such as this from 
<cdio/read.h>:

    #ifndef EXTERNAL_LIBCDIO_CONFIG_H
    #define EXTERNAL_LIBCDIO_CONFIG_H
    /* Need for HAVE_SYS_TYPES_H */
    #include <cdio/cdio_config.h>
    #endif

    #ifdef HAVE_SYS_TYPES_H
    /* Some systems need this for off_t and ssize. */
    #include <sys/types.h>
    #endif

However, things aren't so simple when it comes to <cdio/types.h>, which 
seems to have a real need to include its config.h.

Another path out of this conflict would be to create yet another 
config.h file, perhaps named cdio_header_config.h that contains ONLY the 
macros needed by the cdio headers, such as HAVE_SYS_TYPES_H and 
HAVE_STDINT_H.  Yet again, /usr/include/cdio/types.h complicates things 
because there is so much that it needs.

So, I am stuck, but still thinking about the problem.  Thoughts?

Rob