Horton

Thread

Commits

  1. Fix compiler warnings on 64-bit Windows

  2. Require stdint.h

  1. Fix compiler warnings on 64-bit Windows

    Peter Eisentraut <peter.eisentraut@2ndquadrant.com> — 2020-02-13T14:16:57Z

    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

  2. Re: Fix compiler warnings on 64-bit Windows

    Tom Lane <tgl@sss.pgh.pa.us> — 2020-02-13T15:19:50Z

    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

  3. Re: Fix compiler warnings on 64-bit Windows

    Peter Eisentraut <peter.eisentraut@2ndquadrant.com> — 2020-02-14T09:15:27Z

    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

  4. Re: Fix compiler warnings on 64-bit Windows

    Tom Lane <tgl@sss.pgh.pa.us> — 2020-02-14T14:52:10Z

    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

  5. Re: Fix compiler warnings on 64-bit Windows

    Peter Eisentraut <peter.eisentraut@2ndquadrant.com> — 2020-02-17T08:44:20Z

    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

  6. Re: Fix compiler warnings on 64-bit Windows

    Tom Lane <tgl@sss.pgh.pa.us> — 2020-02-17T14:31:29Z

    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

  7. Re: Fix compiler warnings on 64-bit Windows

    Tom Lane <tgl@sss.pgh.pa.us> — 2020-02-17T15:52:13Z

    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

  8. Re: Fix compiler warnings on 64-bit Windows

    Juan José Santamaría Flecha <juanjo.santamaria@gmail.com> — 2020-02-17T17:24:30Z

    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

  9. Re: Fix compiler warnings on 64-bit Windows

    Tom Lane <tgl@sss.pgh.pa.us> — 2020-02-20T16:24:34Z

    =?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

  10. Re: Fix compiler warnings on 64-bit Windows

    Peter Eisentraut <peter.eisentraut@2ndquadrant.com> — 2020-02-21T19:10:02Z

    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