Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitfd602f2

Browse files
committed
Clean up impenetrable logic in pg_basebackup/receivelog.c.
Coverity complained about possible double free of HandleCopyStream's"copybuf". AFAICS it's mistaken, but it is easy to see why it'sconfused, because management of that buffer is impossibly confusing.It's unreasonable that HandleEndOfCopyStream frees the buffer in somecases 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 actualowner of that variable, HandleCopyStream. This results in one morePQfreemem 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.
1 parentfcd77a6 commitfd602f2

File tree

1 file changed

+14
-7
lines changed

1 file changed

+14
-7
lines changed

‎src/bin/pg_basebackup/receivelog.c

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -803,6 +803,10 @@ HandleCopyStream(PGconn *conn, StreamCtl *stream,
803803
sleeptime=CalculateCopyStreamSleeptime(now,stream->standby_message_timeout,
804804
last_status);
805805

806+
/* Done with any prior message */
807+
PQfreemem(copybuf);
808+
copybuf=NULL;
809+
806810
r=CopyStreamReceive(conn,sleeptime,stream->stop_socket,&copybuf);
807811
while (r!=0)
808812
{
@@ -814,8 +818,8 @@ HandleCopyStream(PGconn *conn, StreamCtl *stream,
814818

815819
if (res==NULL)
816820
gotoerror;
817-
else
818-
returnres;
821+
PQfreemem(copybuf);
822+
returnres;
819823
}
820824

821825
/* Check the message type. */
@@ -844,6 +848,10 @@ HandleCopyStream(PGconn *conn, StreamCtl *stream,
844848
gotoerror;
845849
}
846850

851+
/* Done with that message */
852+
PQfreemem(copybuf);
853+
copybuf=NULL;
854+
847855
/*
848856
* Process the received data, and any subsequent data we can read
849857
* without blocking.
@@ -920,8 +928,8 @@ CopyStreamPoll(PGconn *conn, long timeout_ms, pgsocket stop_socket)
920928
* maximum of 'timeout' ms.
921929
*
922930
* If data was received, returns the length of the data. *buffer is set to
923-
* point to a buffer holding the received message. Thebuffer is only valid
924-
*until thenext CopyStreamReceive call.
931+
* point to a buffer holding the received message. Thecaller must eventually
932+
*free thebuffer with PQfreemem().
925933
*
926934
* Returns 0 if no data was available within timeout, or if wait was
927935
* interrupted by signal or stop_socket input.
@@ -934,8 +942,8 @@ CopyStreamReceive(PGconn *conn, long timeout, pgsocket stop_socket,
934942
char*copybuf=NULL;
935943
intrawlen;
936944

937-
PQfreemem(*buffer);
938-
*buffer=NULL;
945+
/* Caller should have cleared any priorbuffer */
946+
Assert(*buffer==NULL);
939947

940948
/* Try to receive a CopyData message */
941949
rawlen=PQgetCopyData(conn,&copybuf,1);
@@ -1198,7 +1206,6 @@ HandleEndOfCopyStream(PGconn *conn, StreamCtl *stream, char *copybuf,
11981206
}
11991207
still_sending= false;
12001208
}
1201-
PQfreemem(copybuf);
12021209
*stoppos=blockpos;
12031210
returnres;
12041211
}

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp