Fix coding rules violations in walreceiver.c

alvherre · alvherre · commit 86076395ef81 · 2017-10-03T14:58:25.000+02:00
1. Since commit b1a9bad we had pstrdup() inside a spinlock-protected critical section; reported by Andreas Seltenreich. Turn those into strlcpy() to stack-allocated variables instead. Backpatch to 9.6. 2. Since commit 9ed551e we had a pfree() uselessly inside a spinlock-protected critical section. Tom Lane noticed in code review. Move down. Backpatch to 9.6. 3. Since commit 6423390 we had GetCurrentTimestamp() (a kernel call) inside a spinlock-protected critical section. Tom Lane noticed in code review. Move it up. Backpatch to 9.2. 4. Since commit 1bb2558 we did elog(PANIC) while holding spinlock. Tom Lane noticed in code review. Release spinlock before dying. Backpatch to 9.2. Discussion: https://postgr.es/m/87h8vhtgj2.fsf@ansel.ydns.eu
diff --git a/src/backend/replication/walreceiver.c b/src/backend/replication/walreceiver.c
@@ -201,6 +201,7 @@ WalReceiverMain(void)
 	bool		first_stream;
 	WalRcvData *walrcv = WalRcv;
 	TimestampTz last_recv_timestamp;
+	TimestampTz now;
 	bool		ping_sent;
 
 	/*
@@ -209,6 +210,8 @@ WalReceiverMain(void)
 	 */
 	Assert(walrcv != NULL);
 
+	now = GetCurrentTimestamp();
+
 	/*
 	 * Mark walreceiver as running in shared memory.
 	 *
@@ -239,6 +242,7 @@ WalReceiverMain(void)
 		case WALRCV_RESTARTING:
 		default:
 			/* Shouldn't happen */
+			SpinLockRelease(&walrcv->mutex);
 			elog(PANIC, "walreceiver still running according to shared memory state");
 	}
 	/* Advertise our PID so that the startup process can kill us */
@@ -253,7 +257,8 @@ WalReceiverMain(void)
 	startpointTLI = walrcv->receiveStartTLI;
 
 	/* Initialise to a sanish value */
-	walrcv->lastMsgSendTime = walrcv->lastMsgReceiptTime = walrcv->latestWalEndTime = GetCurrentTimestamp();
+	walrcv->lastMsgSendTime =
+		walrcv->lastMsgReceiptTime = walrcv->latestWalEndTime = now;
 
 	SpinLockRelease(&walrcv->mutex);
 
@@ -317,13 +322,13 @@ WalReceiverMain(void)
 	SpinLockAcquire(&walrcv->mutex);
 	memset(walrcv->conninfo, 0, MAXCONNINFO);
 	if (tmp_conninfo)
-	{
 		strlcpy((char *) walrcv->conninfo, tmp_conninfo, MAXCONNINFO);
-		pfree(tmp_conninfo);
-	}
 	walrcv->ready_to_display = true;
 	SpinLockRelease(&walrcv->mutex);
 
+	if (tmp_conninfo)
+		pfree(tmp_conninfo);
+
 	first_stream = true;
 	for (;;)
 	{
@@ -1349,8 +1354,8 @@ pg_stat_get_wal_receiver(PG_FUNCTION_ARGS)
 	TimestampTz last_receipt_time;
 	XLogRecPtr	latest_end_lsn;
 	TimestampTz latest_end_time;
-	char	   *slotname;
-	char	   *conninfo;
+	char		slotname[NAMEDATALEN];
+	char		conninfo[MAXCONNINFO];
 
 	/*
 	 * No WAL receiver (or not ready yet), just return a tuple with NULL
@@ -1377,8 +1382,8 @@ pg_stat_get_wal_receiver(PG_FUNCTION_ARGS)
 	last_receipt_time = walrcv->lastMsgReceiptTime;
 	latest_end_lsn = walrcv->latestWalEnd;
 	latest_end_time = walrcv->latestWalEndTime;
-	slotname = pstrdup(walrcv->slotname);
-	conninfo = pstrdup(walrcv->conninfo);
+	strlcpy(slotname, (char *) walrcv->slotname, sizeof(slotname));
+	strlcpy(conninfo, (char *) walrcv->conninfo, sizeof(conninfo));
 	SpinLockRelease(&walrcv->mutex);
 
 	/* Fetch values */
