On Mon, Feb 15, 2016 at 3:20 PM, Michael Paquier
<michael.paqu...@gmail.com> wrote:
> On Sat, Feb 13, 2016 at 6:25 AM, Alvaro Herrera <alvhe...@alvh.no-ip.org> 
> wrote:
>> I noticed that pgbench calls FD_ISSET on a socket returned by
>> PQsocket() without first checking that it's not invalid.  I don't think
>> there's a real problem here because the socket was already checked a few
>> lines above, but I think applying the same coding pattern to both places
>> is cleaner.
>>
>> Any objections to changing it like this?  I'd probably backpatch to 9.5,
>> but no further (even though this pattern actually appears all the way
>> back.)
>
> Not really, +1 for consistency here, and this makes the code clearer.
>
> Different issues in the same area:
> 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?

With a patch, that's even better.
-- 
Michael
diff --git a/src/bin/pg_basebackup/pg_recvlogical.c b/src/bin/pg_basebackup/pg_recvlogical.c
index 832f9f9..b4325b0 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: bad socket: \"%s\"\n"),
+						progname, strerror(errno));
+				goto error;
+			}
+
 			FD_ZERO(&input_mask);
 			FD_SET(PQsocket(conn), &input_mask);
 
diff --git a/src/bin/scripts/vacuumdb.c b/src/bin/scripts/vacuumdb.c
index c6afcd5..e81123f 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: bad socket: %s\n"), progname,
+				strerror(errno));
+		exit(1);
+	}
 }
 
 static void
diff --git a/src/test/isolation/isolationtester.c b/src/test/isolation/isolationtester.c
index 0a9d25c..3e13a39 100644
--- a/src/test/isolation/isolationtester.c
+++ b/src/test/isolation/isolationtester.c
@@ -720,6 +720,12 @@ try_complete_step(Step *step, int flags)
 	PGresult   *res;
 	bool		canceled = false;
 
+	if (sock < 0)
+	{
+		fprintf(stderr, "bad socket: %s\n", strerror(errno));
+		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

Reply via email to