Re: [HACKERS] Fix Windows socket error checking for MinGW

2013-08-22 Thread Michael Cronenworth 

On 08/21/2013 10:06 PM, Noah Misch wrote:

I concur, but our field experience doing it this way lessens my concern.


I see this change has hit master. I've pulled in the new patch for the 
Fedora MinGW package.


Thanks,
Michael


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

Re: [HACKERS] Fix Windows socket error checking for MinGW

2013-08-21 Thread Noah Misch 
On Mon, Aug 19, 2013 at 08:46:11AM -0500, Michael Cronenworth wrote:
 On 08/17/2013 12:16 AM, Noah Misch wrote:
  1. Redefine those constants for more (all?) compilers.
  2. Remove that block and put #ifdef around all usage of such constants in
  frontend code, as you have done.
  3. Remove that block and make src/backend/port/win32/socket.c 
  frontend-usable,
  so frontend code can treat errno like backend code treats errno.
  
  What do you recommend?
 
 Option 1 is dangerous. I'd rather let the environments keep their constants.

I concur, but our field experience doing it this way lessens my concern.

 Option 2 is the least dangerous but it adds lines of code.

More than that, as Robert explained, we seek to _isolate_ Windows-specific
code.  That's true even when a better-isolated approach takes more lines.

 Option 3: The errno variable is not set in Windows so relying on it is not 
 possible.

Look at src/backend/port/win32/socket.c; it emulates POSIX socket interfaces
on Windows, and that emulation includes setting errno.  I'd favor option 3 if
we weren't already successfully using option 1 for Visual Studio 2010, the
compiler of official 9.2 and 9.3 binaries.  Since we have been doing that, I
figure changing to option 3 now is riskier than staying the course.

nm

-- 
Noah Misch
EnterpriseDB http://www.enterprisedb.com


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

Re: [HACKERS] Fix Windows socket error checking for MinGW

2013-08-20 Thread Andrew Dunstan 


On 08/19/2013 11:36 PM, Michael Cronenworth wrote:

On 08/19/2013 07:35 PM, Noah Misch wrote:

That was option #1.  (You weren't planning to change just the one symbol
causing the failure at hand, were you?)  Reasonable choice.  The 
point in the
code comment quoted above looks bad, but the lack of reports of that 
nature
against official 9.2 binaries corroborates it having not been a 
problem yet.
The only non-socket use I see for the constants in question is the 
EINTR test

in XLogWrite(), which probably can't happen on Windows.


Redefining compiler constants is bad bandaid. A similar bandaid was in 
place to begin with caused this problem. If you believe PostgreSQL 
will never use code that needs the true errno value then go ahead with 
Option 1.



No the reverse is the case. The real problem is that the bandaid was not 
applied sufficiently widely. What we propose is exactly what is already 
in use for the Microsoft compilers, and has thus been well and truly 
tested over some years.


Changing these definitions doesn't change the value of errno in the 
slightest - it only changes the values that we test for.


The caveat in the MSVC-specific code that Noah pointed to still applies, 
but it appears that we don't in fact use these constants anywhere other 
than in relation to sockets.


I'm about to update my buildfarm member jacana so it has the latest 
mingw-w64 compiler and this should exhibit this error. Then I'll apply a 
patch along the lines of option 1. If nothing breaks, I'll backpatch 
that to 9.1 where we enabled use of this compiler.


cheers

andrew





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

Re: [HACKERS] Fix Windows socket error checking for MinGW

2013-08-19 Thread Michael Cronenworth 
On 08/17/2013 12:16 AM, Noah Misch wrote:
 1. Redefine those constants for more (all?) compilers.
 2. Remove that block and put #ifdef around all usage of such constants in
 frontend code, as you have done.
 3. Remove that block and make src/backend/port/win32/socket.c frontend-usable,
 so frontend code can treat errno like backend code treats errno.
 
 What do you recommend?

Option 1 is dangerous. I'd rather let the environments keep their constants.

Option 2 is the least dangerous but it adds lines of code.

Option 3: The errno variable is not set in Windows so relying on it is not 
possible.

If no one likes my patch then you need to come up with your own constants (ex.
PG_EINPROGRESS) and define those based on the compiler environment.


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

