[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Libcdio-devel] HAVE_CONFIG_H vs EXTERNAL_LIBCDIO_CONFIG_H
From: |
Pete Batard |
Subject: |
Re: [Libcdio-devel] HAVE_CONFIG_H vs EXTERNAL_LIBCDIO_CONFIG_H |
Date: |
Fri, 27 Jan 2012 12:46:27 +0000 |
User-agent: |
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:9.0) Gecko/20111222 Thunderbird/9.0.1 |
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