[COMMITTERS] Re: pgsql: Refactor replication connection code of various pg_basebackup ut

[Stephen Frost]

All,

* Andres Freund (and...@anarazel.de) wrote:
> Refactor replication connection code of various pg_basebackup utilities.
> 
> Move some more code to manage replication connection command to
> streamutil.c. A later patch will introduce replication slot via
> pg_receivexlog and this avoid duplicating relevant code between
> pg_receivexlog and pg_recvlogical.

Coverity is complaining that the new Create/DropReplicatSlot functions
leak, which is technically correct (though the cases where that happens
end up being more-or-les immediate-exit cases anyway).  In any case,
here's a patch to clean up things prior to return.

        Thanks,

                Stephen

diff --git a/src/bin/pg_basebackup/streamutil.c b/src/bin/pg_basebackup/streamutil.c
new file mode 100644
index 2f4bac9..d3694da
*** a/src/bin/pg_basebackup/streamutil.c
--- b/src/bin/pg_basebackup/streamutil.c
*************** RunIdentifySystem(PGconn *conn, char **s
*** 251,256 ****
--- 251,258 ----
  	{
  		fprintf(stderr, _("%s: could not send replication command \"%s\": %s"),
  				progname, "IDENTIFY_SYSTEM", PQerrorMessage(conn));
+ 
+ 		PQclear(res);
  		return false;
  	}
  	if (PQntuples(res) != 1 || PQnfields(res) < 3)
*************** RunIdentifySystem(PGconn *conn, char **s
*** 258,263 ****
--- 260,267 ----
  		fprintf(stderr,
  				_("%s: could not identify system: got %d rows and %d fields, expected %d rows and %d or more fields\n"),
  				progname, PQntuples(res), PQnfields(res), 1, 3);
+ 
+ 		PQclear(res);
  		return false;
  	}
  
*************** RunIdentifySystem(PGconn *conn, char **s
*** 277,282 ****
--- 281,288 ----
  			fprintf(stderr,
  					_("%s: could not parse transaction log location \"%s\"\n"),
  					progname, PQgetvalue(res, 0, 2));
+ 
+ 			PQclear(res);
  			return false;
  		}
  		*startpos = ((uint64) hi) << 32 | lo;
*************** CreateReplicationSlot(PGconn *conn, cons
*** 331,336 ****
--- 337,345 ----
  	{
  		fprintf(stderr, _("%s: could not send replication command \"%s\": %s"),
  				progname, query->data, PQerrorMessage(conn));
+ 
+ 		destroyPQExpBuffer(query);
+ 		PQclear(res);
  		return false;
  	}
  
*************** CreateReplicationSlot(PGconn *conn, cons
*** 340,345 ****
--- 349,357 ----
  				_("%s: could not create replication slot \"%s\": got %d rows and %d fields, expected %d rows and %d fields\n"),
  				progname, slot_name,
  				PQntuples(res), PQnfields(res), 1, 4);
+ 
+ 		destroyPQExpBuffer(query);
+ 		PQclear(res);
  		return false;
  	}
  
*************** CreateReplicationSlot(PGconn *conn, cons
*** 353,363 ****
--- 365,379 ----
  			fprintf(stderr,
  					_("%s: could not parse transaction log location \"%s\"\n"),
  					progname, PQgetvalue(res, 0, 1));
+ 
+ 			destroyPQExpBuffer(query);
+ 			PQclear(res);
  			return false;
  		}
  		*startpos = ((uint64) hi) << 32 | lo;
  	}
  
+ 	destroyPQExpBuffer(query);
  	PQclear(res);
  	return true;
  }
*************** DropReplicationSlot(PGconn *conn, const
*** 384,389 ****
--- 400,408 ----
  	{
  		fprintf(stderr, _("%s: could not send replication command \"%s\": %s"),
  				progname, query->data, PQerrorMessage(conn));
+ 
+ 		destroyPQExpBuffer(query);
+ 		PQclear(res);
  		return false;
  	}
  
*************** DropReplicationSlot(PGconn *conn, const
*** 393,398 ****
--- 412,420 ----
  				_("%s: could not drop replication slot \"%s\": got %d rows and %d fields, expected %d rows and %d fields\n"),
  				progname, slot_name,
  				PQntuples(res), PQnfields(res), 0, 0);
+ 
+ 		destroyPQExpBuffer(query);
+ 		PQclear(res);
  		return false;
  	}
  

signature.asc
Description: Digital signature