]> granicus.if.org Git - postgresql/commitdiff
Fix coding rules violations in walreceiver.c
authorAlvaro Herrera <alvherre@alvh.no-ip.org>
Tue, 3 Oct 2017 12:58:25 +0000 (14:58 +0200)
committerAlvaro Herrera <alvherre@alvh.no-ip.org>
Tue, 3 Oct 2017 12:58:25 +0000 (14:58 +0200)
1. Since commit b1a9bad9e744 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 9ed551e0a4fd 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 64233902d22b 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 1bb2558046cc 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

src/backend/replication/walreceiver.c

index 3474514adcca9ff410616cb16410bdf6e5a1766c..57c305d0e54fa61e25355fae882fe1f0670051f0 100644 (file)
@@ -196,6 +196,7 @@ WalReceiverMain(void)
        bool            first_stream;
        WalRcvData *walrcv = WalRcv;
        TimestampTz last_recv_timestamp;
+       TimestampTz now;
        bool            ping_sent;
        char       *err;
 
@@ -205,6 +206,8 @@ WalReceiverMain(void)
         */
        Assert(walrcv != NULL);
 
+       now = GetCurrentTimestamp();
+
        /*
         * Mark walreceiver as running in shared memory.
         *
@@ -235,6 +238,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 */
@@ -249,7 +253,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);
 
@@ -308,13 +313,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 (;;)
        {
@@ -1390,8 +1395,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];
 
        /* Take a lock to ensure value consistency */
        SpinLockAcquire(&WalRcv->mutex);
@@ -1406,8 +1411,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);
 
        /*