Re: [HACKERS] Fix Windows socket error checking for MinGW

2013-08-19 Thread Michael Cronenworth 
On 08/18/2013 12:02 PM, Andrew Dunstan wrote:
 There is a much simpler fix, which is to do these assignments unconditionally 
 in
 src/port/win32.h. The following small change fixes the problem for me:
 

No. Please do not do this.

 
 Note that the original patch appears to be not only misguided but wrong, in 
 that
 it undid a recent important change (commit a099482c) as I read it.

My patch was created against the latest git checkout as of the date I sent the
e-mail. If you could provide the full commit ID I could take a look, but it
seems you don't care about my patch enough I'm not sure you will bother.


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

Re: [HACKERS] Fix Windows socket error checking for MinGW

2013-08-19 Thread Michael Cronenworth 
On 08/17/2013 02:33 PM, Noah Misch wrote:
 One of the reports Michael cited was apparently native:
 http://www.postgresql.org/message-id/e1uclpd-l4...@wrigleys.postgresql.org
 
 Perhaps only some versions of w32api trigger the problem.  I agree we ought to
 understand the necessary conditions before proceeding.

Correct.

This is *not* a gcc/cross-compile issue. It's a conditional variable definition
issue.


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

Re: [HACKERS] Fix Windows socket error checking for MinGW

2013-08-19 Thread Andrew Dunstan 


On 08/19/2013 09:50 AM, Michael Cronenworth wrote:

On 08/18/2013 12:02 PM, Andrew Dunstan wrote:

There is a much simpler fix, which is to do these assignments unconditionally in
src/port/win32.h. The following small change fixes the problem for me:


No. Please do not do this.



If you object to a proposal then you need to explain what's wrong with 
it. Note that we already do exactly what is proposed here for EAGAIN and 
EINTR. In fact there is probably a good argument for doing it for all 
these constants.


I tested the patch I made. The environment was a linux hosted 
cross-compile, with the latest mingw-64 compiler. Before the patch the 
problem complained of was exhibited. After the patch it was not.





Note that the original patch appears to be not only misguided but wrong, in that
it undid a recent important change (commit a099482c) as I read it.

My patch was created against the latest git checkout as of the date I sent the
e-mail. If you could provide the full commit ID I could take a look, but it
seems you don't care about my patch enough I'm not sure you will bother.





I already gave you a sufficient identifier for the commit. In case 
you're not aware, git is quite happy dealing with small commit 
identifiers. If you do git log -1 a099482 you should get the details 
you require.


Frankly, we are not going to go through the code littering it with WSA 
constants inside #ifdef's without a very good reason.



cheers

andrew



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

Re: [HACKERS] Fix Windows socket error checking for MinGW

2013-08-19 Thread Michael Cronenworth 
On 08/19/2013 09:11 AM, Andrew Dunstan wrote:
 I already gave you a sufficient identifier for the commit. In case you're not
 aware, git is quite happy dealing with small commit identifiers. If you do 
 git
 log -1 a099482 you should get the details you require.

It should cross your mind that people have more than one computer and may reply
using a mobile phone that doesn't have git loaded on it.

When I'm back at my Postgres development system I'll gladly look at the git 
history.

P.S. The tone of your replies is very hostile and unwelcome.


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

Re: [HACKERS] Fix Windows socket error checking for MinGW

2013-08-19 Thread Robert Haas 
On Mon, Aug 19, 2013 at 10:18 AM, Michael Cronenworth m...@cchtml.com wrote:
 On 08/19/2013 09:11 AM, Andrew Dunstan wrote:
 I already gave you a sufficient identifier for the commit. In case you're not
 aware, git is quite happy dealing with small commit identifiers. If you do 
 git
 log -1 a099482 you should get the details you require.

 It should cross your mind that people have more than one computer and may 
 reply
 using a mobile phone that doesn't have git loaded on it.

 When I'm back at my Postgres development system I'll gladly look at the git 
 history.

 P.S. The tone of your replies is very hostile and unwelcome.

Uh... I don't think Andrew has said anything terribly hostile.  He
claims that your patch reverts a change made by another recent patch,
and he's right.  He doesn't mean that you intentionally did that by
basing your patch off the wrong git commit; he means that the changes
in your patch happen to be the opposite of part of what was in that
previous patch.  That means that either the previous patch was wrong,
or yours is.

