Re: APR 1.0 released - Mailing list pgsql-hackers

Bruce Momjian wrote:

>Andrew Dunstan wrote:
>  
>
>>
>>I'm not sure exactly what Bruce checked, so I just spent a few cycles 
>>making sure that we did not inadvertantly pick up a define of WIN32 from 
>>windows.h anywhere else. I *think* we are OK on that. However, ISTM this 
>>is a foot just waiting to be shot - in retrospect using WIN32 as our 
>>marker for native Windows, which we do in a great many places (around 
>>300 by my count) was a less than stellar choice, given that it is 
>>defined by windows.h, and especially since we use that header for Cygwin 
>>as well as for Windows native in a few places.
>>    
>>
>
>The use of WIN32 was because it usually does mean MinGW and Cygwin. 
>

But it doesn't. On MinGW WIN32 is a builtin compiler-defined value, and 
on Cygwin it isn't. To see this, do:
 touch empty.c; cpp -dM empty.c | grep WIN32

WIN32 *is* defined by windows.h, but in most cases we only include it if 
WIN32 is *already* defined. windows.h is included unconditionally in our 
win32.h, but again in most cases we only include that if WIN32 is 
already defined. So in most cases where we use it it isn't for Cygwin. 
But there are a few system include files on Cygwin that include it, so 
it's not guaranteed, although I don't think those affect us.



> We
>had lots of Cygwin-specific defines in there already so Win32 just means
>both Mingw and Cygwin.  You will see only a few cases where we want
>Mingw and not Cygwin, but in those case we often also want MSVC and
>Borland, so it really is WIN32 && ! __CYGWIN__.  We do have one or two
>tests for __MINGW32__ where we really do want just that.
>
>Would you look around and see if this can be improved.  I can't see any.
>
>  
>

As I said, I did look at all the include cases. That was based on the 
assumption that we actually wanted what I thought was the intention, 
namely that WIN32 was for Windows native only. If that's not the case we 
would need to review every one of the ~300 cases where WIN32 is used in 
#ifdef and friends.

Bottom line - this is something of a mess. If we can make sure Cygwin 
isn't broken, we can probably live with what have for now. Personally, I 
would have configure work out something cleaner, like, say, defining 
WINDOWS_ALL for both Windows native and Cygwin. Then we could use that 
for cases meant to cover both, and __CYGWIN__  and  __MINGW32__  for the 
specific cases, without worrying what the compiler and/or the system 
header files might have defined for us.

cheers

andrew