On Mon, Aug 30, 2021 at 11:00:40AM +0530, Bharath Rupireddy wrote:
> 1) ReceiveXlogStream in receivelog.c has a duplicate code to execute
> IDENTIFY_SYSTEM replication command on the server which can be
> replaced with RunIdentifySystem().

I have looked at that.

> 2) bool returning ReceiveXlogStream() in pg_receivewal.c is being used
> without type-casting its return return value which might generate a
> warning with some compilers. This kind of type-casting is more common
> in other places in the postgres code base.

This is usually a pattern used for Coverity, to hint it that we don't
care about the error code in a given code path.  IMV, that's not
something to bother about for older code.

> Attaching a patch to fix the above. Thoughts?

The original refactoring of IDENTIFY_SYSTEM is from 0c013e08, and it
feels like I just missed ReceiveXlogStream().  What you have here is
an improvement.

+       if (!RunIdentifySystem(conn, &sysidentifier, &servertli, NULL, NULL))
        {
-           pg_log_error("could not send replication command \"%s\": %s",
-                        "IDENTIFY_SYSTEM", PQerrorMessage(conn));
-           PQclear(res);
+           pg_free(sysidentifier);
            return false;

Here you want to free sysidentifier only if it has been set, and
RunIdentifySystem() may fail before doing that, so you should assign
NULL to sysidentifier when it is declared.
--
Michael

Attachment: signature.asc
Description: PGP signature

Reply via email to