Fix assorted resource leaks in new pg_createsubscriber code.

tglsfdc · tglsfdc · commit 91cbb4b492cb · 2024-04-01T13:47:49.000-04:00
Various error paths did not release resources before returning. While it's likely that the program would just exit shortly later, none of the functions in question have summary exit(1) calls, so they should not be assuming that. Ranier Vilela and Tom Lane, per reports from Coverity Discussion: https://postgr.es/m/CAEudQAr2_SZFxB4kXJiL4+2UaNZxUk5UBJtj0oXyJYMGZu-03g@mail.gmail.com
diff --git a/src/bin/pg_basebackup/pg_createsubscriber.c b/src/bin/pg_basebackup/pg_createsubscriber.c
@@ -251,8 +251,8 @@ usage(void)
 static char *
 get_base_conninfo(const char *conninfo, char **dbname)
 {
-	PQExpBuffer buf = createPQExpBuffer();
-	PQconninfoOption *conn_opts = NULL;
+	PQExpBuffer buf;
+	PQconninfoOption *conn_opts;
 	PQconninfoOption *conn_opt;
 	char	   *errmsg = NULL;
 	char	   *ret;
@@ -262,9 +262,11 @@ get_base_conninfo(const char *conninfo, char **dbname)
 	if (conn_opts == NULL)
 	{
 		pg_log_error("could not parse connection string: %s", errmsg);
+		PQfreemem(errmsg);
 		return NULL;
 	}
 
+	buf = createPQExpBuffer();
 	i = 0;
 	for (conn_opt = conn_opts; conn_opt->keyword != NULL; conn_opt++)
 	{
@@ -497,9 +499,10 @@ connect_database(const char *conninfo, bool exit_on_error)
 	{
 		pg_log_error("connection to database failed: %s",
 					 PQerrorMessage(conn));
+		PQfinish(conn);
+
 		if (exit_on_error)
 			exit(1);
-
 		return NULL;
 	}
 
@@ -509,9 +512,11 @@ connect_database(const char *conninfo, bool exit_on_error)
 	{
 		pg_log_error("could not clear search_path: %s",
 					 PQresultErrorMessage(res));
+		PQclear(res);
+		PQfinish(conn);
+
 		if (exit_on_error)
 			exit(1);
-
 		return NULL;
 	}
 	PQclear(res);
@@ -941,6 +946,8 @@ check_publisher(const struct LogicalRepInfo *dbinfo)
 		failed = true;
 	}
 
+	pg_free(wal_level);
+
 	if (failed)
 		exit(1);
 }
@@ -1209,6 +1216,8 @@ create_logical_replication_slot(PGconn *conn, struct LogicalRepInfo *dbinfo)
 			pg_log_error("could not create replication slot \"%s\" on database \"%s\": %s",
 						 slot_name, dbinfo->dbname,
 						 PQresultErrorMessage(res));
+			PQclear(res);
+			destroyPQExpBuffer(str);
 			return NULL;
 		}
 

Original file line number	Diff line number	Diff line change
`@@ -251,8 +251,8 @@ usage(void)`
`251`	`251`	`static char *`
`252`	`252`	`get_base_conninfo(const char conninfo, char *dbname)`
`253`	`253`	`{`
`254`		`- PQExpBuffer buf = createPQExpBuffer();`
`255`		`- PQconninfoOption *conn_opts = NULL;`
	`254`	`+ PQExpBuffer buf;`
	`255`	`+ PQconninfoOption *conn_opts;`
`256`	`256`	`PQconninfoOption *conn_opt;`
`257`	`257`	`char *errmsg = NULL;`
`258`	`258`	`char *ret;`
`@@ -262,9 +262,11 @@ get_base_conninfo(const char conninfo, char *dbname)`
`262`	`262`	`if (conn_opts == NULL)`
`263`	`263`	`{`
`264`	`264`	`pg_log_error("could not parse connection string: %s", errmsg);`
	`265`	`+ PQfreemem(errmsg);`
`265`	`266`	`return NULL;`
`266`	`267`	`}`
`267`	`268`
	`269`	`+ buf = createPQExpBuffer();`
`268`	`270`	`i = 0;`
`269`	`271`	`for (conn_opt = conn_opts; conn_opt->keyword != NULL; conn_opt++)`
`270`	`272`	`{`
`@@ -497,9 +499,10 @@ connect_database(const char *conninfo, bool exit_on_error)`
`497`	`499`	`{`
`498`	`500`	`pg_log_error("connection to database failed: %s",`
`499`	`501`	`PQerrorMessage(conn));`
	`502`	`+ PQfinish(conn);`
	`503`	`+`
`500`	`504`	`if (exit_on_error)`
`501`	`505`	`exit(1);`
`502`		`-`
`503`	`506`	`return NULL;`
`504`	`507`	`}`
`505`	`508`
`@@ -509,9 +512,11 @@ connect_database(const char *conninfo, bool exit_on_error)`
`509`	`512`	`{`
`510`	`513`	`pg_log_error("could not clear search_path: %s",`
`511`	`514`	`PQresultErrorMessage(res));`
	`515`	`+ PQclear(res);`
	`516`	`+ PQfinish(conn);`
	`517`	`+`
`512`	`518`	`if (exit_on_error)`
`513`	`519`	`exit(1);`
`514`		`-`
`515`	`520`	`return NULL;`
`516`	`521`	`}`
`517`	`522`	`PQclear(res);`
`@@ -941,6 +946,8 @@ check_publisher(const struct LogicalRepInfo *dbinfo)`
`941`	`946`	`failed = true;`
`942`	`947`	`}`
`943`	`948`
	`949`	`+ pg_free(wal_level);`
	`950`	`+`
`944`	`951`	`if (failed)`
`945`	`952`	`exit(1);`
`946`	`953`	`}`
`@@ -1209,6 +1216,8 @@ create_logical_replication_slot(PGconn conn, struct LogicalRepInfo dbinfo)`
`1209`	`1216`	`pg_log_error("could not create replication slot \"%s\" on database \"%s\": %s",`
`1210`	`1217`	`slot_name, dbinfo->dbname,`
`1211`	`1218`	`PQresultErrorMessage(res));`
	`1219`	`+ PQclear(res);`
	`1220`	`+ destroyPQExpBuffer(str);`
`1212`	`1221`	`return NULL;`
`1213`	`1222`	`}`
`1214`	`1223`