[BUG FIX] Compare returned value by socket() against PGINVALID_SOCKET instead of < 0
Hi,
I've tried to fix some bugs reported by Andrey Karpov in an article at
http://www.viva64.com/en/b/0227/
The value returned by socket() is unsigned on Windows and can thus not
be checked if less than zero to detect an error, instead
PGINVALID_SOCKET should be used, which is hard-coded to -1 on
non-windows platforms.
Joel Jacobson
Attachments:
use-PGINVALID_SOCKET.patchapplication/octet-stream; name=use-PGINVALID_SOCKET.patchDownload
*** a/src/backend/libpq/auth.c
--- b/src/backend/libpq/auth.c
***************
*** 1665,1671 **** ident_inet(hbaPort *port)
sock_fd = socket(ident_serv->ai_family, ident_serv->ai_socktype,
ident_serv->ai_protocol);
! if (sock_fd < 0)
{
ereport(LOG,
(errcode_for_socket_access(),
--- 1665,1671 ----
sock_fd = socket(ident_serv->ai_family, ident_serv->ai_socktype,
ident_serv->ai_protocol);
! if (sock_fd == PGINVALID_SOCKET)
{
ereport(LOG,
(errcode_for_socket_access(),
***************
*** 1745,1751 **** ident_inet(hbaPort *port)
ident_response)));
ident_inet_done:
! if (sock_fd >= 0)
closesocket(sock_fd);
pg_freeaddrinfo_all(remote_addr.addr.ss_family, ident_serv);
pg_freeaddrinfo_all(local_addr.addr.ss_family, la);
--- 1745,1751 ----
ident_response)));
ident_inet_done:
! if (sock_fd != PGINVALID_SOCKET)
closesocket(sock_fd);
pg_freeaddrinfo_all(remote_addr.addr.ss_family, ident_serv);
pg_freeaddrinfo_all(local_addr.addr.ss_family, la);
***************
*** 2563,2569 **** CheckRADIUSAuth(Port *port)
packet->length = htons(packet->length);
sock = socket(serveraddrs[0].ai_family, SOCK_DGRAM, 0);
! if (sock < 0)
{
ereport(LOG,
(errmsg("could not create RADIUS socket: %m")));
--- 2563,2569 ----
packet->length = htons(packet->length);
sock = socket(serveraddrs[0].ai_family, SOCK_DGRAM, 0);
! if (sock == PGINVALID_SOCKET)
{
ereport(LOG,
(errmsg("could not create RADIUS socket: %m")));
*** a/src/backend/libpq/pqcomm.c
--- b/src/backend/libpq/pqcomm.c
***************
*** 392,398 **** StreamServerPort(int family, char *hostName, unsigned short portNumber,
break;
}
! if ((fd = socket(addr->ai_family, SOCK_STREAM, 0)) < 0)
{
ereport(LOG,
(errcode_for_socket_access(),
--- 392,398 ----
break;
}
! if ((fd = socket(addr->ai_family, SOCK_STREAM, 0)) == PGINVALID_SOCKET)
{
ereport(LOG,
(errcode_for_socket_access(),
***************
*** 632,638 **** StreamConnection(pgsocket server_fd, Port *port)
port->raddr.salen = sizeof(port->raddr.addr);
if ((port->sock = accept(server_fd,
(struct sockaddr *) & port->raddr.addr,
! &port->raddr.salen)) < 0)
{
ereport(LOG,
(errcode_for_socket_access(),
--- 632,638 ----
port->raddr.salen = sizeof(port->raddr.addr);
if ((port->sock = accept(server_fd,
(struct sockaddr *) & port->raddr.addr,
! &port->raddr.salen)) == PGINVALID_SOCKET)
{
ereport(LOG,
(errcode_for_socket_access(),
*** a/src/backend/postmaster/postmaster.c
--- b/src/backend/postmaster/postmaster.c
***************
*** 2165,2171 **** ConnCreate(int serverFd)
if (StreamConnection(serverFd, port) != STATUS_OK)
{
! if (port->sock >= 0)
StreamClose(port->sock);
ConnFree(port);
return NULL;
--- 2165,2171 ----
if (StreamConnection(serverFd, port) != STATUS_OK)
{
! if (port->sock != PGINVALID_SOCKET)
StreamClose(port->sock);
ConnFree(port);
return NULL;
On Wed, Dec 25, 2013 at 6:05 PM, Joel Jacobson <joel@trustly.com> wrote:
Hi,
I've tried to fix some bugs reported by Andrey Karpov in an article at
http://www.viva64.com/en/b/0227/The value returned by socket() is unsigned on Windows and can thus not
be checked if less than zero to detect an error, instead
PGINVALID_SOCKET should be used, which is hard-coded to -1 on
non-windows platforms.
Though I have not verified all instances you have fixed in your patch, but I
feel the general direction to fix is right.
I think you can upload your patch for next CommitFest.
With Regards,
Amit Kapila.
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
Joel Jacobson wrote:
Hi,
I've tried to fix some bugs reported by Andrey Karpov in an article at
http://www.viva64.com/en/b/0227/The value returned by socket() is unsigned on Windows and can thus not
be checked if less than zero to detect an error, instead
PGINVALID_SOCKET should be used, which is hard-coded to -1 on
non-windows platforms.
Can I bug you into verifying what supported releases need this patch,
and to which does it backpatch cleanly? And if there's any to which it
doesn't, can I further bug you into providing one that does?
--
�lvaro Herrera http://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
On Wed, Dec 25, 2013 at 01:35:15PM +0100, Joel Jacobson wrote:
Hi,
I've tried to fix some bugs reported by Andrey Karpov in an article at
http://www.viva64.com/en/b/0227/The value returned by socket() is unsigned on Windows and can thus not
be checked if less than zero to detect an error, instead
PGINVALID_SOCKET should be used, which is hard-coded to -1 on
non-windows platforms.
I reviewed this patch and you are correct that we are not handling
socket() and accept() returns properly on Windows. We were doing it
properly in most place in the backend, but your patch fixes the
remaining places:
http://msdn.microsoft.com/en-us/library/windows/desktop/ms740516%28v=vs.85%29.aspx
However, libpq doesn't seem to be doing much to handle Windows properly
in this area. I have adjusted libpq to map socket to -1, but the proper
fix is to distribute pgsocket and PGINVALID_SOCKET checks throughout the
libpq code. I am not sure how to handle PQsocket() --- should it still
return -1? Having the return value be conditional on the operating
system is ugly. How much of this should be backpatched? Why aren't we
getting warnings on Windows about assigning the socket() return value to
an integer?
Updated patch attached.
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ Everyone has their own god. +
Attachments:
socket.difftext/x-diff; charset=us-asciiDownload
diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c
new file mode 100644
index 31ade0b..f674372
*** a/src/backend/libpq/auth.c
--- b/src/backend/libpq/auth.c
*************** ident_inet(hbaPort *port)
*** 1453,1459 ****
sock_fd = socket(ident_serv->ai_family, ident_serv->ai_socktype,
ident_serv->ai_protocol);
! if (sock_fd < 0)
{
ereport(LOG,
(errcode_for_socket_access(),
--- 1453,1459 ----
sock_fd = socket(ident_serv->ai_family, ident_serv->ai_socktype,
ident_serv->ai_protocol);
! if (sock_fd == PGINVALID_SOCKET)
{
ereport(LOG,
(errcode_for_socket_access(),
*************** ident_inet(hbaPort *port)
*** 1533,1539 ****
ident_response)));
ident_inet_done:
! if (sock_fd >= 0)
closesocket(sock_fd);
pg_freeaddrinfo_all(remote_addr.addr.ss_family, ident_serv);
pg_freeaddrinfo_all(local_addr.addr.ss_family, la);
--- 1533,1539 ----
ident_response)));
ident_inet_done:
! if (sock_fd != PGINVALID_SOCKET)
closesocket(sock_fd);
pg_freeaddrinfo_all(remote_addr.addr.ss_family, ident_serv);
pg_freeaddrinfo_all(local_addr.addr.ss_family, la);
*************** CheckRADIUSAuth(Port *port)
*** 2351,2357 ****
packet->length = htons(packet->length);
sock = socket(serveraddrs[0].ai_family, SOCK_DGRAM, 0);
! if (sock < 0)
{
ereport(LOG,
(errmsg("could not create RADIUS socket: %m")));
--- 2351,2357 ----
packet->length = htons(packet->length);
sock = socket(serveraddrs[0].ai_family, SOCK_DGRAM, 0);
! if (sock == PGINVALID_SOCKET)
{
ereport(LOG,
(errmsg("could not create RADIUS socket: %m")));
diff --git a/src/backend/libpq/ip.c b/src/backend/libpq/ip.c
new file mode 100644
index 7e8fc78..4db64fb
*** a/src/backend/libpq/ip.c
--- b/src/backend/libpq/ip.c
*************** pg_foreach_ifaddr(PgIfAddrCallback callb
*** 670,676 ****
total;
sock = socket(AF_INET, SOCK_DGRAM, 0);
! if (sock == -1)
return -1;
while (n_buffer < 1024 * 100)
--- 670,676 ----
total;
sock = socket(AF_INET, SOCK_DGRAM, 0);
! if (sock == PGINVALID_SOCKET)
return -1;
while (n_buffer < 1024 * 100)
*************** pg_foreach_ifaddr(PgIfAddrCallback callb
*** 711,717 ****
#ifdef HAVE_IPV6
/* We'll need an IPv6 socket too for the SIOCGLIFNETMASK ioctls */
sock6 = socket(AF_INET6, SOCK_DGRAM, 0);
! if (sock6 == -1)
{
free(buffer);
close(sock);
--- 711,717 ----
#ifdef HAVE_IPV6
/* We'll need an IPv6 socket too for the SIOCGLIFNETMASK ioctls */
sock6 = socket(AF_INET6, SOCK_DGRAM, 0);
! if (sock6 == PGINVALID_SOCKET)
{
free(buffer);
close(sock);
*************** pg_foreach_ifaddr(PgIfAddrCallback callb
*** 788,797 ****
char *ptr,
*buffer = NULL;
size_t n_buffer = 1024;
! int sock;
sock = socket(AF_INET, SOCK_DGRAM, 0);
! if (sock == -1)
return -1;
while (n_buffer < 1024 * 100)
--- 788,797 ----
char *ptr,
*buffer = NULL;
size_t n_buffer = 1024;
! pgsocket sock;
sock = socket(AF_INET, SOCK_DGRAM, 0);
! if (sock == PGINVALID_SOCKET)
return -1;
while (n_buffer < 1024 * 100)
diff --git a/src/backend/libpq/pqcomm.c b/src/backend/libpq/pqcomm.c
new file mode 100644
index 1eae183..0179451
*** a/src/backend/libpq/pqcomm.c
--- b/src/backend/libpq/pqcomm.c
*************** StreamServerPort(int family, char *hostN
*** 392,398 ****
break;
}
! if ((fd = socket(addr->ai_family, SOCK_STREAM, 0)) < 0)
{
ereport(LOG,
(errcode_for_socket_access(),
--- 392,398 ----
break;
}
! if ((fd = socket(addr->ai_family, SOCK_STREAM, 0)) == PGINVALID_SOCKET)
{
ereport(LOG,
(errcode_for_socket_access(),
*************** StreamConnection(pgsocket server_fd, Por
*** 632,638 ****
port->raddr.salen = sizeof(port->raddr.addr);
if ((port->sock = accept(server_fd,
(struct sockaddr *) & port->raddr.addr,
! &port->raddr.salen)) < 0)
{
ereport(LOG,
(errcode_for_socket_access(),
--- 632,638 ----
port->raddr.salen = sizeof(port->raddr.addr);
if ((port->sock = accept(server_fd,
(struct sockaddr *) & port->raddr.addr,
! &port->raddr.salen)) == PGINVALID_SOCKET)
{
ereport(LOG,
(errcode_for_socket_access(),
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
new file mode 100644
index e9072b7..cc844b9
*** a/src/backend/postmaster/postmaster.c
--- b/src/backend/postmaster/postmaster.c
*************** ConnCreate(int serverFd)
*** 2169,2175 ****
if (StreamConnection(serverFd, port) != STATUS_OK)
{
! if (port->sock >= 0)
StreamClose(port->sock);
ConnFree(port);
return NULL;
--- 2169,2175 ----
if (StreamConnection(serverFd, port) != STATUS_OK)
{
! if (port->sock != PGINVALID_SOCKET)
StreamClose(port->sock);
ConnFree(port);
return NULL;
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
new file mode 100644
index d53c41f..ec68be6
*** a/src/interfaces/libpq/fe-connect.c
--- b/src/interfaces/libpq/fe-connect.c
*************** keep_going: /* We will come back to
*** 1632,1639 ****
conn->raddr.salen = addr_cur->ai_addrlen;
/* Open a socket */
! conn->sock = socket(addr_cur->ai_family, SOCK_STREAM, 0);
! if (conn->sock < 0)
{
/*
* ignore socket() failure if we have more addresses
--- 1632,1654 ----
conn->raddr.salen = addr_cur->ai_addrlen;
/* Open a socket */
! {
! /*
! * While we use 'pgsocket' as the socket type in the
! * backend, we use 'int' for libpq socket values.
! * This requires us to map PGINVALID_SOCKET to -1
! * on Windows.
! * See http://msdn.microsoft.com/en-us/library/windows/desktop/ms740516%28v=vs.85%29.aspx
! */
! pgsocket sock = socket(addr_cur->ai_family, SOCK_STREAM, 0);
! #ifdef WIN32
! if (sock == PGINVALID_SOCKET)
! conn->sock = -1;
! else
! #endif
! conn->sock = sock;
! }
! if (conn->sock == -1)
{
/*
* ignore socket() failure if we have more addresses
*************** internal_cancel(SockAddr *raddr, int be_
*** 3136,3142 ****
char *errbuf, int errbufsize)
{
int save_errno = SOCK_ERRNO;
! int tmpsock = -1;
char sebuf[256];
int maxlen;
struct
--- 3151,3157 ----
char *errbuf, int errbufsize)
{
int save_errno = SOCK_ERRNO;
! pgsocket tmpsock = PGINVALID_SOCKET;
char sebuf[256];
int maxlen;
struct
*************** internal_cancel(SockAddr *raddr, int be_
*** 3149,3155 ****
* We need to open a temporary connection to the postmaster. Do this with
* only kernel calls.
*/
! if ((tmpsock = socket(raddr->addr.ss_family, SOCK_STREAM, 0)) < 0)
{
strlcpy(errbuf, "PQcancel() -- socket() failed: ", errbufsize);
goto cancel_errReturn;
--- 3164,3170 ----
* We need to open a temporary connection to the postmaster. Do this with
* only kernel calls.
*/
! if ((tmpsock = socket(raddr->addr.ss_family, SOCK_STREAM, 0)) == PGINVALID_SOCKET)
{
strlcpy(errbuf, "PQcancel() -- socket() failed: ", errbufsize);
goto cancel_errReturn;
*************** cancel_errReturn:
*** 3220,3226 ****
maxlen);
strcat(errbuf, "\n");
}
! if (tmpsock >= 0)
closesocket(tmpsock);
SOCK_ERRNO_SET(save_errno);
return FALSE;
--- 3235,3241 ----
maxlen);
strcat(errbuf, "\n");
}
! if (tmpsock != PGINVALID_SOCKET)
closesocket(tmpsock);
SOCK_ERRNO_SET(save_errno);
return FALSE;
diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h
new file mode 100644
index 22bbe4a..ee975d4
*** a/src/interfaces/libpq/libpq-int.h
--- b/src/interfaces/libpq/libpq-int.h
*************** struct pg_conn
*** 364,369 ****
--- 364,370 ----
PGnotify *notifyTail; /* newest unreported Notify msg */
/* Connection data */
+ /* See PQconnectPoll() for how we use 'int' and not 'pgsocket'. */
int sock; /* Unix FD for socket, -1 if not connected */
SockAddr laddr; /* Local address */
SockAddr raddr; /* Remote address */
On Tue, Apr 1, 2014 at 6:31 AM, Bruce Momjian <bruce@momjian.us> wrote:
I reviewed this patch and you are correct that we are not handling
socket() and accept() returns properly on Windows. We were doing it
properly in most place in the backend, but your patch fixes the
remaining places:http://msdn.microsoft.com/en-us/library/windows/desktop/ms740516%28v=vs.85%29.aspx
However, libpq doesn't seem to be doing much to handle Windows properly
in this area. I have adjusted libpq to map socket to -1, but the proper
fix is to distribute pgsocket and PGINVALID_SOCKET checks throughout the
libpq code. I am not sure how to handle PQsocket() --- should it still
return -1?
I think changing PQsocket() can impact all existing applications as it
is mentioned
in docs that "result of -1 indicates that no server connection is
currently open.".
Do you see any compelling need to change return value of PQSocket() after
your patch?
Having the return value be conditional on the operating
system is ugly. How much of this should be backpatched?
I think it's okay to back patch all the changes.
Is there any part in patch which you feel is risky to back patch?
Why aren't we
getting warnings on Windows about assigning the socket() return value to
an integer?
I think by default Windows doesn't give warning for such code even at Warning
level 4. I have found one related link:
http://stackoverflow.com/questions/75385/make-vs-compiler-catch-signed-unsigned-assignments
Updated patch attached.
It seems you have missed to change at below places.
1.
int
pg_foreach_ifaddr(PgIfAddrCallback callback, void *cb_data)
sock = WSASocket(AF_INET, SOCK_DGRAM, 0, 0, 0, 0);
if (sock == SOCKET_ERROR)
2.
pgwin32_waitforsinglesocket(SOCKET s, int what, int timeout)
{
static HANDLE waitevent = INVALID_HANDLE_VALUE;
static SOCKET current_socket = -1;
With Regards,
Amit Kapila.
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
On Sun, Apr 6, 2014 at 11:45:59AM +0530, Amit Kapila wrote:
On Tue, Apr 1, 2014 at 6:31 AM, Bruce Momjian <bruce@momjian.us> wrote:
I reviewed this patch and you are correct that we are not handling
socket() and accept() returns properly on Windows. We were doing it
properly in most place in the backend, but your patch fixes the
remaining places:http://msdn.microsoft.com/en-us/library/windows/desktop/ms740516%28v=vs.85%29.aspx
However, libpq doesn't seem to be doing much to handle Windows properly
in this area. I have adjusted libpq to map socket to -1, but the proper
fix is to distribute pgsocket and PGINVALID_SOCKET checks throughout the
libpq code. I am not sure how to handle PQsocket() --- should it still
return -1?I think changing PQsocket() can impact all existing applications as
it is mentioned in docs that "result of -1 indicates that no server
connection is currently open.". Do you see any compelling need to
change return value of PQSocket() after your patch?
No, I do not. In fact, the SSL_get_fd() call in secure_read() returns a
signed integer too, and that is passed around like a socket, so in fact
the SSL API doesn't even allow us to get an unsigned value on Windows in
all cases.
Having the return value be conditional on the operating
system is ugly. How much of this should be backpatched?I think it's okay to back patch all the changes.
Is there any part in patch which you feel is risky to back patch?
Well, we would not backpatch this if it is just a stylistic fix, and I
am starting to think it just a style issue. This MSDN website says -1,
SOCKET_ERROR, and INVALID_SOCKET are very similar:
http://msdn.microsoft.com/en-us/library/windows/desktop/cc507522%28v=vs.85%29.aspx
and this Stackoverflow thread says the same:
http://stackoverflow.com/questions/10817252/why-is-invalid-socket-defined-as-0-in-winsock2-h-c
In fact, this C program compiled by gcc on Debian issues no compiler
warnings and returns 'hello', showing that -1 and ~0 compare as equal:
int
main(int argc, char **argv)
{
int i;
unsigned int j;
i = -1;
j = ~0;
if (i == j)
printf("hello\n");
return 0;
}
meaning our incorrect syntax is computed correctly.
Why aren't we
getting warnings on Windows about assigning the socket() return value to
an integer?I think by default Windows doesn't give warning for such code even at Warning
level 4. I have found one related link:
http://stackoverflow.com/questions/75385/make-vs-compiler-catch-signed-unsigned-assignmentsUpdated patch attached.
It seems you have missed to change at below places.
1.
int
pg_foreach_ifaddr(PgIfAddrCallback callback, void *cb_data)
sock = WSASocket(AF_INET, SOCK_DGRAM, 0, 0, 0, 0);
if (sock == SOCKET_ERROR)
Well, the actual problem here is that WSASocket() returns INVALID_SOCKET
per the documentation, not SOCKET_ERROR. I did not use PGINVALID_SOCKET
here because this is Windows-specific code, defining 'sock' as SOCKET.
We could have sock defined as pgsocket, but because this is Windows code
already, it doesn't seem wise to mix portability code in there.
2.
pgwin32_waitforsinglesocket(SOCKET s, int what, int timeout)
{
static HANDLE waitevent = INVALID_HANDLE_VALUE;
static SOCKET current_socket = -1;
Yes, that -1 is wrong and I have changed it to INVALID_SOCKET, again
using the same rules that say PGINVALID_SOCKET doesn't make sense here
as it is Windows-specific code.
I am attaching an updated patch, which explains the PQsocket() return
value issue, and fixes the items listed above. I am inclined to apply
this just to head for correctness, and modify libpq to use pgsocket
consistently in a follow-up patch.
This is not like the readdir() fix we had to backpatch because that was
clearly not catching errors.
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ Everyone has their own god. +
Attachments:
socket.difftext/x-diff; charset=us-asciiDownload
diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c
new file mode 100644
index d062c1d..8fa9aa7
*** a/src/backend/libpq/auth.c
--- b/src/backend/libpq/auth.c
*************** ident_inet(hbaPort *port)
*** 1463,1469 ****
sock_fd = socket(ident_serv->ai_family, ident_serv->ai_socktype,
ident_serv->ai_protocol);
! if (sock_fd < 0)
{
ereport(LOG,
(errcode_for_socket_access(),
--- 1463,1469 ----
sock_fd = socket(ident_serv->ai_family, ident_serv->ai_socktype,
ident_serv->ai_protocol);
! if (sock_fd == PGINVALID_SOCKET)
{
ereport(LOG,
(errcode_for_socket_access(),
*************** ident_inet(hbaPort *port)
*** 1543,1549 ****
ident_response)));
ident_inet_done:
! if (sock_fd >= 0)
closesocket(sock_fd);
pg_freeaddrinfo_all(remote_addr.addr.ss_family, ident_serv);
pg_freeaddrinfo_all(local_addr.addr.ss_family, la);
--- 1543,1549 ----
ident_response)));
ident_inet_done:
! if (sock_fd != PGINVALID_SOCKET)
closesocket(sock_fd);
pg_freeaddrinfo_all(remote_addr.addr.ss_family, ident_serv);
pg_freeaddrinfo_all(local_addr.addr.ss_family, la);
*************** CheckRADIUSAuth(Port *port)
*** 2361,2367 ****
packet->length = htons(packet->length);
sock = socket(serveraddrs[0].ai_family, SOCK_DGRAM, 0);
! if (sock < 0)
{
ereport(LOG,
(errmsg("could not create RADIUS socket: %m")));
--- 2361,2367 ----
packet->length = htons(packet->length);
sock = socket(serveraddrs[0].ai_family, SOCK_DGRAM, 0);
! if (sock == PGINVALID_SOCKET)
{
ereport(LOG,
(errmsg("could not create RADIUS socket: %m")));
diff --git a/src/backend/libpq/ip.c b/src/backend/libpq/ip.c
new file mode 100644
index 7e8fc78..acdbab0
*** a/src/backend/libpq/ip.c
--- b/src/backend/libpq/ip.c
*************** pg_foreach_ifaddr(PgIfAddrCallback callb
*** 547,553 ****
int error;
sock = WSASocket(AF_INET, SOCK_DGRAM, 0, 0, 0, 0);
! if (sock == SOCKET_ERROR)
return -1;
while (n_ii < 1024)
--- 547,553 ----
int error;
sock = WSASocket(AF_INET, SOCK_DGRAM, 0, 0, 0, 0);
! if (sock == INVALID_SOCKET)
return -1;
while (n_ii < 1024)
*************** pg_foreach_ifaddr(PgIfAddrCallback callb
*** 670,676 ****
total;
sock = socket(AF_INET, SOCK_DGRAM, 0);
! if (sock == -1)
return -1;
while (n_buffer < 1024 * 100)
--- 670,676 ----
total;
sock = socket(AF_INET, SOCK_DGRAM, 0);
! if (sock == PGINVALID_SOCKET)
return -1;
while (n_buffer < 1024 * 100)
*************** pg_foreach_ifaddr(PgIfAddrCallback callb
*** 711,717 ****
#ifdef HAVE_IPV6
/* We'll need an IPv6 socket too for the SIOCGLIFNETMASK ioctls */
sock6 = socket(AF_INET6, SOCK_DGRAM, 0);
! if (sock6 == -1)
{
free(buffer);
close(sock);
--- 711,717 ----
#ifdef HAVE_IPV6
/* We'll need an IPv6 socket too for the SIOCGLIFNETMASK ioctls */
sock6 = socket(AF_INET6, SOCK_DGRAM, 0);
! if (sock6 == PGINVALID_SOCKET)
{
free(buffer);
close(sock);
*************** pg_foreach_ifaddr(PgIfAddrCallback callb
*** 788,797 ****
char *ptr,
*buffer = NULL;
size_t n_buffer = 1024;
! int sock;
sock = socket(AF_INET, SOCK_DGRAM, 0);
! if (sock == -1)
return -1;
while (n_buffer < 1024 * 100)
--- 788,797 ----
char *ptr,
*buffer = NULL;
size_t n_buffer = 1024;
! pgsocket sock;
sock = socket(AF_INET, SOCK_DGRAM, 0);
! if (sock == PGINVALID_SOCKET)
return -1;
while (n_buffer < 1024 * 100)
diff --git a/src/backend/libpq/pqcomm.c b/src/backend/libpq/pqcomm.c
new file mode 100644
index 1eae183..0179451
*** a/src/backend/libpq/pqcomm.c
--- b/src/backend/libpq/pqcomm.c
*************** StreamServerPort(int family, char *hostN
*** 392,398 ****
break;
}
! if ((fd = socket(addr->ai_family, SOCK_STREAM, 0)) < 0)
{
ereport(LOG,
(errcode_for_socket_access(),
--- 392,398 ----
break;
}
! if ((fd = socket(addr->ai_family, SOCK_STREAM, 0)) == PGINVALID_SOCKET)
{
ereport(LOG,
(errcode_for_socket_access(),
*************** StreamConnection(pgsocket server_fd, Por
*** 632,638 ****
port->raddr.salen = sizeof(port->raddr.addr);
if ((port->sock = accept(server_fd,
(struct sockaddr *) & port->raddr.addr,
! &port->raddr.salen)) < 0)
{
ereport(LOG,
(errcode_for_socket_access(),
--- 632,638 ----
port->raddr.salen = sizeof(port->raddr.addr);
if ((port->sock = accept(server_fd,
(struct sockaddr *) & port->raddr.addr,
! &port->raddr.salen)) == PGINVALID_SOCKET)
{
ereport(LOG,
(errcode_for_socket_access(),
diff --git a/src/backend/port/win32/socket.c b/src/backend/port/win32/socket.c
new file mode 100644
index 4f1099f..adc0e02
*** a/src/backend/port/win32/socket.c
--- b/src/backend/port/win32/socket.c
*************** int
*** 132,138 ****
pgwin32_waitforsinglesocket(SOCKET s, int what, int timeout)
{
static HANDLE waitevent = INVALID_HANDLE_VALUE;
! static SOCKET current_socket = -1;
static int isUDP = 0;
HANDLE events[2];
int r;
--- 132,138 ----
pgwin32_waitforsinglesocket(SOCKET s, int what, int timeout)
{
static HANDLE waitevent = INVALID_HANDLE_VALUE;
! static SOCKET current_socket = INVALID_SOCKET;
static int isUDP = 0;
HANDLE events[2];
int r;
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
new file mode 100644
index bb771a6..b573fd8
*** a/src/backend/postmaster/postmaster.c
--- b/src/backend/postmaster/postmaster.c
*************** ConnCreate(int serverFd)
*** 2169,2175 ****
if (StreamConnection(serverFd, port) != STATUS_OK)
{
! if (port->sock >= 0)
StreamClose(port->sock);
ConnFree(port);
return NULL;
--- 2169,2175 ----
if (StreamConnection(serverFd, port) != STATUS_OK)
{
! if (port->sock != PGINVALID_SOCKET)
StreamClose(port->sock);
ConnFree(port);
return NULL;
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
new file mode 100644
index d53c41f..459e4a4
*** a/src/interfaces/libpq/fe-connect.c
--- b/src/interfaces/libpq/fe-connect.c
*************** keep_going: /* We will come back to
*** 1632,1639 ****
conn->raddr.salen = addr_cur->ai_addrlen;
/* Open a socket */
! conn->sock = socket(addr_cur->ai_family, SOCK_STREAM, 0);
! if (conn->sock < 0)
{
/*
* ignore socket() failure if we have more addresses
--- 1632,1654 ----
conn->raddr.salen = addr_cur->ai_addrlen;
/* Open a socket */
! {
! /*
! * While we use 'pgsocket' as the socket type in the
! * backend, we use 'int' for libpq socket values.
! * This requires us to map PGINVALID_SOCKET to -1
! * on Windows.
! * See http://msdn.microsoft.com/en-us/library/windows/desktop/ms740516%28v=vs.85%29.aspx
! */
! pgsocket sock = socket(addr_cur->ai_family, SOCK_STREAM, 0);
! #ifdef WIN32
! if (sock == PGINVALID_SOCKET)
! conn->sock = -1;
! else
! #endif
! conn->sock = sock;
! }
! if (conn->sock == -1)
{
/*
* ignore socket() failure if we have more addresses
*************** internal_cancel(SockAddr *raddr, int be_
*** 3136,3142 ****
char *errbuf, int errbufsize)
{
int save_errno = SOCK_ERRNO;
! int tmpsock = -1;
char sebuf[256];
int maxlen;
struct
--- 3151,3157 ----
char *errbuf, int errbufsize)
{
int save_errno = SOCK_ERRNO;
! pgsocket tmpsock = PGINVALID_SOCKET;
char sebuf[256];
int maxlen;
struct
*************** internal_cancel(SockAddr *raddr, int be_
*** 3149,3155 ****
* We need to open a temporary connection to the postmaster. Do this with
* only kernel calls.
*/
! if ((tmpsock = socket(raddr->addr.ss_family, SOCK_STREAM, 0)) < 0)
{
strlcpy(errbuf, "PQcancel() -- socket() failed: ", errbufsize);
goto cancel_errReturn;
--- 3164,3170 ----
* We need to open a temporary connection to the postmaster. Do this with
* only kernel calls.
*/
! if ((tmpsock = socket(raddr->addr.ss_family, SOCK_STREAM, 0)) == PGINVALID_SOCKET)
{
strlcpy(errbuf, "PQcancel() -- socket() failed: ", errbufsize);
goto cancel_errReturn;
*************** cancel_errReturn:
*** 3220,3226 ****
maxlen);
strcat(errbuf, "\n");
}
! if (tmpsock >= 0)
closesocket(tmpsock);
SOCK_ERRNO_SET(save_errno);
return FALSE;
--- 3235,3241 ----
maxlen);
strcat(errbuf, "\n");
}
! if (tmpsock != PGINVALID_SOCKET)
closesocket(tmpsock);
SOCK_ERRNO_SET(save_errno);
return FALSE;
*************** PQerrorMessage(const PGconn *conn)
*** 5300,5308 ****
--- 5315,5333 ----
return conn->errorMessage.data;
}
+ /*
+ * In Windows, socket values are unsigned, and an invalid socket value
+ * (INVALID_SOCKET) is ~0, which equals -1 in comparisons (with no compiler
+ * warning). Ideally we would return an unsigned value for PQsocket() on
+ * Windows, but that would cause the function's return value to differ from
+ * Unix, so we just return -1 for invalid sockets.
+ * http://msdn.microsoft.com/en-us/library/windows/desktop/cc507522%28v=vs.85%29.aspx
+ * http://stackoverflow.com/questions/10817252/why-is-invalid-socket-defined-as-0-in-winsock2-h-c
+ */
int
PQsocket(const PGconn *conn)
{
+
if (!conn)
return -1;
return conn->sock;
diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h
new file mode 100644
index 22bbe4a..ee975d4
*** a/src/interfaces/libpq/libpq-int.h
--- b/src/interfaces/libpq/libpq-int.h
*************** struct pg_conn
*** 364,369 ****
--- 364,370 ----
PGnotify *notifyTail; /* newest unreported Notify msg */
/* Connection data */
+ /* See PQconnectPoll() for how we use 'int' and not 'pgsocket'. */
int sock; /* Unix FD for socket, -1 if not connected */
SockAddr laddr; /* Local address */
SockAddr raddr; /* Remote address */
On Tue, Apr 8, 2014 at 11:32 PM, Bruce Momjian <bruce@momjian.us> wrote:
On Sun, Apr 6, 2014 at 11:45:59AM +0530, Amit Kapila wrote:
In fact, this C program compiled by gcc on Debian issues no compiler
warnings and returns 'hello', showing that -1 and ~0 compare as equal:int
main(int argc, char **argv)
{
int i;
unsigned int j;i = -1;
j = ~0;if (i == j)
printf("hello\n");return 0;
}
I have add below code to check it's usage as per PG:
if (j < 0)
printf("hello-1\n");
It doesn't print hello-1, which means that all the check's in code
for <sock_desc> < 0 can have problem.
1.
int
pg_foreach_ifaddr(PgIfAddrCallback callback, void *cb_data)
sock = WSASocket(AF_INET, SOCK_DGRAM, 0, 0, 0, 0);
if (sock == SOCKET_ERROR)Well, the actual problem here is that WSASocket() returns INVALID_SOCKET
per the documentation, not SOCKET_ERROR. I did not use PGINVALID_SOCKET
here because this is Windows-specific code, defining 'sock' as SOCKET.
We could have sock defined as pgsocket, but because this is Windows code
already, it doesn't seem wise to mix portability code in there.
I think it's better to use check like below, just for matter of
consistency with other place
if (sock == INVALID_SOCKET)
With Regards,
Amit Kapila.
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
On Thu, Apr 10, 2014 at 11:05:49AM +0530, Amit Kapila wrote:
On Tue, Apr 8, 2014 at 11:32 PM, Bruce Momjian <bruce@momjian.us> wrote:
On Sun, Apr 6, 2014 at 11:45:59AM +0530, Amit Kapila wrote:
In fact, this C program compiled by gcc on Debian issues no compiler
warnings and returns 'hello', showing that -1 and ~0 compare as equal:int
main(int argc, char **argv)
{
int i;
unsigned int j;i = -1;
j = ~0;if (i == j)
printf("hello\n");return 0;
}I have add below code to check it's usage as per PG:
if (j < 0)
printf("hello-1\n");It doesn't print hello-1, which means that all the check's in code
for <sock_desc> < 0 can have problem.
Ah, yes, good point. This is going to require backpatching then.
1.
int
pg_foreach_ifaddr(PgIfAddrCallback callback, void *cb_data)
sock = WSASocket(AF_INET, SOCK_DGRAM, 0, 0, 0, 0);
if (sock == SOCKET_ERROR)Well, the actual problem here is that WSASocket() returns INVALID_SOCKET
per the documentation, not SOCKET_ERROR. I did not use PGINVALID_SOCKET
here because this is Windows-specific code, defining 'sock' as SOCKET.
We could have sock defined as pgsocket, but because this is Windows code
already, it doesn't seem wise to mix portability code in there.I think it's better to use check like below, just for matter of
consistency with other place
if (sock == INVALID_SOCKET)
Agreed. That is how I have coded the patch.
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ Everyone has their own god. +
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Thu, Apr 10, 2014 at 5:21 PM, Bruce Momjian <bruce@momjian.us> wrote:
On Thu, Apr 10, 2014 at 11:05:49AM +0530, Amit Kapila wrote:
Ah, yes, good point. This is going to require backpatching then.
I also think so.
I think it's better to use check like below, just for matter of
consistency with other place
if (sock == INVALID_SOCKET)Agreed. That is how I have coded the patch.
Sorry, I didn't checked the latest patch before that comment.
I verified that your last patch is fine. Regression test also went fine.
With Regards,
Amit Kapila.
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
On Fri, Apr 11, 2014 at 10:00 AM, Amit Kapila <amit.kapila16@gmail.com> wrote:
On Thu, Apr 10, 2014 at 5:21 PM, Bruce Momjian <bruce@momjian.us> wrote:
On Thu, Apr 10, 2014 at 11:05:49AM +0530, Amit Kapila wrote:
Ah, yes, good point. This is going to require backpatching then.
I also think so.
I think it's better to use check like below, just for matter of
consistency with other place
if (sock == INVALID_SOCKET)Agreed. That is how I have coded the patch.
Sorry, I didn't checked the latest patch before that comment.
I verified that your last patch is fine. Regression test also went fine.
I have noticed small thing which I forgot to mention in previous mail.
I think below added extra line is not required.
int
PQsocket(const PGconn *conn)
{
+
With Regards,
Amit Kapila.
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
On Fri, Apr 11, 2014 at 10:03:08AM +0530, Amit Kapila wrote:
On Fri, Apr 11, 2014 at 10:00 AM, Amit Kapila <amit.kapila16@gmail.com> wrote:
On Thu, Apr 10, 2014 at 5:21 PM, Bruce Momjian <bruce@momjian.us> wrote:
On Thu, Apr 10, 2014 at 11:05:49AM +0530, Amit Kapila wrote:
Ah, yes, good point. This is going to require backpatching then.
I also think so.
I think it's better to use check like below, just for matter of
consistency with other place
if (sock == INVALID_SOCKET)Agreed. That is how I have coded the patch.
Sorry, I didn't checked the latest patch before that comment.
I verified that your last patch is fine. Regression test also went fine.
I have noticed small thing which I forgot to mention in previous mail.
I think below added extra line is not required.int
PQsocket(const PGconn *conn)
{
+
Yes, I saw that yesterday and fixed it. I also did a dry run of
backpatching and only 8.4 had conflicts, so I think we are good there.
(This is like the readdir() fix all over again.)
Once this is applied I will work on changing the libpq socket type to
use portable pgsocket, but I am not planning to backpatch that unless we
find a bug.
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ Everyone has their own god. +
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Bruce Momjian wrote:
Yes, I saw that yesterday and fixed it. I also did a dry run of
backpatching and only 8.4 had conflicts, so I think we are good there.
(This is like the readdir() fix all over again.)Once this is applied I will work on changing the libpq socket type to
use portable pgsocket, but I am not planning to backpatch that unless we
find a bug.
Are we done with this patch?
--
�lvaro Herrera http://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
On Wed, Apr 16, 2014 at 10:34:55AM -0300, Alvaro Herrera wrote:
Bruce Momjian wrote:
Yes, I saw that yesterday and fixed it. I also did a dry run of
backpatching and only 8.4 had conflicts, so I think we are good there.
(This is like the readdir() fix all over again.)Once this is applied I will work on changing the libpq socket type to
use portable pgsocket, but I am not planning to backpatch that unless we
find a bug.Are we done with this patch?
I am about to patch it now. I was going to do it yesterday but was
concerned I wouldn't be online long enough to catch any buildfarm
failure.
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ Everyone has their own god. +
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Fri, Apr 11, 2014 at 08:28:55AM -0400, Bruce Momjian wrote:
On Fri, Apr 11, 2014 at 10:03:08AM +0530, Amit Kapila wrote:
On Fri, Apr 11, 2014 at 10:00 AM, Amit Kapila <amit.kapila16@gmail.com> wrote:
On Thu, Apr 10, 2014 at 5:21 PM, Bruce Momjian <bruce@momjian.us> wrote:
On Thu, Apr 10, 2014 at 11:05:49AM +0530, Amit Kapila wrote:
Ah, yes, good point. This is going to require backpatching then.
I also think so.
I think it's better to use check like below, just for matter of
consistency with other place
if (sock == INVALID_SOCKET)Agreed. That is how I have coded the patch.
Sorry, I didn't checked the latest patch before that comment.
I verified that your last patch is fine. Regression test also went fine.
I have noticed small thing which I forgot to mention in previous mail.
I think below added extra line is not required.int
PQsocket(const PGconn *conn)
{
+Yes, I saw that yesterday and fixed it. I also did a dry run of
backpatching and only 8.4 had conflicts, so I think we are good there.
(This is like the readdir() fix all over again.)
Patch applied back through 9.0. 8.4 didn't have the infrastructure for
a proper fix.
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ Everyone has their own god. +
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Fri, Apr 11, 2014 at 08:28:55AM -0400, Bruce Momjian wrote:
Once this is applied I will work on changing the libpq socket type to
use portable pgsocket, but I am not planning to backpatch that unless we
find a bug.
Attached is a follow up patch which stores socket values in libpq as
pgsocket, rather than int, and maps it to -1 only for the PQsocket()
external return value. In the interest of time, I will apply this later
today, and only to head as it does not fix a bug.
This is the last open item I was working on.
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ Everyone has their own god. +
Attachments:
libpq.difftext/x-diff; charset=us-asciiDownload
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
new file mode 100644
index 51d4de4..90b944a
*** a/src/interfaces/libpq/fe-connect.c
--- b/src/interfaces/libpq/fe-connect.c
*************** pqDropConnection(PGconn *conn)
*** 398,406 ****
/* Drop any SSL state */
pqsecure_close(conn);
/* Close the socket itself */
! if (conn->sock >= 0)
closesocket(conn->sock);
! conn->sock = -1;
/* Discard any unread/unsent data */
conn->inStart = conn->inCursor = conn->inEnd = 0;
conn->outCount = 0;
--- 398,406 ----
/* Drop any SSL state */
pqsecure_close(conn);
/* Close the socket itself */
! if (conn->sock != PGINVALID_SOCKET)
closesocket(conn->sock);
! conn->sock = PGINVALID_SOCKET;
/* Discard any unread/unsent data */
conn->inStart = conn->inCursor = conn->inEnd = 0;
conn->outCount = 0;
*************** keep_going: /* We will come back to
*** 1631,1654 ****
addr_cur->ai_addrlen);
conn->raddr.salen = addr_cur->ai_addrlen;
! /* Open a socket */
! {
! /*
! * While we use 'pgsocket' as the socket type in the
! * backend, we use 'int' for libpq socket values.
! * This requires us to map PGINVALID_SOCKET to -1
! * on Windows.
! * See http://msdn.microsoft.com/en-us/library/windows/desktop/ms740516%28v=vs.85%29.aspx
! */
! pgsocket sock = socket(addr_cur->ai_family, SOCK_STREAM, 0);
! #ifdef WIN32
! if (sock == PGINVALID_SOCKET)
! conn->sock = -1;
! else
! #endif
! conn->sock = sock;
! }
! if (conn->sock == -1)
{
/*
* ignore socket() failure if we have more addresses
--- 1631,1638 ----
addr_cur->ai_addrlen);
conn->raddr.salen = addr_cur->ai_addrlen;
! conn->sock = socket(addr_cur->ai_family, SOCK_STREAM, 0);
! if (conn->sock == PGINVALID_SOCKET)
{
/*
* ignore socket() failure if we have more addresses
*************** makeEmptyPGconn(void)
*** 2717,2723 ****
conn->client_encoding = PG_SQL_ASCII;
conn->std_strings = false; /* unless server says differently */
conn->verbosity = PQERRORS_DEFAULT;
! conn->sock = -1;
conn->auth_req_received = false;
conn->password_needed = false;
conn->dot_pgpass_used = false;
--- 2701,2707 ----
conn->client_encoding = PG_SQL_ASCII;
conn->std_strings = false; /* unless server says differently */
conn->verbosity = PQERRORS_DEFAULT;
! conn->sock = PGINVALID_SOCKET;
conn->auth_req_received = false;
conn->password_needed = false;
conn->dot_pgpass_used = false;
*************** closePGconn(PGconn *conn)
*** 2882,2888 ****
* Note that the protocol doesn't allow us to send Terminate messages
* during the startup phase.
*/
! if (conn->sock >= 0 && conn->status == CONNECTION_OK)
{
/*
* Try to send "close connection" message to backend. Ignore any
--- 2866,2872 ----
* Note that the protocol doesn't allow us to send Terminate messages
* during the startup phase.
*/
! if (conn->sock != PGINVALID_SOCKET && conn->status == CONNECTION_OK)
{
/*
* Try to send "close connection" message to backend. Ignore any
*************** PQgetCancel(PGconn *conn)
*** 3103,3109 ****
if (!conn)
return NULL;
! if (conn->sock < 0)
return NULL;
cancel = malloc(sizeof(PGcancel));
--- 3087,3093 ----
if (!conn)
return NULL;
! if (conn->sock == PGINVALID_SOCKET)
return NULL;
cancel = malloc(sizeof(PGcancel));
*************** PQrequestCancel(PGconn *conn)
*** 3284,3290 ****
if (!conn)
return FALSE;
! if (conn->sock < 0)
{
strlcpy(conn->errorMessage.data,
"PQrequestCancel() -- connection is not open\n",
--- 3268,3274 ----
if (!conn)
return FALSE;
! if (conn->sock == PGINVALID_SOCKET)
{
strlcpy(conn->errorMessage.data,
"PQrequestCancel() -- connection is not open\n",
*************** PQsocket(const PGconn *conn)
*** 5329,5335 ****
{
if (!conn)
return -1;
! return conn->sock;
}
int
--- 5313,5319 ----
{
if (!conn)
return -1;
! return (conn->sock != PGINVALID_SOCKET) ? conn->sock : -1;
}
int
diff --git a/src/interfaces/libpq/fe-exec.c b/src/interfaces/libpq/fe-exec.c
new file mode 100644
index 8ccf6d3..50e4035
*** a/src/interfaces/libpq/fe-exec.c
--- b/src/interfaces/libpq/fe-exec.c
*************** PQfn(PGconn *conn,
*** 2549,2555 ****
/* clear the error string */
resetPQExpBuffer(&conn->errorMessage);
! if (conn->sock < 0 || conn->asyncStatus != PGASYNC_IDLE ||
conn->result != NULL)
{
printfPQExpBuffer(&conn->errorMessage,
--- 2549,2555 ----
/* clear the error string */
resetPQExpBuffer(&conn->errorMessage);
! if (conn->sock == PGINVALID_SOCKET || conn->asyncStatus != PGASYNC_IDLE ||
conn->result != NULL)
{
printfPQExpBuffer(&conn->errorMessage,
diff --git a/src/interfaces/libpq/fe-misc.c b/src/interfaces/libpq/fe-misc.c
new file mode 100644
index a7afd42..cc487b2
*** a/src/interfaces/libpq/fe-misc.c
--- b/src/interfaces/libpq/fe-misc.c
*************** pqReadData(PGconn *conn)
*** 604,610 ****
int someread = 0;
int nread;
! if (conn->sock < 0)
{
printfPQExpBuffer(&conn->errorMessage,
libpq_gettext("connection not open\n"));
--- 604,610 ----
int someread = 0;
int nread;
! if (conn->sock == PGINVALID_SOCKET)
{
printfPQExpBuffer(&conn->errorMessage,
libpq_gettext("connection not open\n"));
*************** pqSendSome(PGconn *conn, int len)
*** 800,806 ****
int remaining = conn->outCount;
int result = 0;
! if (conn->sock < 0)
{
printfPQExpBuffer(&conn->errorMessage,
libpq_gettext("connection not open\n"));
--- 800,806 ----
int remaining = conn->outCount;
int result = 0;
! if (conn->sock == PGINVALID_SOCKET)
{
printfPQExpBuffer(&conn->errorMessage,
libpq_gettext("connection not open\n"));
*************** pqSocketCheck(PGconn *conn, int forRead,
*** 1011,1017 ****
if (!conn)
return -1;
! if (conn->sock < 0)
{
printfPQExpBuffer(&conn->errorMessage,
libpq_gettext("socket not open\n"));
--- 1011,1017 ----
if (!conn)
return -1;
! if (conn->sock == PGINVALID_SOCKET)
{
printfPQExpBuffer(&conn->errorMessage,
libpq_gettext("socket not open\n"));
diff --git a/src/interfaces/libpq/fe-protocol2.c b/src/interfaces/libpq/fe-protocol2.c
new file mode 100644
index f3fddaa..10510b5
*** a/src/interfaces/libpq/fe-protocol2.c
--- b/src/interfaces/libpq/fe-protocol2.c
*************** pqGetline2(PGconn *conn, char *s, int ma
*** 1211,1217 ****
{
int result = 1; /* return value if buffer overflows */
! if (conn->sock < 0 ||
conn->asyncStatus != PGASYNC_COPY_OUT)
{
*s = '\0';
--- 1211,1217 ----
{
int result = 1; /* return value if buffer overflows */
! if (conn->sock == PGINVALID_SOCKET ||
conn->asyncStatus != PGASYNC_COPY_OUT)
{
*s = '\0';
diff --git a/src/interfaces/libpq/fe-protocol3.c b/src/interfaces/libpq/fe-protocol3.c
new file mode 100644
index 47cd7f4..d895589
*** a/src/interfaces/libpq/fe-protocol3.c
--- b/src/interfaces/libpq/fe-protocol3.c
*************** pqGetline3(PGconn *conn, char *s, int ma
*** 1568,1574 ****
{
int status;
! if (conn->sock < 0 ||
(conn->asyncStatus != PGASYNC_COPY_OUT &&
conn->asyncStatus != PGASYNC_COPY_BOTH) ||
conn->copy_is_binary)
--- 1568,1574 ----
{
int status;
! if (conn->sock == PGINVALID_SOCKET ||
(conn->asyncStatus != PGASYNC_COPY_OUT &&
conn->asyncStatus != PGASYNC_COPY_BOTH) ||
conn->copy_is_binary)
diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h
new file mode 100644
index ee975d4..0725c17
*** a/src/interfaces/libpq/libpq-int.h
--- b/src/interfaces/libpq/libpq-int.h
*************** struct pg_conn
*** 365,371 ****
/* Connection data */
/* See PQconnectPoll() for how we use 'int' and not 'pgsocket'. */
! int sock; /* Unix FD for socket, -1 if not connected */
SockAddr laddr; /* Local address */
SockAddr raddr; /* Remote address */
ProtocolVersion pversion; /* FE/BE protocol version in use */
--- 365,371 ----
/* Connection data */
/* See PQconnectPoll() for how we use 'int' and not 'pgsocket'. */
! pgsocket sock; /* FD for socket, PGINVALID_SOCKET if unconnected */
SockAddr laddr; /* Local address */
SockAddr raddr; /* Remote address */
ProtocolVersion pversion; /* FE/BE protocol version in use */
On Wed, Apr 16, 2014 at 11:22:28AM -0400, Bruce Momjian wrote:
On Fri, Apr 11, 2014 at 08:28:55AM -0400, Bruce Momjian wrote:
Once this is applied I will work on changing the libpq socket type to
use portable pgsocket, but I am not planning to backpatch that unless we
find a bug.Attached is a follow up patch which stores socket values in libpq as
pgsocket, rather than int, and maps it to -1 only for the PQsocket()
external return value. In the interest of time, I will apply this later
today, and only to head as it does not fix a bug.This is the last open item I was working on.
Applied. FYI, Joel Jacobson was the initial author of this thread of
patches; report by PVS-Studio.
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ Everyone has their own god. +
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers