Thread: Fix compiler warnings on 64-bit Windows
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]
and MSVC equivalently
warning C4312: 'type cast': conversion from 'int' to 'void *' of
greater size
warning C4311: 'type cast': pointer truncation from 'void *' to 'long'
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.
The code actually worked fine because the integer values in use are
all small. So this is just to get the test code to compile warning-free.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachment
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
Attachment
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