windows consolidated cleanup
The attached patch is intended to clean up a bunch of compiler warnings
seen on Windows due to mismatches of signedness or constness, unused
variables, redefined macros and a missing prototype.
It doesn't clean up all the warnings by any means, but it fixes quite a few.
One thing I'm a bit confused about is this type of warning:
src\backend\utils\misc\guc-file.c(977): warning C4003: not enough actual parameters for macro 'GUC_yywrap'
If someone can suggest a good fix That would be nice.
cheers
andrew
Attachments:
w32cleanup.patchtext/x-patch; name=w32cleanup.patchDownload
*** a/src/backend/main/main.c
--- b/src/backend/main/main.c
***************
*** 393,399 **** get_current_username(const char *progname)
return strdup(pw->pw_name);
#else
long namesize = 256 /* UNLEN */ + 1;
! char *name;
name = malloc(namesize);
if (!GetUserName(name, &namesize))
--- 393,399 ----
return strdup(pw->pw_name);
#else
long namesize = 256 /* UNLEN */ + 1;
! unsigned char *name;
name = malloc(namesize);
if (!GetUserName(name, &namesize))
*** a/src/backend/port/win32/socket.c
--- b/src/backend/port/win32/socket.c
***************
*** 370,376 **** pgwin32_recv(SOCKET s, char *buf, int len, int f)
}
int
! pgwin32_send(SOCKET s, char *buf, int len, int flags)
{
WSABUF wbuf;
int r;
--- 370,376 ----
}
int
! pgwin32_send(SOCKET s, const char *buf, int len, int flags)
{
WSABUF wbuf;
int r;
***************
*** 380,386 **** pgwin32_send(SOCKET s, char *buf, int len, int flags)
return -1;
wbuf.len = len;
! wbuf.buf = buf;
/*
* Readiness of socket to send data to UDP socket may be not true: socket
--- 380,386 ----
return -1;
wbuf.len = len;
! wbuf.buf = (char *) buf;
/*
* Readiness of socket to send data to UDP socket may be not true: socket
*** a/src/backend/utils/adt/pg_locale.c
--- b/src/backend/utils/adt/pg_locale.c
***************
*** 63,69 ****
--- 63,78 ----
#include "utils/syscache.h"
#ifdef WIN32
+ /*
+ * This Windows file defines StrNCpy. We don't need it here, so we undefine
+ * it to keep the compiler quiet, and undefine it again after the file is
+ * included, so we don't accidentally use theirs.
+ */
+ #undef StrNCpy
#include <shlwapi.h>
+ #ifdef StrNCpy
+ #undef STrNCpy
+ #endif
#endif
#define MAX_L10N_DATA 80
***************
*** 555,561 **** strftime_win32(char *dst, size_t dstlen, const wchar_t *format, const struct tm
dst[len] = '\0';
if (encoding != PG_UTF8)
{
! char *convstr = pg_do_encoding_conversion(dst, len, PG_UTF8, encoding);
if (dst != convstr)
{
--- 564,571 ----
dst[len] = '\0';
if (encoding != PG_UTF8)
{
! char *convstr = pg_do_encoding_conversion((unsigned char *) dst,
! len, PG_UTF8, encoding);
if (dst != convstr)
{
*** a/src/bin/initdb/initdb.c
--- b/src/bin/initdb/initdb.c
***************
*** 1561,1567 **** normalize_locale_name(char *new, const char *old)
static void
setup_collation(void)
{
! #ifdef HAVE_LOCALE_T
int i;
FILE *locale_a_handle;
char localebuf[NAMEDATALEN];
--- 1561,1567 ----
static void
setup_collation(void)
{
! #if defined(HAVE_LOCALE_T) && !defined(WIN32)
int i;
FILE *locale_a_handle;
char localebuf[NAMEDATALEN];
***************
*** 1687,1696 **** setup_collation(void)
printf(_("No usable system locales were found.\n"));
printf(_("Use the option \"--debug\" to see details.\n"));
}
! #else /* not HAVE_LOCALE_T */
printf(_("not supported on this platform\n"));
fflush(stdout);
! #endif /* not HAVE_LOCALE_T */
}
/*
--- 1687,1696 ----
printf(_("No usable system locales were found.\n"));
printf(_("Use the option \"--debug\" to see details.\n"));
}
! #else /* not HAVE_LOCALE_T && not WIN32 */
printf(_("not supported on this platform\n"));
fflush(stdout);
! #endif /* not HAVE_LOCALE_T && not WIN32*/
}
/*
***************
*** 2387,2393 **** setlocales(void)
--- 2387,2396 ----
#ifdef WIN32
typedef BOOL (WINAPI * __CreateRestrictedToken) (HANDLE, DWORD, DWORD, PSID_AND_ATTRIBUTES, DWORD, PLUID_AND_ATTRIBUTES, DWORD, PSID_AND_ATTRIBUTES, PHANDLE);
+ /* Windows API define missing from some versions of MingW headers */
+ #ifndef DISABLE_MAX_PRIVILEGE
#define DISABLE_MAX_PRIVILEGE 0x1
+ #endif
/*
* Create a restricted token and execute the specified process with it.
*** a/src/bin/pg_ctl/pg_ctl.c
--- b/src/bin/pg_ctl/pg_ctl.c
***************
*** 1461,1468 **** typedef BOOL (WINAPI * __SetInformationJobObject) (HANDLE, JOBOBJECTINFOCLASS, L
typedef BOOL (WINAPI * __AssignProcessToJobObject) (HANDLE, HANDLE);
typedef BOOL (WINAPI * __QueryInformationJobObject) (HANDLE, JOBOBJECTINFOCLASS, LPVOID, DWORD, LPDWORD);
! /* Windows API define missing from MingW headers */
#define DISABLE_MAX_PRIVILEGE 0x1
/*
* Create a restricted token, a job object sandbox, and execute the specified
--- 1461,1470 ----
typedef BOOL (WINAPI * __AssignProcessToJobObject) (HANDLE, HANDLE);
typedef BOOL (WINAPI * __QueryInformationJobObject) (HANDLE, JOBOBJECTINFOCLASS, LPVOID, DWORD, LPDWORD);
! /* Windows API define missing from some versions of MingW headers */
! #ifndef DISABLE_MAX_PRIVILEGE
#define DISABLE_MAX_PRIVILEGE 0x1
+ #endif
/*
* Create a restricted token, a job object sandbox, and execute the specified
*** a/src/include/port/win32.h
--- b/src/include/port/win32.h
***************
*** 336,342 **** SOCKET pgwin32_accept(SOCKET s, struct sockaddr * addr, int *addrlen);
int pgwin32_connect(SOCKET s, const struct sockaddr * name, int namelen);
int pgwin32_select(int nfds, fd_set *readfs, fd_set *writefds, fd_set *exceptfds, const struct timeval * timeout);
int pgwin32_recv(SOCKET s, char *buf, int len, int flags);
! int pgwin32_send(SOCKET s, char *buf, int len, int flags);
const char *pgwin32_socket_strerror(int err);
int pgwin32_waitforsinglesocket(SOCKET s, int what, int timeout);
--- 336,342 ----
int pgwin32_connect(SOCKET s, const struct sockaddr * name, int namelen);
int pgwin32_select(int nfds, fd_set *readfs, fd_set *writefds, fd_set *exceptfds, const struct timeval * timeout);
int pgwin32_recv(SOCKET s, char *buf, int len, int flags);
! int pgwin32_send(SOCKET s, const char *buf, int len, int flags);
const char *pgwin32_socket_strerror(int err);
int pgwin32_waitforsinglesocket(SOCKET s, int what, int timeout);
*** a/src/pl/plperl/plperl.c
--- b/src/pl/plperl/plperl.c
***************
*** 3551,3557 **** hv_store_string(HV *hv, const char *key, SV *val)
* does not appear that hashes track UTF-8-ness of keys at all in Perl
* 5.6.
*/
! hlen = -strlen(hkey);
ret = hv_store(hv, hkey, hlen, val, 0);
if (hkey != key)
--- 3551,3557 ----
* does not appear that hashes track UTF-8-ness of keys at all in Perl
* 5.6.
*/
! hlen = - (int) strlen(hkey);
ret = hv_store(hv, hkey, hlen, val, 0);
if (hkey != key)
***************
*** 3576,3582 **** hv_fetch_string(HV *hv, const char *key)
GetDatabaseEncoding(), PG_UTF8);
/* See notes in hv_store_string */
! hlen = -strlen(hkey);
ret = hv_fetch(hv, hkey, hlen, 0);
if (hkey != key)
--- 3576,3582 ----
GetDatabaseEncoding(), PG_UTF8);
/* See notes in hv_store_string */
! hlen = - (int) strlen(hkey);
ret = hv_fetch(hv, hkey, hlen, 0);
if (hkey != key)
*** a/src/pl/plperl/plperl.h
--- b/src/pl/plperl/plperl.h
***************
*** 26,36 ****
--- 26,72 ----
#endif
#endif
+ /*
+ * Supply a value of PERL_UNUSED_DECL that will satisfy gcc - the one
+ * perl itself supplies doesn't seem to.
+ */
+ #if defined(__GNUC__)
+ #define PERL_UNUSED_DECL __attribute__ ((unused))
+ #endif
+
+ /*
+ * Sometimes perl carefully scribbles on our *printf macros.
+ * So we undefine them ehere and redefine them after it's done its dirty deed.
+ */
+
+ #ifdef USE_REPL_SNPRINTF
+ #undef snprintf
+ #undef vsnprintf
+ #endif
+
+
/* required for perl API */
#include "EXTERN.h"
#include "perl.h"
#include "XSUB.h"
+ /* put back our snprintf and vsnprintf */
+ #ifdef USE_REPL_SNPRINTF
+ #ifdef snprintf
+ #undef snprintf
+ #endif
+ #ifdef vnsprintf
+ #undef vsnprintf
+ #endif
+ #ifdef __GNUC__
+ #define vsnprintf(...) pg_vsnprintf(__VA_ARGS__)
+ #define snprintf(...) pg_snprintf(__VA_ARGS__)
+ #else
+ #define vsnprintf pg_vsnprintf
+ #define snprintf pg_snprintf
+ #endif /* __GNUC__ */
+ #endif /* USE_REPL_SNPRINTF */
+
/* perl version and platform portability */
#define NEED_eval_pv
#define NEED_newRV_noinc
*** a/src/pl/plpython/plpython.c
--- b/src/pl/plpython/plpython.c
***************
*** 84,89 **** typedef int Py_ssize_t;
--- 84,101 ----
PyObject_HEAD_INIT(type) size,
#endif
+ /*
+ * Some Python headers define these two symbols (e.g. on Windows) which is
+ * possibly a bit unfriendly. Use the Postgres definitions (or lack thereof).
+ */
+ #ifdef HAVE_STRERROR
+ #undef HAVE_STRERROR
+ #endif
+
+ #ifdef HAVE_TZNAME
+ #undef HAVE_TZNAME
+ #endif
+
#include "postgres.h"
/* system stuff */
*** a/src/port/getopt.c
--- b/src/port/getopt.c
***************
*** 61,66 **** extern char *optarg;
--- 61,68 ----
#define BADARG (int)':'
#define EMSG ""
+ int getopt(int nargc, char *const * nargv, const char * ostr);
+
/*
* getopt
* Parse argc/argv argument vector.
***************
*** 72,81 **** extern char *optarg;
* returning -1.)
*/
int
! getopt(nargc, nargv, ostr)
! int nargc;
! char *const * nargv;
! const char *ostr;
{
static char *place = EMSG; /* option letter processing */
char *oli; /* option letter list index */
--- 74,80 ----
* returning -1.)
*/
int
! getopt(int nargc, char *const * nargv, const char * ostr)
{
static char *place = EMSG; /* option letter processing */
char *oli; /* option letter list index */
*** a/src/port/noblock.c
--- b/src/port/noblock.c
***************
*** 23,29 **** pg_set_noblock(pgsocket sock)
#if !defined(WIN32)
return (fcntl(sock, F_SETFL, O_NONBLOCK) != -1);
#else
! long ioctlsocket_ret = 1;
/* Returns non-0 on failure, while fcntl() returns -1 on failure */
return (ioctlsocket(sock, FIONBIO, &ioctlsocket_ret) == 0);
--- 23,29 ----
#if !defined(WIN32)
return (fcntl(sock, F_SETFL, O_NONBLOCK) != -1);
#else
! unsigned long ioctlsocket_ret = 1;
/* Returns non-0 on failure, while fcntl() returns -1 on failure */
return (ioctlsocket(sock, FIONBIO, &ioctlsocket_ret) == 0);
***************
*** 42,48 **** pg_set_block(pgsocket sock)
return false;
return true;
#else
! long ioctlsocket_ret = 0;
/* Returns non-0 on failure, while fcntl() returns -1 on failure */
return (ioctlsocket(sock, FIONBIO, &ioctlsocket_ret) == 0);
--- 42,48 ----
return false;
return true;
#else
! unsigned long ioctlsocket_ret = 0;
/* Returns non-0 on failure, while fcntl() returns -1 on failure */
return (ioctlsocket(sock, FIONBIO, &ioctlsocket_ret) == 0);
*** a/src/test/regress/pg_regress.c
--- b/src/test/regress/pg_regress.c
***************
*** 140,148 **** __attribute__((format(printf, 2, 3)));
#ifdef WIN32
typedef BOOL (WINAPI * __CreateRestrictedToken) (HANDLE, DWORD, DWORD, PSID_AND_ATTRIBUTES, DWORD, PLUID_AND_ATTRIBUTES, DWORD, PSID_AND_ATTRIBUTES, PHANDLE);
! /* Windows API define missing from MingW headers */
#define DISABLE_MAX_PRIVILEGE 0x1
#endif
/*
* allow core files if possible.
--- 140,150 ----
#ifdef WIN32
typedef BOOL (WINAPI * __CreateRestrictedToken) (HANDLE, DWORD, DWORD, PSID_AND_ATTRIBUTES, DWORD, PLUID_AND_ATTRIBUTES, DWORD, PSID_AND_ATTRIBUTES, PHANDLE);
! /* Windows API define missing from some versions of MingW headers */
! #ifndef DISABLE_MAX_PRIVILEGE
#define DISABLE_MAX_PRIVILEGE 0x1
#endif
+ #endif
/*
* allow core files if possible.
*** a/src/timezone/pgtz.c
--- b/src/timezone/pgtz.c
***************
*** 1125,1131 **** identify_system_timezone(void)
for (idx = 0;; idx++)
{
char keyname[256];
! char zonename[256];
DWORD namesize;
FILETIME lastwrite;
HKEY key;
--- 1125,1131 ----
for (idx = 0;; idx++)
{
char keyname[256];
! unsigned char zonename[256];
DWORD namesize;
FILETIME lastwrite;
HKEY key;
On 4/24/2011 1:29 AM, Andrew Dunstan wrote:
The attached patch is intended to clean up a bunch of compiler warnings seen on
Windows due to mismatches of signedness or constness, unused variables,
redefined macros and a missing prototype.It doesn't clean up all the warnings by any means, but it fixes quite a few.
One thing I'm a bit confused about is this type of warning:
src\backend\utils\misc\guc-file.c(977): warning C4003: not enough actual
parameters for macro 'GUC_yywrap'If someone can suggest a good fix That would be nice.
The macro is defined as taking one argument.
// guc-file.c line 354
#define GUC_yywrap(n) 1
The macro is overriding the prototype declared at line 627, which has a void
argument list (assuming YY_SKIP_YYWRAP is !defined). Since all code references
to this do not provide an argument, I'd say the macro is incorrect.
--
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.com/
On sön, 2011-04-24 at 01:29 -0400, Andrew Dunstan wrote:
One thing I'm a bit confused about is this type of warning:
src\backend\utils\misc\guc-file.c(977): warning C4003: not enough actual parameters for macro 'GUC_yywrap'
If someone can suggest a good fix That would be nice.
Upstream claims that complaining about this is a bug in the compiler.
The hunk below looks a bit evil.
At least a comment would be good to explain why this is necessary.
Show quoted text
On sön, 2011-04-24 at 01:29 -0400, Andrew Dunstan wrote:
*** a/src/backend/port/win32/socket.c --- b/src/backend/port/win32/socket.c *************** *** 370,376 **** pgwin32_recv(SOCKET s, char *buf, int len, int f) }int ! pgwin32_send(SOCKET s, char *buf, int len, int flags) { WSABUF wbuf; int r; --- 370,376 ---- }int
! pgwin32_send(SOCKET s, const char *buf, int len, int flags)
{
WSABUF wbuf;
int r;
***************
*** 380,386 **** pgwin32_send(SOCKET s, char *buf, int len, int flags)
return -1;wbuf.len = len;
! wbuf.buf = buf;/* * Readiness of socket to send data to UDP socket may be not true: socket --- 380,386 ---- return -1;wbuf.len = len;
! wbuf.buf = (char *) buf;/*
* Readiness of socket to send data to UDP socket may be not true: socket
On 04/24/2011 09:11 AM, Andrew Chernow wrote:
On 4/24/2011 1:29 AM, Andrew Dunstan wrote:
The attached patch is intended to clean up a bunch of compiler
warnings seen on
Windows due to mismatches of signedness or constness, unused variables,
redefined macros and a missing prototype.It doesn't clean up all the warnings by any means, but it fixes quite
a few.One thing I'm a bit confused about is this type of warning:
src\backend\utils\misc\guc-file.c(977): warning C4003: not enough actual
parameters for macro 'GUC_yywrap'If someone can suggest a good fix That would be nice.
The macro is defined as taking one argument.
// guc-file.c line 354
#define GUC_yywrap(n) 1The macro is overriding the prototype declared at line 627, which has
a void argument list (assuming YY_SKIP_YYWRAP is !defined). Since all
code references to this do not provide an argument, I'd say the macro
is incorrect.
Thanks for looking.
All our scanners are in fact defined with "%option noyywrap", so
YY_SKIP_WRAP is defined.
But the macro is incorrect unless you're generating a reentrant scanner
as we are for the core scanner (which is why we don't see this error for
the core scanner, only all the others).
I have a mildly ugly fix for it in pg_flex.bat (only MSVC compilers
complain about it AFAIK).
cheers
andrew
On 04/24/2011 10:53 AM, Peter Eisentraut wrote:
On sön, 2011-04-24 at 01:29 -0400, Andrew Dunstan wrote:
One thing I'm a bit confused about is this type of warning:
src\backend\utils\misc\guc-file.c(977): warning C4003: not enough actual parameters for macro 'GUC_yywrap'
If someone can suggest a good fix That would be nice.
Upstream claims that complaining about this is a bug in the compiler.
One person said that. But the bug is still open, and has been for four
years (so you can see how much attention they pay to this tracker at least.)
cheers
andrew
Peter Eisentraut <peter_e@gmx.net> writes:
The hunk below looks a bit evil.
At least a comment would be good to explain why this is necessary.
Yeah, having to cast away const seems uglier than the original problem.
Can't we avoid that?
BTW, all of my machines as well as the Single Unix Spec are agreed that
the second argument to send() is "const void *", not "const char *".
If we're going to tweak this I think we should make it match exactly.
regards, tom lane
Andrew Dunstan <andrew@dunslane.net> writes:
The attached patch is intended to clean up a bunch of compiler warnings
seen on Windows due to mismatches of signedness or constness, unused
variables, redefined macros and a missing prototype.
BTW, this hunk:
*** a/src/pl/plpython/plpython.c --- b/src/pl/plpython/plpython.c *************** *** 84,89 **** typedef int Py_ssize_t; --- 84,101 ---- PyObject_HEAD_INIT(type) size, #endif
+ /* + * Some Python headers define these two symbols (e.g. on Windows) which is + * possibly a bit unfriendly. Use the Postgres definitions (or lack thereof). + */ + #ifdef HAVE_STRERROR + #undef HAVE_STRERROR + #endif + + #ifdef HAVE_TZNAME + #undef HAVE_TZNAME + #endif + #include "postgres.h"
/* system stuff */
is indicative of far worse problems than the one it claims to solve.
This file is in fundamental violation of the first commandment of
Postgres #includes, which is "thou shalt have no other gods before c.h".
We need to put postgres.h *before* the Python.h include. I don't know
what issues led to the current arrangement but it is fraught with
portability gotchas. In particular it's just about guaranteed to fail
on platforms where <stdio.h> reacts to _FILE_OFFSET_BITS --- plpython.c
is going to get compiled expecting a different stdio library than the
rest of the backend.
regards, tom lane
On 04/24/2011 12:25 PM, Tom Lane wrote:
Andrew Dunstan<andrew@dunslane.net> writes:
The attached patch is intended to clean up a bunch of compiler warnings
seen on Windows due to mismatches of signedness or constness, unused
variables, redefined macros and a missing prototype.BTW, this hunk:
[snip]
is indicative of far worse problems than the one it claims to solve.
This file is in fundamental violation of the first commandment of
Postgres #includes, which is "thou shalt have no other gods before c.h".
We need to put postgres.h *before* the Python.h include. I don't know
what issues led to the current arrangement but it is fraught with
portability gotchas. In particular it's just about guaranteed to fail
on platforms where<stdio.h> reacts to _FILE_OFFSET_BITS --- plpython.c
is going to get compiled expecting a different stdio library than the
rest of the backend.
Well, I certainly noticed that postgres.h wasn't first, but assumed it
had been sanctioned long ago. It's been that way for a long time.
I'll leave that bit out of this cleanup, but we should look at this
whole mess separately ASAP.
cheers
andrew
On 04/24/2011 12:14 PM, Tom Lane wrote:
Peter Eisentraut<peter_e@gmx.net> writes:
The hunk below looks a bit evil.
At least a comment would be good to explain why this is necessary.Yeah, having to cast away const seems uglier than the original problem.
Can't we avoid that?
I'm not sure how, since the second argument to send() is declared const,
and the buf member of a WSABUF isn't. Why is this worse? The compiler
warning is effectively telling us that the compiler will be discarding
constness anyway, isn't it?
BTW, all of my machines as well as the Single Unix Spec are agreed that
the second argument to send() is "const void *", not "const char *".
If we're going to tweak this I think we should make it match exactly.
I'm OK with that - not sure if it will generate *more* casts or
warnings, though.
cheers
andrew
On 04/24/2011 09:11 AM, Andrew Chernow wrote:
On 4/24/2011 1:29 AM, Andrew Dunstan wrote:
The attached patch is intended to clean up a bunch of compiler
warnings seen on
Windows due to mismatches of signedness or constness, unused variables,
redefined macros and a missing prototype.It doesn't clean up all the warnings by any means, but it fixes quite
a few.One thing I'm a bit confused about is this type of warning:
src\backend\utils\misc\guc-file.c(977): warning C4003: not enough actual
parameters for macro 'GUC_yywrap'If someone can suggest a good fix That would be nice.
The macro is defined as taking one argument.
// guc-file.c line 354
#define GUC_yywrap(n) 1The macro is overriding the prototype declared at line 627, which has
a void argument list (assuming YY_SKIP_YYWRAP is !defined). Since all
code references to this do not provide an argument, I'd say the macro
is incorrect.
Here's the fix that worked for me:
*** a/src/tools/msvc/pgflex.bat
--- b/src/tools/msvc/pgflex.bat
***************
*** 25,32 **** if "%1" == "contrib\seg\segscan.l" call :generate %1 contrib\seg\segscan.c
--- 25,38 ----
echo Unknown flex input: %1
exit 1 + REM for non-reentrant scanners we need to fix up yywrap definition
+ REM to keep the MS compiler happy
+ REM for reentrant scanners (like the core scanner) we do not
+ REM need to (and must not) change the yywrap definition
:generate
flex %3 -o%2 %1
+ if errorlevel 1 exit %errorlevel%
+ if not "%1" == "src\backend\parser\scan.l" perl -pi.bak -e "s/yywrap\(n\)/yywrap()/" %2
exit %errorlevel%
:noflexcheers
andrew
Andrew Dunstan <andrew@dunslane.net> writes:
On 04/24/2011 09:11 AM, Andrew Chernow wrote:
The macro is overriding the prototype declared at line 627, which has
a void argument list (assuming YY_SKIP_YYWRAP is !defined). Since all
code references to this do not provide an argument, I'd say the macro
is incorrect.
Here's the fix that worked for me:
Could we please make this conditional on seeing "%option reentrant" in
the .l file, instead of hard-wiring an assumption about which scanners
are re-entrant?
regards, tom lane
On sön, 2011-04-24 at 12:25 -0400, Tom Lane wrote:
This file is in fundamental violation of the first commandment of
Postgres #includes, which is "thou shalt have no other gods before c.h".
We need to put postgres.h *before* the Python.h include. I don't know
what issues led to the current arrangement but it is fraught with
portability gotchas. In particular it's just about guaranteed to fail
on platforms where <stdio.h> reacts to _FILE_OFFSET_BITS --- plpython.c
is going to get compiled expecting a different stdio library than the
rest of the backend.
Here is where this happened:
commit ab6ee1f9fc7039b1e8d8ebf939da3fd55e73efad
Author: Joe Conway <mail@joeconway.com>
Date: Thu Aug 5 03:10:29 2004 +0000
Move include for Python.h above postgres.h to eliminate compiler warning.
diff --git a/src/pl/plpython/plpython.c b/src/pl/plpython/plpython.c
index 76ea031..07eed86 100644
--- a/src/pl/plpython/plpython.c
+++ b/src/pl/plpython/plpython.c
@@ -29,11 +29,12 @@
* MAINTENANCE, SUPPORT, UPDATES, ENHANCEMENTS, OR MODIFICATIONS.
*
* IDENTIFICATION
- * $PostgreSQL: pgsql/src/pl/plpython/plpython.c,v 1.52 2004/08/04 21:34:29 tgl Exp $
+ * $PostgreSQL: pgsql/src/pl/plpython/plpython.c,v 1.53 2004/08/05 03:10:29 joe Exp $
*
*********************************************************************
*/+#include <Python.h>
#include "postgres.h"
/* system stuff */
@@ -54,7 +55,6 @@
#include "utils/syscache.h"
#include "utils/typcache.h"
-#include <Python.h>
#include <compile.h>
#include <eval.h>
If you switch it back around, you indeed get a bunch of annoying
warnings. This will need some playing around it get right.
On 04/24/2011 07:31 PM, Peter Eisentraut wrote:
On sön, 2011-04-24 at 12:25 -0400, Tom Lane wrote:
This file is in fundamental violation of the first commandment of
Postgres #includes, which is "thou shalt have no other gods before c.h".
We need to put postgres.h *before* the Python.h include. I don't know
what issues led to the current arrangement but it is fraught with
portability gotchas. In particular it's just about guaranteed to fail
on platforms where<stdio.h> reacts to _FILE_OFFSET_BITS --- plpython.c
is going to get compiled expecting a different stdio library than the
rest of the backend.Here is where this happened:
commit ab6ee1f9fc7039b1e8d8ebf939da3fd55e73efad
Author: Joe Conway<mail@joeconway.com>
Date: Thu Aug 5 03:10:29 2004 +0000Move include for Python.h above postgres.h to eliminate compiler warning.
diff --git a/src/pl/plpython/plpython.c b/src/pl/plpython/plpython.c index 76ea031..07eed86 100644 --- a/src/pl/plpython/plpython.c +++ b/src/pl/plpython/plpython.c @@ -29,11 +29,12 @@ * MAINTENANCE, SUPPORT, UPDATES, ENHANCEMENTS, OR MODIFICATIONS. * * IDENTIFICATION - * $PostgreSQL: pgsql/src/pl/plpython/plpython.c,v 1.52 2004/08/04 21:34:29 tgl Exp $ + * $PostgreSQL: pgsql/src/pl/plpython/plpython.c,v 1.53 2004/08/05 03:10:29 joe Exp $ * ********************************************************************* */+#include<Python.h>
#include "postgres.h"/* system stuff */
@@ -54,7 +55,6 @@
#include "utils/syscache.h"
#include "utils/typcache.h"-#include<Python.h>
#include<compile.h>
#include<eval.h>If you switch it back around, you indeed get a bunch of annoying
warnings. This will need some playing around it get right.
On my Linux system the attached compiles without warnings. If this seems
like the way to go I'll investigate more on Windows.
cheers
andrew
Attachments:
pythonh.patchtext/x-patch; name=pythonh.patchDownload
diff --git a/src/pl/plpython/plpython.c b/src/pl/plpython/plpython.c
index e03d7ce..cfe7f78 100644
--- a/src/pl/plpython/plpython.c
+++ b/src/pl/plpython/plpython.c
@@ -6,6 +6,21 @@
*********************************************************************
*/
+#include "postgres.h"
+
+/*
+ * Save settings the Python headers might override
+ */
+#ifdef _POSIX_C_SOURCE
+#define _PGSAVE_POSIX_C_SOURCE _POSIX_C_SOURCE
+#undef _POSIX_C_SOURCE
+#endif
+
+#ifdef _XOPEN_SOURCE
+#define _PGSAVE_XOPEN_SOURCE _XOPEN_SOURCE
+#undef _XOPEN_SOURCE
+#endif
+
#if defined(_MSC_VER) && defined(_DEBUG)
/* Python uses #pragma to bring in a non-default libpython on VC++ if
* _DEBUG is defined */
@@ -84,7 +99,20 @@ typedef int Py_ssize_t;
PyObject_HEAD_INIT(type) size,
#endif
-#include "postgres.h"
+/*
+ * Restore settings the Python headers might have overridden.
+ */
+#ifdef _PGSAVE_POSIX_C_SOURCE
+#undef _POSIX_C_SOURCE
+#define _POSIX_C_SOURCE _PGSAVE_POSIX_C_SOURCE
+#undef _PGSAVE_POSIX_C_SOURCE
+#endif
+
+#ifdef _PGSAVE_XOPEN_SOURCE
+#undef _XOPEN_SOURCE
+#define _XOPEN_SOURCE _PGSAVE_XOPEN_SOURCE
+#undef _PGSAVE_XOPEN_SOURCE
+#endif
/* system stuff */
#include <unistd.h>
Andrew Dunstan <andrew@dunslane.net> writes:
On my Linux system the attached compiles without warnings. If this seems
like the way to go I'll investigate more on Windows.
Hmm ...
+/* + * Save settings the Python headers might override + */ +#ifdef _POSIX_C_SOURCE +#define _PGSAVE_POSIX_C_SOURCE _POSIX_C_SOURCE +#undef _POSIX_C_SOURCE +#endif ... +/* + * Restore settings the Python headers might have overridden. + */ +#ifdef _PGSAVE_POSIX_C_SOURCE +#undef _POSIX_C_SOURCE +#define _POSIX_C_SOURCE _PGSAVE_POSIX_C_SOURCE +#undef _PGSAVE_POSIX_C_SOURCE +#endif
I don't believe that this sequence will restore the contents of the
_POSIX_C_SOURCE macro to what it was before. For that matter, it's
not even quite right about ensuring that the macro's defined-ness
status is restored (what if the python headers set _POSIX_C_SOURCE
when it wasn't set before?). We might not need more than defined-ness
to be right, though.
What in the world are the python headers doing fooling with these
macros, anyway??
regards, tom lane
On 07/24/2011 11:46 PM, Tom Lane wrote:
[python headers set _POSIX_C_SOURCE and _XOPEN_SOURCE]
What in the world are the python headers doing fooling with these
macros, anyway??
Good question. It seems unfriendly. It looks like you're just about guaranteed to get a warning if you include any system header before you include Python.h.
So either we have to dance around that or we have to give up the idea that postgres.h must come first. It wouldn't be the first time we've had to do that sort of dance.
The reason we get warnings about these and not about many other things it defines (such as the HAVE_foo macros) is that these are set to values different from those encountered in the previously included headers.
cheers
andrew
Andrew Dunstan <andrew@dunslane.net> writes:
On 07/24/2011 11:46 PM, Tom Lane wrote:
[python headers set _POSIX_C_SOURCE and _XOPEN_SOURCE]
What in the world are the python headers doing fooling with these
macros, anyway??
The reason we get warnings about these and not about many other things it defines (such as the HAVE_foo macros) is that these are set to values different from those encountered in the previously included headers.
That's pretty scary in itself, since it suggests that the Python guys
know or think that changing those values will do something magic.
I'm worried that they are trying to do the same kind of thing that
we are trying to do with our put-postgres.h-first rule, namely ensure
that all loadable modules match the core's idea of libc properties.
If that's what's going on here, and their idea of those properties
is different from our standard build, then we may have worse problems
than a compiler warning.
regards, tom lane
Andrew Dunstan <andrew@dunslane.net> writes:
[python headers set _POSIX_C_SOURCE and _XOPEN_SOURCE]
BTW ... so far as I can find, there is no attempt anywhere in the
Postgres sources to set either of these macros. And my understanding of
their purpose is that *system* headers should not be setting them at
all, rather the application sets them to indicate which POSIX feature
level it would like. So perhaps the real question here is where the
heck are your conflicting values coming from ...
regards, tom lane
On 07/25/2011 10:36 AM, Tom Lane wrote:
Andrew Dunstan<andrew@dunslane.net> writes:
[python headers set _POSIX_C_SOURCE and _XOPEN_SOURCE]
BTW ... so far as I can find, there is no attempt anywhere in the
Postgres sources to set either of these macros. And my understanding of
their purpose is that *system* headers should not be setting them at
all, rather the application sets them to indicate which POSIX feature
level it would like. So perhaps the real question here is where the
heck are your conflicting values coming from ...
_POSIX_C_SOURCE at least is defined in features.h, which is included by
huge numbers of system headers, many of which are included by c.h.
cheers
andrew
Andrew Dunstan <andrew@dunslane.net> writes:
On 07/25/2011 10:36 AM, Tom Lane wrote:
Andrew Dunstan<andrew@dunslane.net> writes:
[python headers set _POSIX_C_SOURCE and _XOPEN_SOURCE]
BTW ... so far as I can find, there is no attempt anywhere in the
Postgres sources to set either of these macros. And my understanding of
their purpose is that *system* headers should not be setting them at
all, rather the application sets them to indicate which POSIX feature
level it would like. So perhaps the real question here is where the
heck are your conflicting values coming from ...
_POSIX_C_SOURCE at least is defined in features.h, which is included by
huge numbers of system headers, many of which are included by c.h.
What is features.h, and have its authors read the POSIX standard?
AFAICS they have no business defining this symbol.
regards, tom lane
On 07/25/2011 10:52 AM, Tom Lane wrote:
Andrew Dunstan<andrew@dunslane.net> writes:
On 07/25/2011 10:36 AM, Tom Lane wrote:
Andrew Dunstan<andrew@dunslane.net> writes:
[python headers set _POSIX_C_SOURCE and _XOPEN_SOURCE]
BTW ... so far as I can find, there is no attempt anywhere in the
Postgres sources to set either of these macros. And my understanding of
their purpose is that *system* headers should not be setting them at
all, rather the application sets them to indicate which POSIX feature
level it would like. So perhaps the real question here is where the
heck are your conflicting values coming from ..._POSIX_C_SOURCE at least is defined in features.h, which is included by
huge numbers of system headers, many of which are included by c.h.What is features.h, and have its authors read the POSIX standard?
AFAICS they have no business defining this symbol.
[andrew@emma ~]$ rpm -q -f /usr/include/features.h
glibc-headers-2.13-1.x86_64
[andrew@emma ~]$ cat foo.c
#include <stdio.h>
#include <Python.h>
main() {}
[andrew@emma ~]$ gcc -I/usr/include/python2.7/ -c foo.c
In file included from /usr/include/python2.7/pyconfig.h:6:0,
from /usr/include/python2.7/Python.h:8,
from foo.c:2:
/usr/include/python2.7/pyconfig-64.h:1158:0: warning:
"_POSIX_C_SOURCE" redefined
/usr/include/features.h:214:0: note: this is the location of the
previous definition
See now?
cheers
andrew
Andrew Dunstan <andrew@dunslane.net> writes:
On 07/25/2011 10:52 AM, Tom Lane wrote:
What is features.h, and have its authors read the POSIX standard?
AFAICS they have no business defining this symbol.
[andrew@emma ~]$ rpm -q -f /usr/include/features.h
glibc-headers-2.13-1.x86_64
Oh, for some reason I was thinking this was mingw-specific.
[ pokes around ... ] I still think it's a bad idea for the header
files to be defining this, but they'll probably point at the part
of the POSIX spec that says the results are undefined if the macro
is changed after the first system header is #included.
I can't immediately think of any way to actually do what you were
trying to do (ie, save and restore the definition of the macro).
I wonder whether it would be good enough to do this:
#include postgres.h
#include everything else we want except python headers
#undef _POSIX_C_SOURCE
#undef _XOPEN_SOURCE
#include python headers
... rest of .c file ...
This should only fail if (a) some macro imported from system headers
attempts to test the value of a feature macro, and (b) the results
vary between the system default setting and the setting the python
headers selected. Neither of these things seem very probable.
regards, tom lane
On 07/25/2011 12:03 PM, Tom Lane wrote:
Andrew Dunstan<andrew@dunslane.net> writes:
On 07/25/2011 10:52 AM, Tom Lane wrote:
What is features.h, and have its authors read the POSIX standard?
AFAICS they have no business defining this symbol.[andrew@emma ~]$ rpm -q -f /usr/include/features.h
glibc-headers-2.13-1.x86_64Oh, for some reason I was thinking this was mingw-specific.
[ pokes around ... ] I still think it's a bad idea for the header
files to be defining this, but they'll probably point at the part
of the POSIX spec that says the results are undefined if the macro
is changed after the first system header is #included.I can't immediately think of any way to actually do what you were
trying to do (ie, save and restore the definition of the macro).
I wonder whether it would be good enough to do this:#include postgres.h
#include everything else we want except python headers
#undef _POSIX_C_SOURCE
#undef _XOPEN_SOURCE#include python headers
... rest of .c file ...
This should only fail if (a) some macro imported from system headers
attempts to test the value of a feature macro, and (b) the results
vary between the system default setting and the setting the python
headers selected. Neither of these things seem very probable.
OK, attached gives a clean build and passes regression on my Windows box
that builds with Python. I had to undefine a few more things and save
and restore our *snprintf settings (with code borrowed from plperl.h,
where we did this sort of cleanup a while ago).
cheers
andrew
Attachments:
plpython.patchtext/x-patch; name=plpython.patchDownload
diff --git a/src/pl/plpython/plpython.c b/src/pl/plpython/plpython.c
index e03d7ce..da34a17 100644
--- a/src/pl/plpython/plpython.c
+++ b/src/pl/plpython/plpython.c
@@ -6,6 +6,56 @@
*********************************************************************
*/
+#include "postgres.h"
+
+/* system stuff */
+#include <unistd.h>
+#include <fcntl.h>
+
+/* postgreSQL stuff */
+#include "catalog/pg_proc.h"
+#include "catalog/pg_type.h"
+#include "commands/trigger.h"
+#include "executor/spi.h"
+#include "funcapi.h"
+#include "fmgr.h"
+#include "mb/pg_wchar.h"
+#include "miscadmin.h"
+#include "nodes/makefuncs.h"
+#include "parser/parse_type.h"
+#include "tcop/tcopprot.h"
+#include "access/transam.h"
+#include "access/xact.h"
+#include "utils/builtins.h"
+#include "utils/hsearch.h"
+#include "utils/lsyscache.h"
+#include "utils/memutils.h"
+#include "utils/rel.h"
+#include "utils/syscache.h"
+#include "utils/typcache.h"
+
+/*
+ * Undefine some things that get (re)defined in the
+ * Python headers. They aren't used below and we've
+ * already included all the headers we need, so this
+ * should be pretty safe.
+ */
+
+#undef _POSIX_C_SOURCE
+#undef _XOPEN_SOURCE
+#undef HAVE_STRERROR
+#undef HAVE_TZNAME
+
+/*
+ * Sometimes python carefully scribbles on our *printf macros.
+ * So we undefine them here and redefine them after it's done its dirty deed.
+ */
+
+#ifdef USE_REPL_SNPRINTF
+#undef snprintf
+#undef vsnprintf
+#endif
+
#if defined(_MSC_VER) && defined(_DEBUG)
/* Python uses #pragma to bring in a non-default libpython on VC++ if
* _DEBUG is defined */
@@ -84,34 +134,6 @@ typedef int Py_ssize_t;
PyObject_HEAD_INIT(type) size,
#endif
-#include "postgres.h"
-
-/* system stuff */
-#include <unistd.h>
-#include <fcntl.h>
-
-/* postgreSQL stuff */
-#include "catalog/pg_proc.h"
-#include "catalog/pg_type.h"
-#include "commands/trigger.h"
-#include "executor/spi.h"
-#include "funcapi.h"
-#include "fmgr.h"
-#include "mb/pg_wchar.h"
-#include "miscadmin.h"
-#include "nodes/makefuncs.h"
-#include "parser/parse_type.h"
-#include "tcop/tcopprot.h"
-#include "access/transam.h"
-#include "access/xact.h"
-#include "utils/builtins.h"
-#include "utils/hsearch.h"
-#include "utils/lsyscache.h"
-#include "utils/memutils.h"
-#include "utils/rel.h"
-#include "utils/syscache.h"
-#include "utils/typcache.h"
-
/* define our text domain for translations */
#undef TEXTDOMAIN
#define TEXTDOMAIN PG_TEXTDOMAIN("plpython")
@@ -119,6 +141,23 @@ typedef int Py_ssize_t;
#include <compile.h>
#include <eval.h>
+/* put back our snprintf and vsnprintf */
+#ifdef USE_REPL_SNPRINTF
+#ifdef snprintf
+#undef snprintf
+#endif
+#ifdef vsnprintf
+#undef vsnprintf
+#endif
+#ifdef __GNUC__
+#define vsnprintf(...) pg_vsnprintf(__VA_ARGS__)
+#define snprintf(...) pg_snprintf(__VA_ARGS__)
+#else
+#define vsnprintf pg_vsnprintf
+#define snprintf pg_snprintf
+#endif /* __GNUC__ */
+#endif /* USE_REPL_SNPRINTF */
+
PG_MODULE_MAGIC;
/* convert Postgresql Datum or tuple into a PyObject.