I'm not sure why it should be Andrew's job to care about which
machines you have git installed on.  If you can't look up the SHA he
provided on the platform you're currently using, that's your problem,
not his.  This is not exactly time-critical, so you could have just as
easily waited until you were back at your laptop before replying.

TBH, I think Andrew is going above and beyond the call of duty by
developing and testing a patch for the problem you reported.

And FWIW, I also agree that we should avoid putting in direct
references to WSA* constants in various portions of the code base.
We've worked pretty hard to paper over to the differences between
Windows and everything else, and the reward of that labor is that 95%
of the code we write doesn't have to care about which operating system
it's running on.  That's a good thing, because many developers - such
as myself - do not have Windows development environments set up, and
even those that do will not welcome an increased need to test a
proposed patch on multiple platforms.  I think everyone here accepts
that some level of additional effort is going to be needed, on an
ongoing basis, to support Windows.  But keeping that to a minimum is
important.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


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

Re: [HACKERS] Fix Windows socket error checking for MinGW

2013-08-19 Thread Alvaro Herrera 
Michael Cronenworth wrote:
 On 08/17/2013 12:16 AM, Noah Misch wrote:
  1. Redefine those constants for more (all?) compilers.
  2. Remove that block and put #ifdef around all usage of such constants in
  frontend code, as you have done.
  3. Remove that block and make src/backend/port/win32/socket.c 
  frontend-usable,
  so frontend code can treat errno like backend code treats errno.
  
  What do you recommend?
 
 Option 1 is dangerous.

How so?

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training  Services


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

Re: [HACKERS] Fix Windows socket error checking for MinGW

2013-08-19 Thread Andrew Dunstan 


On 08/19/2013 10:35 AM, Robert Haas wrote:

TBH, I think Andrew is going above and beyond the call of duty by
developing and testing a patch for the problem you reported.


I've actually known about this problem for a while, but mistakenly 
thought it only occurred for cross-compilation: 
http://people.planetpostgresql.org/andrew/index.php?/archives/264-Cross-compiling-PostgreSQL-for-WIndows.html, 
so I'm at least grateful for this thread having prodded me about it, as 
well as pointing fairly closely to the problem area.



cheers

andrew



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

Re: [HACKERS] Fix Windows socket error checking for MinGW

2013-08-19 Thread Noah Misch 
On Sun, Aug 18, 2013 at 01:02:57PM -0400, Andrew Dunstan wrote:
 On 08/17/2013 01:16 AM, Noah Misch wrote:
 /*
   * For Microsoft Visual Studio 2010 and above we intentionally redefine
   * the regular Berkeley error constants and set them to the WSA constants.
   * Note that this will break if those constants are used for anything else
   * than Windows Sockets errors.
   */
 #if _MSC_VER = 1600
 #pragma warning(disable:4005)
 #define EMSGSIZE WSAEMSGSIZE
 #define EAFNOSUPPORT WSAEAFNOSUPPORT
 #define EWOULDBLOCK WSAEWOULDBLOCK
 #define EPROTONOSUPPORT WSAEPROTONOSUPPORT
 #define ECONNRESET WSAECONNRESET
 #define EINPROGRESS WSAEINPROGRESS
 #define ENOBUFS WSAENOBUFS
 #define ECONNREFUSED WSAECONNREFUSED
 #define EOPNOTSUPP WSAEOPNOTSUPP
 #pragma warning(default:4005)
 #endif

 I suspect we should do one of the following:

 1. Redefine those constants for more (all?) compilers.
 2. Remove that block and put #ifdef around all usage of such constants in
 frontend code, as you have done.
 3. Remove that block and make src/backend/port/win32/socket.c 
 frontend-usable,
 so frontend code can treat errno like backend code treats errno.

 There is a much simpler fix, which is to do these assignments  
 unconditionally in src/port/win32.h. The following small change fixes  
 the problem for me:

That was option #1.  (You weren't planning to change just the one symbol
causing the failure at hand, were you?)  Reasonable choice.  The point in the
code comment quoted above looks bad, but the lack of reports of that nature
against official 9.2 binaries corroborates it having not been a problem yet.
The only non-socket use I see for the constants in question is the EINTR test
in XLogWrite(), which probably can't happen on Windows.

 Note that the original patch appears to be not only misguided but wrong,  
 in that it undid a recent important change (commit a099482c) as I read 
 it.

