[Zlib-devel] zlib 1.2.6.1: build breaks on MinGW because of __builtin_bswap32

[Cosmin Truta]

Hi,

I'll start with the summary: may I suggest introducing
HAVE__BUILTIN_BSWAP32, and replacing !NO_VIS with
HAVE__ATTRIBUTE__VISIBILITY? Otherwise, compilation breaks, unless it
is autoconfigured via ./configure (and breaks win32/Makefile.gcc in
particular, because that one doesn't use ./configure).

**

I just tried zlib 1.2.6.1. Compilation fails very early, with the
following error:

zutil.h:254:24: fatal error: byteswap.h: No such file or directory

This happens because it is assumed that __builtin_bswap32 exists on
any gcc starting with gcc-4.3. It does not exist on MinGW, though.
Chances are that this is not the only platform where it fails.

In general, I would like to suggest not using GCC version numbers for
extensions like this, but instead use configuration macros like:
#ifdef HAVE__BUILTIN_BSWAP32
/* ... use __builtin_bswap32 */
#endif

This strategy will allow, on one hand, to make things not fail even if
the particular gcc version does not support this extension, and, on
the other hand, will allow other compilers to benefit from that. (For
example, clang has __builtin_bswap32 as well, so clang could benefit
from a non-gcc-specific approach as well.)

In addition, along these lines, I would like to suggest, if possible,
to replace NO_VIZ with HAVE__ATTRIBUTE__VISIBILITY, as follows:
replace
#if ... && !defined(NO_VIZ)
with
#if ... && defined(HAVE__ATTRIBUTE__VISIBILITY)

What's the difference? The answer is, it's better to have something
that works by default, and can be enhanced if the configure script can
detect the extension availability and define
HAVE_NONSTANDARD_FEATURE_FOO. What we're having right now is a
reversed mechanism, which breaks by default if the non-standard
feature "foo" is missing -- unless the configure script or
win32/makefile.gcc (or any other makefile) defines NO_FOO.

Sincerely,
Cosmin