Re: APR 1.0 released - Mailing list pgsql-hackers

Andrew Dunstan schrieb:
> 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.

Most of the ~300 cases are ok for CYGWIN. And probably for MINGW also.
But I don't do MINGW countertests. I assume you do :)

Just palloc misses some pending fixes for CYGWIN. cvs head didn't has 
this fixed.
I'll come with a new patch to cvs HEAD soon.
I'm quite busy with apache and php porting also.
And I want to be careful not to break the FRONTEND section.

At least beta2 needed this patch:
--- postgresql-8.0.0beta2/src/include/utils/palloc.h.orig    2004-08-29 
05:13:11.000000000 +0100
+++ postgresql-8.0.0beta2/src/include/utils/palloc.h    2004-09-03 
14:03:50.279562100 +0100
@@ -80,7 +80,7 @@
 #define pstrdup(str)  MemoryContextStrdup(CurrentMemoryContext, (str))

-#ifdef WIN32
+#if defined(WIN32) || defined(__CYGWIN__) extern void *pgport_palloc(Size sz); extern char *pgport_pstrdup(const char
*str);extern void pgport_pfree(void *pointer);
 


-- 
Reini Urban
http://xarch.tu-graz.ac.at/home/rurban/