Ah; true enough.

Thanks,
nm

-- 
Noah Misch
EnterpriseDB http://www.enterprisedb.com


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

Re: [HACKERS] Fix Windows socket error checking for MinGW

2013-08-19 Thread Michael Cronenworth 

On 08/19/2013 07:35 PM, Noah Misch wrote:

That was option #1.  (You weren't planning to change just the one symbol
causing the failure at hand, were you?)  Reasonable choice.  The point in the
code comment quoted above looks bad, but the lack of reports of that nature
against official 9.2 binaries corroborates it having not been a problem yet.
The only non-socket use I see for the constants in question is the EINTR test
in XLogWrite(), which probably can't happen on Windows.


Redefining compiler constants is bad bandaid. A similar bandaid was in 
place to begin with caused this problem. If you believe PostgreSQL will 
never use code that needs the true errno value then go ahead with Option 1.



Ah; true enough.


After looking over the changes it was a merge mistake. I originally 
wrote the patch on the 9.2 branch and ported it to master. Nothing to 
call misguided.


Just so that everyone understands: I was looking for comments to discuss 
a solution to the socket error checking problem. I'm not looking to 
shove anything down one's throat.



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

Re: [HACKERS] Fix Windows socket error checking for MinGW

2013-08-18 Thread Andrew Dunstan 


On 08/17/2013 01:16 AM, Noah Misch wrote:

On Fri, Aug 16, 2013 at 06:56:45PM -0500, Michael Cronenworth wrote:

I started a thread on the general list so read that for more info.

http://www.postgresql.org/message-id/520a6e55.40...@cchtml.com

I'm also going to submit the patch to CommitFest.
+#ifndef WIN32
if (SOCK_ERRNO == EWOULDBLOCK)
+#else
+   if (SOCK_ERRNO == WSAEWOULDBLOCK)
+#endif

Thanks for looking into this.  I suspect this patch is achieving the right
runtime behavior, but some cleanup is in order.  src/include/port/win32.h
makes some effort to preempt the need for a patch like this, but the relevant
code isn't used for MinGW:

/*
  * For Microsoft Visual Studio 2010 and above we intentionally redefine
  * the regular Berkeley error constants and set them to the WSA constants.
  * Note that this will break if those constants are used for anything else
  * than Windows Sockets errors.
  */
#if _MSC_VER = 1600
#pragma warning(disable:4005)
#define EMSGSIZE WSAEMSGSIZE
#define EAFNOSUPPORT WSAEAFNOSUPPORT
#define EWOULDBLOCK WSAEWOULDBLOCK
#define EPROTONOSUPPORT WSAEPROTONOSUPPORT
#define ECONNRESET WSAECONNRESET
#define EINPROGRESS WSAEINPROGRESS
#define ENOBUFS WSAENOBUFS
#define ECONNREFUSED WSAECONNREFUSED
#define EOPNOTSUPP WSAEOPNOTSUPP
#pragma warning(default:4005)
#endif

I suspect we should do one of the following:

1. Redefine those constants for more (all?) compilers.
2. Remove that block and put #ifdef around all usage of such constants in
frontend code, as you have done.
3. Remove that block and make src/backend/port/win32/socket.c frontend-usable,
so frontend code can treat errno like backend code treats errno.

What do you recommend?

Thanks,
nm





There is a much simpler fix, which is to do these assignments 
unconditionally in src/port/win32.h. The following small change fixes 
the problem for me:


diff --git a/src/include/port/win32.h b/src/include/port/win32.h
index 3a68ea4..5b93220 100644
--- a/src/include/port/win32.h
+++ b/src/include/port/win32.h
@@ -278,9 +278,8 @@ typedef int pid_t;
 #ifndef EAFNOSUPPORT
 #define EAFNOSUPPORT WSAEAFNOSUPPORT
 #endif
-#ifndef EWOULDBLOCK
+#undef EWOULDBLOCK
 #define EWOULDBLOCK WSAEWOULDBLOCK
