Re: replace IDENTIFY_SYSTEM code in receivelog.c with RunIdentifySystem() - Mailing list pgsql-hackers

From Michael Paquier
Subject Re: replace IDENTIFY_SYSTEM code in receivelog.c with RunIdentifySystem()
Date
Msg-id YSx6shCZ70EV/Lk2@paquier.xyz
Whole thread Raw
In response to replace IDENTIFY_SYSTEM code in receivelog.c with RunIdentifySystem()  (Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>)
Responses Re: replace IDENTIFY_SYSTEM code in receivelog.c with RunIdentifySystem()
List pgsql-hackers
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

pgsql-hackers by date:

Previous
From: Masahiko Sawada
Date:
Subject: Re: Separate out FileSet from SharedFileSet (was Re: pgsql: pgstat: Bring up pgstat in BaseInit() to fix uninitialized use o)
Next
From: Amit Kapila
Date:
Subject: Re: Added schema level support for publication.