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 <br...@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ Everyone has their own god. +
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 */
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
