Re: [Libcdio-devel] autoheader in the global namespace

[Rocky Bernstein]

On Thu, Oct 11, 2012 at 9:15 PM, Robert William Fuller <
address@hidden> wrote:

> Indeed, that is the case.  The CDIO_ prefix is a post version 0.83 change
> to libcdio.  I found the commit here:
>
> http://git.savannah.gnu.org/**gitweb/?p=libcdio.git;a=**commit;h=**
> 39bd8104b8576c71c2c170f59c39ed**c2823b6bf5<http://git.savannah.gnu.org/gitweb/?p=libcdio.git;a=commit;h=39bd8104b8576c71c2c170f59c39edc2823b6bf5>
>
> I suppose I could develop only for versions >= 0.90 which is not yet
> released.  I guess I need to build libcdio from git and develop against
> that.  Of course that means all sorts of ugliness, like overriding the
> include and library paths for libcdio to be /usr/local and things like
> that....
>

You read the post where I said I thought there would be a release towards
the end of the month, right?


>
> Rob
>
>
> On 10/11/2012 09:01 PM, Robert William Fuller wrote:
>
>> It's not limited to MacOS. My 64-bit Wheezy (Debian testing) distro does
>> not have the CDIO_ prefix on things. Do I need a later version of
>> libcdio than what will ship with Wheezy? I have libcdio 0.83 on Wheezy
>> and it lacks the CDIO_ prefix. From cdio_config.h on Wheezy:
>>
>> /* Define to the full name and version of this package. */
>> #define PACKAGE_STRING "libcdio 0.83"
>>
>> /* Define to the one symbol short name of this package. */
>> #define PACKAGE_TARNAME "libcdio"
>>
>> /* Define to the home page for this package. */
>> #define PACKAGE_URL ""
>>
>> /* Define to the version of this package. */
>> #define PACKAGE_VERSION "0.83"
>>
>> Rob
>>
>> On 10/11/2012 08:11 PM, Rocky Bernstein wrote:
>>
>>> All #define's in cdio_config.h should start CDIO_. For example:
>>>
>>>
>>> #define CDIO_PACKAGE "libcdio"
>>>
>>> The transformation is done inside include/cdio/Makefile.am. For
>>> GNU/Linux,
>>> the relevant portion expands in Makefile to:
>>>
>>> @/bin/sed -r -e 's/^(#[ \t]*define) /\1 CDIO_/'
>>> $(top_builddir)/config.h>>**cdio_config.h
>>>
>>> There is some adjustment for other kinds of "sed" options.
>>>
>>> So most likely your sed isn't sed'ing on OSX.
>>>
>>>
>>> On Thu, Oct 11, 2012 at 7:29 PM, Robert William Fuller<
>>> address@hidden> wrote:
>>>
>>>  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
>>>>
>>>>
>>>>
>>>
>>
>
>