-#endif
 #ifndef ECONNRESET
 #define ECONNRESET WSAECONNRESET
 #endif


Note that the original patch appears to be not only misguided but wrong, 
in that it undid a recent important change (commit a099482c) as I read it.


cheers

andrew


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

Re: [HACKERS] Fix Windows socket error checking for MinGW

2013-08-17 Thread Andrew Dunstan 


On 08/17/2013 01:16 AM, Noah Misch wrote:

On Fri, Aug 16, 2013 at 06:56:45PM -0500, Michael Cronenworth wrote:

I started a thread on the general list so read that for more info.

http://www.postgresql.org/message-id/520a6e55.40...@cchtml.com

I'm also going to submit the patch to CommitFest.
+#ifndef WIN32
if (SOCK_ERRNO == EWOULDBLOCK)
+#else
+   if (SOCK_ERRNO == WSAEWOULDBLOCK)
+#endif

Thanks for looking into this.  I suspect this patch is achieving the right
runtime behavior, but some cleanup is in order.  src/include/port/win32.h
makes some effort to preempt the need for a patch like this, but the relevant
code isn't used for MinGW:

/*
  * For Microsoft Visual Studio 2010 and above we intentionally redefine
  * the regular Berkeley error constants and set them to the WSA constants.
  * Note that this will break if those constants are used for anything else
  * than Windows Sockets errors.
  */
#if _MSC_VER = 1600
#pragma warning(disable:4005)
#define EMSGSIZE WSAEMSGSIZE
#define EAFNOSUPPORT WSAEAFNOSUPPORT
#define EWOULDBLOCK WSAEWOULDBLOCK
#define EPROTONOSUPPORT WSAEPROTONOSUPPORT
#define ECONNRESET WSAECONNRESET
#define EINPROGRESS WSAEINPROGRESS
#define ENOBUFS WSAENOBUFS
#define ECONNREFUSED WSAECONNREFUSED
#define EOPNOTSUPP WSAEOPNOTSUPP
#pragma warning(default:4005)
#endif

I suspect we should do one of the following:

1. Redefine those constants for more (all?) compilers.
2. Remove that block and put #ifdef around all usage of such constants in
frontend code, as you have done.
3. Remove that block and make src/backend/port/win32/socket.c frontend-usable,
so frontend code can treat errno like backend code treats errno.

What do you recommend?




We don't seem to have a problem with this on native builds, only on 
cross-compiles AFAIK (see buildfarm for proof). The native mingw-w64 
build works just fine. So my first question is going to be why is the 
cross-compile different?



cheers

andrew


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

Re: [HACKERS] Fix Windows socket error checking for MinGW

2013-08-17 Thread Noah Misch 
On Sat, Aug 17, 2013 at 02:04:24PM -0400, Andrew Dunstan wrote:
 On Fri, Aug 16, 2013 at 06:56:45PM -0500, Michael Cronenworth wrote:
 +#ifndef WIN32
 if (SOCK_ERRNO == EWOULDBLOCK)
 +#else
 +   if (SOCK_ERRNO == WSAEWOULDBLOCK)
 +#endif

 We don't seem to have a problem with this on native builds, only on  
 cross-compiles AFAIK (see buildfarm for proof). The native mingw-w64  
 build works just fine. So my first question is going to be why is the  
 cross-compile different?

One of the reports Michael cited was apparently native:
http://www.postgresql.org/message-id/e1uclpd-l4...@wrigleys.postgresql.org

Perhaps only some versions of w32api trigger the problem.  I agree we ought to
understand the necessary conditions before proceeding.

-- 
Noah Misch
EnterpriseDB http://www.enterprisedb.com


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

[HACKERS] Fix Windows socket error checking for MinGW

2013-08-16 Thread Michael Cronenworth 

I started a thread on the general list so read that for more info.

http://www.postgresql.org/message-id/520a6e55.40...@cchtml.com

I'm also going to submit the patch to CommitFest.

Thanks,
Michael
From bb79da0013d5169b4530df28ece0c296004d1db4 Mon Sep 17 00:00:00 2001
From: Michael Cronenworth m...@cchtml.com
Date: Fri, 16 Aug 2013 18:50:28 -0500
Subject: [PATCH] libpq: Fix Windows socket error checking for MinGW

