On Sun, Mar 6, 2016 at 12:52 PM, Alvaro Herrera
<alvhe...@2ndquadrant.com> wrote:
> Peter Eisentraut wrote:
>> On 2/17/16 10:52 PM, Michael Paquier wrote:
>> > On Thu, Feb 18, 2016 at 1:58 AM, Alvaro Herrera
>> > <alvhe...@2ndquadrant.com> wrote:
>> >> Michael Paquier wrote:
>> >>> Hi all,
>> >>>
>> >>> After looking at Alvaro's message mentioning the handling of
>> >>> PQsocket() for invalid sockets, I just had a look by curiosity at
>> >>> other calls of this routine, and found a couple of issues:
>> >>> 1) In vacuumdb.c, init_slot() does not check for the return value of
>> >>> PQsocket():
>> >>> slot->sock = PQsocket(conn);
>> >>> 2) In isolationtester.c, try_complete_step() should do the same.
>> >>> 3) In pg_recvlogical.c for StreamLogicalLog() I am spotting the same
>> >>> problem.
>> >>> I guess those ones should be fixed as well, no?
>> >>
>> >> I patched pgbench to use PQerrorMessage rather than strerror(errno). I
>> >> think your patch should do the same.
>> >
>> > OK, this looks like a good idea. I would suggest doing the same in
>> > receivelog.c then.
>>
>> Let's make the error messages consistent as "invalid socket". "bad
>> socket" isn't really our style, and pg_basebackup saying "socket not
>> open" is just plain incorrect.
>
> You're completely right. Let's patch pgbench that way too.
Here is v3 then, switching to "invalid socket" for those error
messages. There are two extra messages in fe-misc.c and
libpqwalreceiver.c that need a rewording that I have detected as well.
--
Michael
diff --git a/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c b/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
index f670957..4ee4d71 100644
--- a/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
+++ b/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
@@ -331,7 +331,7 @@ libpq_select(int timeout_ms)
if (PQsocket(streamConn) < 0)
ereport(ERROR,
(errcode_for_socket_access(),
- errmsg("socket not open")));
+ errmsg("invalid socket: %s", PQerrorMessage(streamConn))));
/* We use poll(2) if available, otherwise select(2) */
{
diff --git a/src/bin/pg_basebackup/pg_recvlogical.c b/src/bin/pg_basebackup/pg_recvlogical.c
index 832f9f9..6d12705 100644
--- a/src/bin/pg_basebackup/pg_recvlogical.c
+++ b/src/bin/pg_basebackup/pg_recvlogical.c
@@ -360,6 +360,14 @@ StreamLogicalLog(void)
struct timeval timeout;
struct timeval *timeoutptr = NULL;
+ if (PQsocket(conn) < 0)
+ {
+ fprintf(stderr,
+ _("%s: invalid socket: %s"),
+ progname, PQerrorMessage(conn));
+ goto error;
+ }
+
FD_ZERO(&input_mask);
FD_SET(PQsocket(conn), &input_mask);
diff --git a/src/bin/pg_basebackup/receivelog.c b/src/bin/pg_basebackup/receivelog.c
index 6d7e635..01c42fc 100644
--- a/src/bin/pg_basebackup/receivelog.c
+++ b/src/bin/pg_basebackup/receivelog.c
@@ -956,7 +956,8 @@ CopyStreamPoll(PGconn *conn, long timeout_ms)
if (PQsocket(conn) < 0)
{
- fprintf(stderr, _("%s: socket not open"), progname);
+ fprintf(stderr, _("%s: invalid socket: %s"), progname,
+ PQerrorMessage(conn));
return -1;
}
diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index 8b0b17a..92df750 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -3797,7 +3797,7 @@ threadRun(void *arg)
sock = PQsocket(st->con);
if (sock < 0)
{
- fprintf(stderr, "bad socket: %s", PQerrorMessage(st->con));
+ fprintf(stderr, "invalid socket: %s", PQerrorMessage(st->con));
goto done;
}
@@ -3867,7 +3867,8 @@ threadRun(void *arg)
if (sock < 0)
{
- fprintf(stderr, "bad socket: %s", PQerrorMessage(st->con));
+ fprintf(stderr, "invalid socket: %s",
+ PQerrorMessage(st->con));
goto done;
}
if (FD_ISSET(sock, &input_mask) ||
diff --git a/src/bin/scripts/vacuumdb.c b/src/bin/scripts/vacuumdb.c
index c6afcd5..b673be8 100644
--- a/src/bin/scripts/vacuumdb.c
+++ b/src/bin/scripts/vacuumdb.c
@@ -70,7 +70,7 @@ static void DisconnectDatabase(ParallelSlot *slot);
static int select_loop(int maxFd, fd_set *workerset, bool *aborting);
-static void init_slot(ParallelSlot *slot, PGconn *conn);
+static void init_slot(ParallelSlot *slot, PGconn *conn, const char *progname);
static void help(const char *progname);
@@ -421,14 +421,14 @@ vacuum_one_database(const char *dbname, vacuumingOptions *vacopts,
* array contains the connection.
*/
slots = (ParallelSlot *) pg_malloc(sizeof(ParallelSlot) * concurrentCons);
- init_slot(slots, conn);
+ init_slot(slots, conn, progname);
if (parallel)
{
for (i = 1; i < concurrentCons; i++)
{
conn = connectDatabase(dbname, host, port, username, prompt_password,
progname, false, true);
- init_slot(slots + i, conn);
+ init_slot(slots + i, conn, progname);
}
}
@@ -917,11 +917,18 @@ select_loop(int maxFd, fd_set *workerset, bool *aborting)
}
static void
-init_slot(ParallelSlot *slot, PGconn *conn)
+init_slot(ParallelSlot *slot, PGconn *conn, const char *progname)
{
slot->connection = conn;
slot->isFree = true;
slot->sock = PQsocket(conn);
+
+ if (slot->sock < 0)
+ {
+ fprintf(stderr, _("%s: invalid socket: %s"), progname,
+ PQerrorMessage(conn));
+ exit(1);
+ }
}
static void
diff --git a/src/interfaces/libpq/fe-misc.c b/src/interfaces/libpq/fe-misc.c
index 30cee7f..32da8ca 100644
--- a/src/interfaces/libpq/fe-misc.c
+++ b/src/interfaces/libpq/fe-misc.c
@@ -1058,7 +1058,7 @@ pqSocketCheck(PGconn *conn, int forRead, int forWrite, time_t end_time)
if (conn->sock == PGINVALID_SOCKET)
{
printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("socket not open\n"));
+ libpq_gettext("invalid socket\n"));
return -1;
}
diff --git a/src/test/isolation/isolationtester.c b/src/test/isolation/isolationtester.c
index 6461ae8..2969ce9 100644
--- a/src/test/isolation/isolationtester.c
+++ b/src/test/isolation/isolationtester.c
@@ -705,6 +705,12 @@ try_complete_step(Step *step, int flags)
PGresult *res;
bool canceled = false;
+ if (sock < 0)
+ {
+ fprintf(stderr, "invalid socket: %s", PQerrorMessage(conn));
+ exit_nicely();
+ }
+
gettimeofday(&start_time, NULL);
FD_ZERO(&read_set);
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
