Horton

Thread

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Move include for Python.h above postgres.h to eliminate compiler warning.

  1. windows consolidated cleanup

    Andrew Dunstan <andrew@dunslane.net> — 2011-04-24T05:29:42Z

    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

  2. Re: windows consolidated cleanup

    Andrew Chernow <ac@esilo.com> — 2011-04-24T13:11:41Z

    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/

  3. Re: windows consolidated cleanup

    Peter Eisentraut <peter_e@gmx.net> — 2011-04-24T14:53:34Z

    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.

http://sourceforge.net/tracker/index.php?func=detail&aid=1783536&group_id=97492&atid=618177

  4. Re: windows consolidated cleanup

    Peter Eisentraut <peter_e@gmx.net> — 2011-04-24T14:57:59Z

    The hunk below looks a bit evil.

At least a comment would be good to explain why this is necessary.

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

  5. Re: windows consolidated cleanup

    Andrew Dunstan <andrew@dunslane.net> — 2011-04-24T15:10:56Z

    
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) 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.

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

  6. Re: windows consolidated cleanup

    Andrew Dunstan <andrew@dunslane.net> — 2011-04-24T15:47:08Z

    
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.
>
> http://sourceforge.net/tracker/index.php?func=detail&aid=1783536&group_id=97492&atid=618177
>

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

  7. Re: windows consolidated cleanup

    Tom Lane <tgl@sss.pgh.pa.us> — 2011-04-24T16:14:16Z

    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

  8. Re: windows consolidated cleanup

    Tom Lane <tgl@sss.pgh.pa.us> — 2011-04-24T16:25:29Z

    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

  9. Re: windows consolidated cleanup

    Andrew Dunstan <andrew@dunslane.net> — 2011-04-24T17:35:06Z

    
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

  10. Re: windows consolidated cleanup

    Andrew Dunstan <andrew@dunslane.net> — 2011-04-24T17:47:34Z

    
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

  11. Re: windows consolidated cleanup

    Andrew Dunstan <andrew@dunslane.net> — 2011-04-24T19:40:25Z

    
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) 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.


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% 
       :noflex


cheers

andrew

  12. Re: windows consolidated cleanup

    Tom Lane <tgl@sss.pgh.pa.us> — 2011-04-24T21:10:16Z

    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

  13. Re: windows consolidated cleanup

    Peter Eisentraut <peter_e@gmx.net> — 2011-04-24T23:31:53Z

    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.

  14. python cleanup

    Andrew Dunstan <andrew@dunslane.net> — 2011-07-24T23:46:31Z

    
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 +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 my Linux system the attached compiles without warnings. If this seems 
like the way to go I'll investigate more on Windows.

cheers

andrew

  15. Re: python cleanup

    Tom Lane <tgl@sss.pgh.pa.us> — 2011-07-25T03:46:33Z

    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

  16. Re: python cleanup

    Andrew Dunstan <andrew@dunslane.net> — 2011-07-25T14:18:10Z

    
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

  17. Re: python cleanup

    Tom Lane <tgl@sss.pgh.pa.us> — 2011-07-25T14:30:52Z

    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

  18. Re: python cleanup

    Tom Lane <tgl@sss.pgh.pa.us> — 2011-07-25T14:36:01Z

    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

  19. Re: python cleanup

    Andrew Dunstan <andrew@dunslane.net> — 2011-07-25T14:40:41Z

    
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

  20. Re: python cleanup

    Tom Lane <tgl@sss.pgh.pa.us> — 2011-07-25T14:52:43Z

    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

  21. Re: python cleanup

    Andrew Dunstan <andrew@dunslane.net> — 2011-07-25T15:09:16Z

    
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

  22. Re: python cleanup

    Tom Lane <tgl@sss.pgh.pa.us> — 2011-07-25T16:03:19Z

    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

  23. Re: python cleanup

    Andrew Dunstan <andrew@dunslane.net> — 2011-07-31T23:28:16Z

    
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_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.


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