Hi,

>>> For the win32.h, I really don't understand why _WINSOCKAPI_ was defined 
>>> before
>>> <winsock2.h>
>>> some google suggests that defining _WINSOCKAPI_ before<windows.h>  prevents
>>> inclusion of winsock.h but that does not have relation to inclusion of
>>> <winsock2.h>  and if<winsock2.h>  is included first, it should be ok.
>>> 
>>> If this guess is right, perhaps it could be better to remove the three 
>>> lines.
>>> #if !defined(WIN64) || defined(WIN32_ONLY_COMPILER)
>>> #define _WINSOCKAPI_
>>> #endif
> 
> No, this broke some compilers, IIRC (probably the native mingw compiler, 
> which is in use by several buildfarm members). Getting this right was very 
> tricky and time-consuming when I was adding support for the 64 bit mingw-w64 
> compiler, and there were a couple of rounds of breakage.
> 
> I'm therefore much more inclined to go the way of your earlier patch, which 
> seems much less risky.


I agree that original patch could be less risky. 
However, it doesn't match what Microsoft says:
http://msdn.microsoft.com/en-us/library/windows/desktop/ms737629%28v=vs.85%29.aspx

So, I think the standard way is not defining _WINSOCKAPI_, and if any compiler 
requires that,
which I think unlikely, then it should be defined as a exceptional case.
At least, native mingw GCC 4.5.2 (20110802 catalogue) and
4.6.1 (latest catalogue) does compile happily without the three lines.

>> I only changed this for consistency. For me, it works without that define in 
>> all test
>> environments, too.
>> 
>>> +/* __MINGW64_VERSION_MAJOR is related to both 32/64 bit gcc compiles by
>>> + * mingw-w64, however it gots defined only after
>>> Why not use __MINGW32__, which is defined without including any headers?
> 
> Because it's defined by other than mingw-w64 compilers.

I see. That's because mingw (not -w64). 
Should it be ok if mingw is ok with that condition?

> We have a bunch of compilers to support here. There are LOTS of compiler 
> scenarios on Windows (several versions of MSVC, 32bit and 64bit mingw-w64, 
> native mingw gcc, and a couple of Cygwin based compilers), and keeping track 
> of them all and making sure they don't break can be a pain.


Yes, that really is a pain.

The code block
#if _MSC_VER >= 1400 || defined(WIN64)
#define errcode __msvc_errcode
#include <crtdefs.h>
#undef errcode
#endif

looks as if there is no real need to crtdefs.h but, they wanted to prevent
definition of errcode and therefor put to the first place.

So, I was afraid moving the include downwards might break by 
by including a header that internally includes crtdefs.h.
If this is not problematic for MSVC (perhaps you know better on that),  
I have no objection in moving the order.


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to