Обсуждение: 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
Вложения
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