Re: replace IDENTIFY_SYSTEM code in receivelog.c with RunIdentifySystem()

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