Re: [Libcdio-devel] HAVE_CONFIG_H vs EXTERNAL_LIBCDIO_CONFIG_H

[Rocky Bernstein]

On Thu, Jan 26, 2012 at 6:09 PM, Pete Batard <address@hidden> wrote:

> The first hurdle we meet for MSVC support has to do with the above, and
> the apparent lack of harmonization between using a (topdir)/config.h or a
> cdio/cdio_config.h.
>
> As a matter of fact, one of the first thing the make process has to do is
> duplicate the toplevel config.h into include/cdio/cdio_config.h, because
> the current libcdio compilation process very much seems to require both.


> For instance, cdio/types.h has only:
>
> #ifndef EXTERNAL_LIBCDIO_CONFIG_H
> #define EXTERNAL_LIBCDIO_CONFIG_H
> #include <cdio/cdio_config.h>
> #endif
>
> Which means only cdio/config.h can be used there, whereas lib/udf/udf_fs.c
> has only:
>
> #ifdef HAVE_CONFIG_H
> #include "config.h"
> #define __CDIO_CONFIG_H__ 1
> #endif
>
> Now, I can understand the rationale for giving the choice between using
> either a toplevel config.h or an "external" cdio_config.h,


I'm not sure that the rationale you understand is why those two are there.

cdio/types.h that you cite above is externally installed. It is available
for use in compilation before and after libcdio is installed.  It forms the
API.

On the other hand lib/udf/udf_fs.c that you also cite above is part of the
implementation -- compiled at "build time".

So I'm not sure what this "choice" thing is about that you seem to be
referring to. I don't see any need to give udf_fs.c  a choice -- it should
to use config.h. I don't see a need to give cdio/types.h a choice either -
there is some configuration-specific feature it uses and it since this
header is external so it has to get it from cdio_config.h not config.h.

At present, cdio_config.h and config.h contain exactly the same thing. In
the past I had tried to only include in cdio_config.h only those things
that might be of more interest from a "how was libcdio configured?"
standpoint or from the standpoint of configuration-specific details that
headers like cdio/types.h might need. As an example of the former -- was
libcddb available? An  example of the latter -- is this architecture
bigendian?

 But I gave up on this since it was too difficult to maintain.


> but in this case, shouldn't all files provide an exclusive choice between
> including one or the other? In other words, shouldn't we have all our
> config dependent files starting with:
>
> #if defined(HAVE_CONFIG_H) && !defined(__CDIO_CONFIG_H__)
> #define __CDIO_CONFIG_H__ 1
> #include "config.h"
> #else
> #if !defined(EXTERNAL_LIBCDIO_**CONFIG_H)
> #define EXTERNAL_LIBCDIO_CONFIG_H
> #include <cdio/cdio_config.h>
> #endif
> #endif
>
> As a matter of fact, this is precisely how I modified all the sources that
> I integrated for MSVC support in my app, and I'm planning to do the same to
> add MSVC support in -pbatard. However, because the current libcdio seems to
> completely ignore this issue, I can't help wondering if I'm missing
> something...
> Is there any rationale for what looks like a very messy state of config
> support
>

> Regards,
>
> /Pete
>
>