Re: APR 1.0 released

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/