Fix compiler warnings on 64-bit Windows

Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:

GCC reports various instances of
warning: cast to pointer from integer of different size
[-Wint-to-pointer-cast]
warning: cast from pointer to integer of different size
[-Wpointer-to-int-cast]
in ECPG test files. This is because void* and long are cast back and
forth, but on 64-bit Windows, these have different sizes. Fix by
using intptr_t instead.

Hm. Silencing the warnings is a laudable goal, but I'm very dubious
of allowing these test files to depend on pg_config.h. That doesn't
correspond to real-world ECPG usage, so it seems likely that it could
come back to bite us some day.

According to C99 and POSIX, intptr_t should be provided by <stdint.h> ...
now that we're requiring C99, can we get away with just #include'ing
that directly in these test files?

regards, tom lane

On 2020-02-13 16:19, Tom Lane wrote:

According to C99 and POSIX, intptr_t should be provided by <stdint.h> ...
now that we're requiring C99, can we get away with just #include'ing
that directly in these test files?

I think in the past we were worried about the C library not being fully
C99. But the build farm indicates that even the trailing edge OS X and
HP-UX members have it, so I'm content to require it. Then we should
probably remove the Autoconf tests altogether.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:

On 2020-02-13 16:19, Tom Lane wrote:

According to C99 and POSIX, intptr_t should be provided by <stdint.h> ...
now that we're requiring C99, can we get away with just #include'ing
that directly in these test files?

I think in the past we were worried about the C library not being fully
C99. But the build farm indicates that even the trailing edge OS X and
HP-UX members have it, so I'm content to require it. Then we should
probably remove the Autoconf tests altogether.

Yeah, I think that the C99 requirement has obsoleted a number of configure
tests and related hackery in c.h. We just haven't got round to cleaning
that up yet.

BTW: I'm still concerned about the possibility of the C library being
less than C99. The model that was popular back then, and which still
exists on e.g. gaur, was that you could install a C99 *compiler* on
a pre-C99 system, and the compiler would bring its own standard header
files as necessary. While I don't have the machine booted up to check,
I'm pretty sure that gaur's <stdint.h> is being supplied by the gcc
installation not directly from /usr/include. On the other hand, that
compiler installation is still dependent on the vendor-supplied libc.

So the sorts of tests I think we can get away with removing have to do
with the presence of C99-required headers, macros, typedefs, etc.
Anything that is checking the presence or behavior of code in libc,
we probably need to be more careful about.

regards, tom lane

On 2020-02-14 15:52, Tom Lane wrote:

Yeah, I think that the C99 requirement has obsoleted a number of configure
tests and related hackery in c.h. We just haven't got round to cleaning
that up yet.

BTW: I'm still concerned about the possibility of the C library being
less than C99. The model that was popular back then, and which still
exists on e.g. gaur, was that you could install a C99 *compiler* on
a pre-C99 system, and the compiler would bring its own standard header
files as necessary. While I don't have the machine booted up to check,
I'm pretty sure that gaur's <stdint.h> is being supplied by the gcc
installation not directly from /usr/include. On the other hand, that
compiler installation is still dependent on the vendor-supplied libc.

Yeah, stdint.h belongs to the compiler, whereas intttypes.h belongs to
the C library. So if we require a C99 compiler we can get rid of all
tests and workarounds for stdint.h missing. Patch attached.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:

On 2020-02-14 15:52, Tom Lane wrote:

BTW: I'm still concerned about the possibility of the C library being
less than C99. The model that was popular back then, and which still
exists on e.g. gaur, was that you could install a C99 *compiler* on
a pre-C99 system, and the compiler would bring its own standard header
files as necessary. While I don't have the machine booted up to check,
I'm pretty sure that gaur's <stdint.h> is being supplied by the gcc
installation not directly from /usr/include. On the other hand, that
compiler installation is still dependent on the vendor-supplied libc.

Yeah, stdint.h belongs to the compiler, whereas intttypes.h belongs to
the C library. So if we require a C99 compiler we can get rid of all
tests and workarounds for stdint.h missing. Patch attached.

I tried this on gaur's host, and got:

$ make -s
In file included from ../../src/include/postgres_fe.h:25,
from base64.c:18:
../../src/include/c.h:67:20: stdint.h: No such file or directory
make[2]: *** [base64.o] Error 1
make[1]: *** [all-common-recurse] Error 2
make: *** [all-src-recurse] Error 2

Ooops. Poking around, it looks like this version of gcc has brought its
own stdbool.h, but not stdint.h:

