database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-06-30 21:42:05 +03:00
Code Activity

Clean up impenetrable logic in pg_basebackup/receivelog.c.

Browse Source 
Coverity complained about possible double free of HandleCopyStream's
"copybuf".  AFAICS it's mistaken, but it is easy to see why it's
confused, because management of that buffer is impossibly confusing.
It's unreasonable that HandleEndOfCopyStream frees the buffer in some
cases but not others, updates the caller's state for that in no case,
and has not a single comment about how complicated that makes things.

Let's put all the responsibility for freeing copybuf in the actual
owner of that variable, HandleCopyStream.  This results in one more
PQfreemem call than before, but the logic is far easier to follow,
both for humans and machines.

Since this isn't (quite) actually broken, no back-patch.
This commit is contained in:
Tom Lane
2025-02-12 16:07:23 -05:00
parent fcd77a6873
commit fd602f29c1
1 changed files with 14 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files

21
src/bin/pg_basebackup/receivelog.c
View File

@ -803,6 +803,10 @@ HandleCopyStream(PGconn *conn, StreamCtl *stream,
sleeptime = CalculateCopyStreamSleeptime(now, stream->standby_message_timeout, sleeptime = CalculateCopyStreamSleeptime(now, stream->standby_message_timeout,
last_status); last_status);
/* Done with any prior message */
PQfreemem(copybuf);
copybuf = NULL;
r = CopyStreamReceive(conn, sleeptime, stream->stop_socket, &copybuf); r = CopyStreamReceive(conn, sleeptime, stream->stop_socket, &copybuf);
while (r != 0) while (r != 0)
{ {
@ -814,8 +818,8 @@ HandleCopyStream(PGconn *conn, StreamCtl *stream,
if (res == NULL) if (res == NULL)
goto error; goto error;
else PQfreemem(copybuf);
return res; return res;
} }
/* Check the message type. */ /* Check the message type. */
@ -844,6 +848,10 @@ HandleCopyStream(PGconn *conn, StreamCtl *stream,
goto error; goto error;
} }
/* Done with that message */
PQfreemem(copybuf);
copybuf = NULL;
/* /*
* Process the received data, and any subsequent data we can read * Process the received data, and any subsequent data we can read
* without blocking. * without blocking.
@ -920,8 +928,8 @@ CopyStreamPoll(PGconn *conn, long timeout_ms, pgsocket stop_socket)
* maximum of 'timeout' ms. * maximum of 'timeout' ms.
* *
* If data was received, returns the length of the data. *buffer is set to * If data was received, returns the length of the data. *buffer is set to
* point to a buffer holding the received message. The buffer is only valid * point to a buffer holding the received message. The caller must eventually
* until the next CopyStreamReceive call. * free the buffer with PQfreemem().
* *
* Returns 0 if no data was available within timeout, or if wait was * Returns 0 if no data was available within timeout, or if wait was
* interrupted by signal or stop_socket input. * interrupted by signal or stop_socket input.
@ -934,8 +942,8 @@ CopyStreamReceive(PGconn *conn, long timeout, pgsocket stop_socket,
char *copybuf = NULL; char *copybuf = NULL;
int rawlen; int rawlen;
PQfreemem(*buffer); /* Caller should have cleared any prior buffer */
*buffer = NULL; Assert(*buffer == NULL);
/* Try to receive a CopyData message */ /* Try to receive a CopyData message */
rawlen = PQgetCopyData(conn, &copybuf, 1); rawlen = PQgetCopyData(conn, &copybuf, 1);
@ -1198,7 +1206,6 @@ HandleEndOfCopyStream(PGconn *conn, StreamCtl *stream, char *copybuf,
} }
still_sending = false; still_sending = false;
} }
PQfreemem(copybuf);
*stoppos = blockpos; *stoppos = blockpos;
return res; return res;
} }