---
 src/interfaces/libpq/fe-connect.c | 12 
 src/interfaces/libpq/fe-misc.c| 32 
 src/interfaces/libpq/fe-secure.c  | 16 
 3 files changed, 56 insertions(+), 4 deletions(-)

diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index 18fcb0c..3693245 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -1779,11 +1779,15 @@ keep_going:		/* We will come back to here until there is
 	if (connect(conn-sock, addr_cur-ai_addr,
 addr_cur-ai_addrlen)  0)
 	{
-		if (SOCK_ERRNO == EINPROGRESS ||
-#ifdef WIN32
-			SOCK_ERRNO == EWOULDBLOCK ||
+#ifndef WIN32
+ 		if (SOCK_ERRNO == EINPROGRESS ||
+ 			SOCK_ERRNO == EWOULDBLOCK ||
+ 			SOCK_ERRNO == EINTR)
+#else
+		if (SOCK_ERRNO == WSAEINPROGRESS ||
+			SOCK_ERRNO == WSAEWOULDBLOCK ||
+			SOCK_ERRNO == WSAEINTR)
 #endif
-			SOCK_ERRNO == EINTR)
 		{
 			/*
 			 * This is fine - we're in non-blocking mode, and
diff --git a/src/interfaces/libpq/fe-misc.c b/src/interfaces/libpq/fe-misc.c
index 6be3249..112cd12 100644
--- a/src/interfaces/libpq/fe-misc.c
+++ b/src/interfaces/libpq/fe-misc.c
@@ -655,7 +655,11 @@ retry3:
 		  conn-inBufSize - conn-inEnd);
 	if (nread  0)
 	{
+#ifndef WIN32
 		if (SOCK_ERRNO == EINTR)
+#else
+		if (SOCK_ERRNO == WSAEINTR)
+#endif
 			goto retry3;
 		/* Some systems return EAGAIN/EWOULDBLOCK for no data */
 #ifdef EAGAIN
@@ -663,12 +667,20 @@ retry3:
 			return someread;
 #endif
 #if defined(EWOULDBLOCK)  (!defined(EAGAIN) || (EWOULDBLOCK != EAGAIN))
+#ifndef WIN32
 		if (SOCK_ERRNO == EWOULDBLOCK)
+#else
+		if (SOCK_ERRNO == WSAEWOULDBLOCK)
+#endif
 			return someread;
 #endif
 		/* We might get ECONNRESET here if using TCP and backend died */
 #ifdef ECONNRESET
+#ifndef WIN32
 		if (SOCK_ERRNO == ECONNRESET)
+#else
+		if (SOCK_ERRNO == WSAECONNRESET)
+#endif
 			goto definitelyFailed;
 #endif
 		/* pqsecure_read set the error message for us */