$ ls /usr/include/std*
/usr/include/std_space.h /usr/include/stdio.h
/usr/include/stdarg.h /usr/include/stdlib.h
/usr/include/stddef.h
$ find /opt/gcc-3.4.6 -name 'std*.h'
/opt/gcc-3.4.6/lib/gcc/hppa2.0-hp-hpux10.20/3.4.6/include/stdarg.h
/opt/gcc-3.4.6/lib/gcc/hppa2.0-hp-hpux10.20/3.4.6/include/stdbool.h
/opt/gcc-3.4.6/lib/gcc/hppa2.0-hp-hpux10.20/3.4.6/include/stddef.h
/opt/gcc-3.4.6/lib/gcc/hppa2.0-hp-hpux10.20/3.4.6/include/stdio.h
/opt/gcc-3.4.6/lib/gcc/hppa2.0-hp-hpux10.20/3.4.6/include/stdlib.h
/opt/gcc-3.4.6/lib/gcc/hppa2.0-hp-hpux10.20/3.4.6/install-tools/include/stdarg.h
/opt/gcc-3.4.6/lib/gcc/hppa2.0-hp-hpux10.20/3.4.6/install-tools/include/stdbool.h
/opt/gcc-3.4.6/lib/gcc/hppa2.0-hp-hpux10.20/3.4.6/install-tools/include/stddef.h

Kind of annoying. Perhaps more recent gcc versions fixed that?
Anyway, this seems like a bit of a blocker for this idea, at least
unless I update or retire this buildfarm critter.

regards, tom lane

I wrote:

Ooops. Poking around, it looks like this version of gcc has brought its
own stdbool.h, but not stdint.h:
...
Kind of annoying. Perhaps more recent gcc versions fixed that?

Here we go, in the gcc 4.5.x release notes:

GCC now ensures that a C99-conforming <stdint.h> is present on most
targets, and uses information about the types in this header to
implement the Fortran bindings to those types. GCC does not ensure the
presence of such a header, and does not implement the Fortran
bindings, on the following targets: NetBSD, VxWorks, VMS, SymbianOS,
WinCE, LynxOS, Netware, QNX, Interix, TPF.

4.5 seems annoyingly recent for this purpose (barely 10 years old).
Also, I'd previously tried and failed to use 4.2.4 and 4.0.4 on that
platform --- they didn't seem to be able to cope with the old header
files. (Now I wonder if the lack of stdint.h had something to do
with it... although those versions did build, they just were buggy.)

Anyway, I'll have a go at updating gaur to use 4.5.x. There is a
sane-looking stdint.h on my second-oldest dinosaur, prairiedog.
Don't know about the situation on Windows, though. We might want
to take a close look at NetBSD, too, based on the GCC notes.

regards, tom lane

On Mon, Feb 17, 2020 at 4:52 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:

Anyway, I'll have a go at updating gaur to use 4.5.x. There is a
sane-looking stdint.h on my second-oldest dinosaur, prairiedog.
Don't know about the situation on Windows, though. We might want
to take a close look at NetBSD, too, based on the GCC notes.

As for Windows, stdint.h was included in VS2010, and currently Postgres
supports VS2013 to 2019.

Regards

=?UTF-8?Q?Juan_Jos=C3=A9_Santamar=C3=ADa_Flecha?= <juanjo.santamaria@gmail.com> writes:

On Mon, Feb 17, 2020 at 4:52 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:

Anyway, I'll have a go at updating gaur to use 4.5.x. There is a
sane-looking stdint.h on my second-oldest dinosaur, prairiedog.
Don't know about the situation on Windows, though. We might want
to take a close look at NetBSD, too, based on the GCC notes.

As for Windows, stdint.h was included in VS2010, and currently Postgres
supports VS2013 to 2019.

I've now updated gaur to gcc 4.5.4 (took a little more hair-pulling
than I would have wished). I confirm that 0001-Require-stdint.h.patch
works in that environment, so I think you can go ahead and push it.

I think there is room for more extensive trimming of no-longer-useful
configure checks, but I'll start a separate thread about that.

regards, tom lane

On 2020-02-20 17:24, Tom Lane wrote:

=?UTF-8?Q?Juan_Jos=C3=A9_Santamar=C3=ADa_Flecha?= <juanjo.santamaria@gmail.com> writes:

On Mon, Feb 17, 2020 at 4:52 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:

Anyway, I'll have a go at updating gaur to use 4.5.x. There is a
sane-looking stdint.h on my second-oldest dinosaur, prairiedog.
Don't know about the situation on Windows, though. We might want
to take a close look at NetBSD, too, based on the GCC notes.

As for Windows, stdint.h was included in VS2010, and currently Postgres
supports VS2013 to 2019.

I've now updated gaur to gcc 4.5.4 (took a little more hair-pulling
than I would have wished). I confirm that 0001-Require-stdint.h.patch
works in that environment, so I think you can go ahead and push it.

Done, and also the appropriately reworked Windows warnings patch from
the beginning of the thread.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services