Re: [Libcdio-devel] HAVE_CONFIG_H vs EXTERNAL_LIBCDIO_CONFIG_H

[Pete Batard]

On 2012.01.27 03:48, Rocky Bernstein wrote:
> I'm not sure that the rationale you understand is why those two are there.
>
> cdio_config.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".

Ah, OK. I was seeing it as a way to avoid conflicts between multiple 
config.h on projects that reuse multiple parts.

My problem is, having duplicates as both a liability (if for any reason 
they get out of sync) and it is also a bother for MSVC and derivatives 
(eg. WDK), since we need to add steps to duplicate config.h and make 
sure the files are kept in sync, as we can't rely on configure/make.

Instead, I'd propose removing cdio_config.h from the include/cdio 
headers altogether.

A quick check of the headers shows that besides types.h, only 3 headers 
actually use HAVE_#### macros: cdio.h, read.h and rock.h, and only for:
- HAVE_SYS_TYPES_H
- HAVE_UNISTD_H
- HAVE_S_ISSOCK / HAVE_S_ISLNK

Now, the ones from cdio.h don't seem to serve any purpose, since we 
don't use any of the sys/types.h or unistd.h types directly there. And 
sys/types could actually be removed since cdio/types.h also includes 
sys/types.h if available. I suspect unistd.h is used downstream for 
ssize_t but that's probably about it.

But with both sys/types.h and unistd.h are expected to be available on 
most platforms, it should be fairly easy to remove the HAVE_ and add 
explicit #ifdefs for the platforms where they aren't. Thus, removing the 
first two HAVE_### should be easy as long as we can figure out the 
platforms that don't have them (MSVC would be one of those for unistd.h)

The S_ISSOCK, S_ISLNK should also be something we should be able to sort 
out without having to rely on configure, especially as they are macros 
which we immediately redefine if not available. There seems to be only 
one definition possible for these macros across platforms, so the 
HAVE_S_ISSOCK and HAVE_S_ISLNK can be removed as well.

So that only leaves cdio/types.h.

The additional HAVE_### types.h required there are:
- HAVE_STDINT_H, HAVE_INTTYPES_H, HAVE_STDBOOL_H
- HAVE_ISOC99_PRAGMA

HAVE_STDBOOL_H is not an issue, since we only use it for true/false and 
bool, which we should be able to redefine always (if not already 
defined) to avoid the dependency. I very much doubt that we will run 
into conflicts when mixing our definitions of boolean types with 
potentially slightly different ones, so IMO, the stdbool.h requirement 
in the API headers can and should go.

As to HAVE_ISOC99_PRAGMA, well, since we should be able to detect C99 
support from the compiler (__STDC_VERSION__ > 199901 for gcc, __C99__ or 
some other macro on others), and the expectation would be that a 
compiler that states C99 will have C99 pragma support, I don't see it as 
much of an issue as well. This is even more true as anything GNU won't 
rely on it (uses GNUC_PACKED) => HAVE_ISOC99_PRAGMA can go too.

So this really leaves us with stdint.h and inttypes.h (and most likely 
stdint.h only as I don't think the headers actually rely on anything 
from inttypes.h)...

While these are C99 files, which we could detect by checking C99 
support, they may very well be available when C99 is not in use. In 
fact, this is how libcdio is compiled on gcc platforms at the moment.

The only types we really appear to require in the headers from stdint.h, 
at least in the cdio_headers, are the (u)int##_t. However, unlike was is 
the case for bool/false/true, trying to redefine those always so that we 
can drop the stdint.h requirement may be tricky.

My proposal then, since this is 2012 and fewer and fewer people are 
expected to use libcdio on the older platforms, is to assume stdint.h to 
be available by default, except where specified (AMIGA, older MSVCs and 
???). Now, I reckon that this may very well break existing apps, but on 
the other hand, given the headache that not using standard int types can 
incur, if it pushes people try to locate an stdint.h for their platform, 
and start to use proper types such as int32_t, uint64_t in their 
applications, this may not be such a bad move...

Also, the expectation is that anybody who finds their platform broken 
would contact this mailing list, so that we add a special case for their 
platform.

I would however suggest bumping the libcdio version number of any 
libcdio release we do that, as we may want to let people know that 
potential breakage may occur. Besides, we may also have potential 
breakage for stream operations due to previous commits anyway, so we may 
as well use a fake API bump to sort a few things.

Therefore, the next commit you should see in my branch should be one 
that removes the requirement for cdio_config.h in the API headers.

Regards,

/Pete