@@ -748,7 +760,11 @@ retry4:
 		  conn-inBufSize - conn-inEnd);
 	if (nread  0)
 	{
+#ifndef WIN32
 		if (SOCK_ERRNO == EINTR)
+#else
+		if (SOCK_ERRNO == WSAEINTR)
+#endif
 			goto retry4;
 		/* Some systems return EAGAIN/EWOULDBLOCK for no data */
 #ifdef EAGAIN
@@ -756,12 +772,20 @@ retry4:
 			return 0;
 #endif
 #if defined(EWOULDBLOCK)  (!defined(EAGAIN) || (EWOULDBLOCK != EAGAIN))
+#ifndef WIN32
 		if (SOCK_ERRNO == EWOULDBLOCK)
+#else
+		if (SOCK_ERRNO == WSAEWOULDBLOCK)
+#endif
 			return 0;
 #endif
 		/* We might get ECONNRESET here if using TCP and backend died */
 #ifdef ECONNRESET
+#ifndef WIN32
 		if (SOCK_ERRNO == ECONNRESET)
+#else
+		if (SOCK_ERRNO == WSAECONNRESET)
+#endif
 			goto definitelyFailed;
 #endif
 		/* pqsecure_read set the error message for us */
@@ -834,10 +858,18 @@ pqSendSome(PGconn *conn, int len)
 	break;
 #endif
 #if defined(EWOULDBLOCK)  (!defined(EAGAIN) || (EWOULDBLOCK != EAGAIN))
+#ifndef WIN32
 case EWOULDBLOCK:
+#else
+case WSAEWOULDBLOCK:
+#endif
 	break;
 #endif
+#ifndef WIN32
 case EINTR:
+#else
+case WSAEINTR:
+#endif
 	continue;
 
 default:
diff --git a/src/interfaces/libpq/fe-secure.c b/src/interfaces/libpq/fe-secure.c
index b16968b..aa48bd8 100644
--- a/src/interfaces/libpq/fe-secure.c
+++ b/src/interfaces/libpq/fe-secure.c
@@ -451,12 +451,20 @@ rloop:
 #if defined(EWOULDBLOCK)  (!defined(EAGAIN) || (EWOULDBLOCK != EAGAIN))
 case EWOULDBLOCK:
 #endif
+#ifndef WIN32
 case EINTR:
+#else
+case WSAEWOULDBLOCK:
+case WSAEINTR:
+#endif
 	/* no error message, caller is expected to retry */
 	break;
 
 #ifdef ECONNRESET
 case ECONNRESET:
+#ifdef WIN32
+case WSAECONNRESET:
+#endif
 	printfPQExpBuffer(conn-errorMessage,
 	  libpq_gettext(
 server closed the connection unexpectedly\n
@@ -635,7 +643,12 @@ retry_masked:
 #if defined(EWOULDBLOCK)  (!defined(EAGAIN) || (EWOULDBLOCK != EAGAIN))
 case EWOULDBLOCK:
 #endif
+#ifndef WIN32
 case EINTR:
+#else
+case WSAEWOULDBLOCK:
+case WSAEINTR:
+#endif
 	/* no error message, caller is expected to retry */
 	break;
 
@@ -647,6 +660,9 @@ retry_masked:
 #ifdef ECONNRESET
 case ECONNRESET:
 #endif
+#ifdef WIN32
+case WSAECONNRESET:
+#endif
 	printfPQExpBuffer(conn-errorMessage,
 	  libpq_gettext(
 server closed the connection unexpectedly\n
-- 
1.8.3.1


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

Re: [HACKERS] Fix Windows socket error checking for MinGW

2013-08-16 Thread Noah Misch 
On Fri, Aug 16, 2013 at 06:56:45PM -0500, Michael Cronenworth wrote:
 I started a thread on the general list so read that for more info.

 http://www.postgresql.org/message-id/520a6e55.40...@cchtml.com

 I'm also going to submit the patch to CommitFest.

 +#ifndef WIN32
   if (SOCK_ERRNO == EWOULDBLOCK)
 +#else
 + if (SOCK_ERRNO == WSAEWOULDBLOCK)
 +#endif

Thanks for looking into this.  I suspect this patch is achieving the right
runtime behavior, but some cleanup is in order.  src/include/port/win32.h
makes some effort to preempt the need for a patch like this, but the relevant
code isn't used for MinGW:

/*
 * For Microsoft Visual Studio 2010 and above we intentionally redefine
 * the regular Berkeley error constants and set them to the WSA constants.
 * Note that this will break if those constants are used for anything else
 * than Windows Sockets errors.
 */
#if _MSC_VER = 1600
#pragma warning(disable:4005)
#define EMSGSIZE WSAEMSGSIZE
#define EAFNOSUPPORT WSAEAFNOSUPPORT
#define EWOULDBLOCK WSAEWOULDBLOCK
#define EPROTONOSUPPORT WSAEPROTONOSUPPORT
#define ECONNRESET WSAECONNRESET
#define EINPROGRESS WSAEINPROGRESS
#define ENOBUFS WSAENOBUFS
#define ECONNREFUSED WSAECONNREFUSED
#define EOPNOTSUPP WSAEOPNOTSUPP
#pragma warning(default:4005)
#endif

I suspect we should do one of the following:

1. Redefine those constants for more (all?) compilers.
2. Remove that block and put #ifdef around all usage of such constants in
frontend code, as you have done.
3. Remove that block and make src/backend/port/win32/socket.c frontend-usable,
so frontend code can treat errno like backend code treats errno.

What do you recommend?

Thanks,
nm

-- 
Noah Misch
EnterpriseDB http://www.enterprisedb.com


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