Remove redundant null pointer checks before PQclear and PQconninfoFree

petere · petere · commit 5faef9d58201 · 2022-07-03T20:11:05.000+02:00
These functions already had the free()-like behavior of handling null pointers as a no-op. But it wasn't documented, so add it explicitly to the documentation, too. Discussion: https://www.postgresql.org/message-id/flat/dac5d2d0-98f5-94d9-8e69-46da2413593d%40enterprisedb.com
diff --git a/contrib/dblink/dblink.c b/contrib/dblink/dblink.c
@@ -157,8 +157,7 @@ dblink_res_internalerror(PGconn *conn, PGresult *res, const char *p2)
 {
 	char	   *msg = pchomp(PQerrorMessage(conn));
 
-	if (res)
-		PQclear(res);
+	PQclear(res);
 	elog(ERROR, "%s: %s", p2, msg);
 }
 
@@ -2756,8 +2755,7 @@ dblink_res_error(PGconn *conn, const char *conname, PGresult *res,
 	 * leaking all the strings too, but those are in palloc'd memory that will
 	 * get cleaned up eventually.
 	 */
-	if (res)
-		PQclear(res);
+	PQclear(res);
 
 	/*
 	 * Format the basic errcontext string.  Below, we'll add on something
diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c
@@ -2790,8 +2790,7 @@ postgresEndDirectModify(ForeignScanState *node)
 		return;
 
 	/* Release PGresult */
-	if (dmstate->result)
-		PQclear(dmstate->result);
+	PQclear(dmstate->result);
 
 	/* Release remote connection */
 	ReleaseConnection(dmstate->conn);
@@ -3604,8 +3603,7 @@ get_remote_estimate(const char *sql, PGconn *conn,
 	}
 	PG_FINALLY();
 	{
-		if (res)
-			PQclear(res);
+		PQclear(res);
 	}
 	PG_END_TRY();
 }
@@ -3853,8 +3851,7 @@ fetch_more_data(ForeignScanState *node)
 	}
 	PG_FINALLY();
 	{
-		if (res)
-			PQclear(res);
+		PQclear(res);
 	}
 	PG_END_TRY();
 
@@ -4338,8 +4335,7 @@ store_returning_result(PgFdwModifyState *fmstate,
 	}
 	PG_CATCH();
 	{
-		if (res)
-			PQclear(res);
+		PQclear(res);
 		PG_RE_THROW();
 	}
 	PG_END_TRY();
@@ -4627,8 +4623,7 @@ get_returning_data(ForeignScanState *node)
 		}
 		PG_CATCH();
 		{
-			if (dmstate->result)
-				PQclear(dmstate->result);
+			PQclear(dmstate->result);
 			PG_RE_THROW();
 		}
 		PG_END_TRY();
@@ -4957,8 +4952,7 @@ postgresAnalyzeForeignTable(Relation relation,
 	}
 	PG_FINALLY();
 	{
-		if (res)
-			PQclear(res);
+		PQclear(res);
 	}
 	PG_END_TRY();
 
@@ -5114,8 +5108,7 @@ postgresAcquireSampleRowsFunc(Relation relation, int elevel,
 	}
 	PG_CATCH();
 	{
-		if (res)
-			PQclear(res);
+		PQclear(res);
 		PG_RE_THROW();
 	}
 	PG_END_TRY();
@@ -5496,8 +5489,7 @@ postgresImportForeignSchema(ImportForeignSchemaStmt *stmt, Oid serverOid)
 	}
 	PG_FINALLY();
 	{
-		if (res)
-			PQclear(res);
+		PQclear(res);
 	}
 	PG_END_TRY();
 
diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
@@ -3628,6 +3628,9 @@ char *PQresultErrorField(const PGresult *res, int fieldcode);
 <synopsis>
 void PQclear(PGresult *res);
 </synopsis>
+
+        If the argument is a <symbol>NULL</symbol> pointer, no operation is
+        performed.
        </para>
 
        <para>
@@ -6670,6 +6673,8 @@ void PQfreemem(void *ptr);
 <synopsis>
 void PQconninfoFree(PQconninfoOption *connOptions);
 </synopsis>
+      If the argument is a <symbol>NULL</symbol> pointer, no operation is
+      performed.
      </para>
 
      <para>
diff --git a/src/bin/pg_basebackup/streamutil.c b/src/bin/pg_basebackup/streamutil.c
@@ -197,16 +197,14 @@ GetConnection(void)
 		PQfinish(tmpconn);
 		free(values);
 		free(keywords);
-		if (conn_opts)
-			PQconninfoFree(conn_opts);
+		PQconninfoFree(conn_opts);
 		return NULL;
 	}
 
 	/* Connection ok! */
 	free(values);
 	free(keywords);
-	if (conn_opts)
-		PQconninfoFree(conn_opts);
+	PQconninfoFree(conn_opts);
 
 	/*
 	 * Set always-secure search path, so malicious users can't get control.
diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c
@@ -1502,8 +1502,7 @@ connectDatabase(const char *dbname, const char *connection_string,
 
 		free(keywords);
 		free(values);
-		if (conn_opts)
-			PQconninfoFree(conn_opts);
+		PQconninfoFree(conn_opts);
 
 		/*
 		 * Merge the connection info inputs given in form of connection string
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
@@ -3476,8 +3476,7 @@ do_connect(enum trivalue reuse_previous_specification,
 
 	/* Release locally allocated data, whether we succeeded or not */
 	pg_free(password);
-	if (cinfo)
-		PQconninfoFree(cinfo);
+	PQconninfoFree(cinfo);
 
 	if (!success)
 	{
diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c
@@ -463,8 +463,7 @@ ClearOrSaveResult(PGresult *result)
 		{
 			case PGRES_NONFATAL_ERROR:
 			case PGRES_FATAL_ERROR:
-				if (pset.last_error_result)
-					PQclear(pset.last_error_result);
+				PQclear(pset.last_error_result);
 				pset.last_error_result = result;
 				break;
 
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
@@ -3492,8 +3492,7 @@ describeOneTableDetails(const char *schemaname,
 
 	free(view_def);
 
-	if (res)
-		PQclear(res);
+	PQclear(res);
 
 	return retval;
 }
diff --git a/src/fe_utils/query_utils.c b/src/fe_utils/query_utils.c
@@ -85,8 +85,7 @@ executeMaintenanceCommand(PGconn *conn, const char *query, bool echo)
 
 	r = (res && PQresultStatus(res) == PGRES_COMMAND_OK);
 
-	if (res)
-		PQclear(res);
+	PQclear(res);
 
 	return r;
 }
diff --git a/src/interfaces/ecpg/ecpglib/descriptor.c b/src/interfaces/ecpg/ecpglib/descriptor.c
@@ -923,8 +923,7 @@ ECPGdescribe(int line, int compat, bool input, const char *connection_name, cons
 					if (!ecpg_check_PQresult(res, line, con->connection, compat))
 						break;
 
-					if (desc->result != NULL)
-						PQclear(desc->result);
+					PQclear(desc->result);
 
 					desc->result = res;
 					ret = true;
diff --git a/src/interfaces/ecpg/ecpglib/execute.c b/src/interfaces/ecpg/ecpglib/execute.c
@@ -1714,8 +1714,7 @@ ecpg_process_output(struct statement *stmt, bool clear_result)
 					status = false;
 				else
 				{
-					if (desc->result)
-						PQclear(desc->result);
+					PQclear(desc->result);
 					desc->result = stmt->results;
 					clear_result = false;
 					ecpg_log("ecpg_process_output on line %d: putting result (%d tuples) into descriptor %s\n",
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
@@ -3766,8 +3766,7 @@ PQconnectPoll(PGconn *conn)
 				}
 
 				/* Something went wrong with "SHOW transaction_read_only". */
-				if (res)
-					PQclear(res);
+				PQclear(res);
 
 				/* Append error report to conn->errorMessage. */
 				appendPQExpBuffer(&conn->errorMessage,
@@ -3818,8 +3817,7 @@ PQconnectPoll(PGconn *conn)
 				}
 
 				/* Something went wrong with "SELECT pg_is_in_recovery()". */
-				if (res)
-					PQclear(res);
+				PQclear(res);
 
 				/* Append error report to conn->errorMessage. */
 				appendPQExpBuffer(&conn->errorMessage,
diff --git a/src/interfaces/libpq/fe-exec.c b/src/interfaces/libpq/fe-exec.c
@@ -775,12 +775,10 @@ PQclear(PGresult *res)
 void
 pqClearAsyncResult(PGconn *conn)
 {
-	if (conn->result)
-		PQclear(conn->result);
+	PQclear(conn->result);
 	conn->result = NULL;
 	conn->error_result = false;
-	if (conn->next_result)
-		PQclear(conn->next_result);
+	PQclear(conn->next_result);
 	conn->next_result = NULL;
 }
 
@@ -2437,8 +2435,7 @@ PQexecFinish(PGconn *conn)
 	lastResult = NULL;
 	while ((result = PQgetResult(conn)) != NULL)
 	{
-		if (lastResult)
-			PQclear(lastResult);
+		PQclear(lastResult);
 		lastResult = result;
 		if (result->resultStatus == PGRES_COPY_IN ||
 			result->resultStatus == PGRES_COPY_OUT ||

Original file line number	Diff line number	Diff line change
`@@ -2790,8 +2790,7 @@ postgresEndDirectModify(ForeignScanState *node)`
`2790`	`2790`	`return;`
`2791`	`2791`
`2792`	`2792`	`/* Release PGresult */`
`2793`		`- if (dmstate->result)`
`2794`		`- PQclear(dmstate->result);`
	`2793`	`+ PQclear(dmstate->result);`
`2795`	`2794`
`2796`	`2795`	`/* Release remote connection */`
`2797`	`2796`	`ReleaseConnection(dmstate->conn);`
`@@ -3604,8 +3603,7 @@ get_remote_estimate(const char sql, PGconn conn,`
`3604`	`3603`	`}`
`3605`	`3604`	`PG_FINALLY();`
`3606`	`3605`	`{`
`3607`		`- if (res)`
`3608`		`- PQclear(res);`
	`3606`	`+ PQclear(res);`
`3609`	`3607`	`}`
`3610`	`3608`	`PG_END_TRY();`
`3611`	`3609`	`}`
`@@ -3853,8 +3851,7 @@ fetch_more_data(ForeignScanState *node)`
`3853`	`3851`	`}`
`3854`	`3852`	`PG_FINALLY();`
`3855`	`3853`	`{`
`3856`		`- if (res)`
`3857`		`- PQclear(res);`
	`3854`	`+ PQclear(res);`
`3858`	`3855`	`}`
`3859`	`3856`	`PG_END_TRY();`
`3860`	`3857`
`@@ -4338,8 +4335,7 @@ store_returning_result(PgFdwModifyState *fmstate,`
`4338`	`4335`	`}`
`4339`	`4336`	`PG_CATCH();`
`4340`	`4337`	`{`
`4341`		`- if (res)`
`4342`		`- PQclear(res);`
	`4338`	`+ PQclear(res);`
`4343`	`4339`	`PG_RE_THROW();`
`4344`	`4340`	`}`
`4345`	`4341`	`PG_END_TRY();`
`@@ -4627,8 +4623,7 @@ get_returning_data(ForeignScanState *node)`
`4627`	`4623`	`}`
`4628`	`4624`	`PG_CATCH();`
`4629`	`4625`	`{`
`4630`		`- if (dmstate->result)`
`4631`		`- PQclear(dmstate->result);`
	`4626`	`+ PQclear(dmstate->result);`
`4632`	`4627`	`PG_RE_THROW();`
`4633`	`4628`	`}`
`4634`	`4629`	`PG_END_TRY();`
`@@ -4957,8 +4952,7 @@ postgresAnalyzeForeignTable(Relation relation,`
`4957`	`4952`	`}`
`4958`	`4953`	`PG_FINALLY();`
`4959`	`4954`	`{`
`4960`		`- if (res)`
`4961`		`- PQclear(res);`
	`4955`	`+ PQclear(res);`
`4962`	`4956`	`}`
`4963`	`4957`	`PG_END_TRY();`
`4964`	`4958`
`@@ -5114,8 +5108,7 @@ postgresAcquireSampleRowsFunc(Relation relation, int elevel,`
`5114`	`5108`	`}`
`5115`	`5109`	`PG_CATCH();`
`5116`	`5110`	`{`
`5117`		`- if (res)`
`5118`		`- PQclear(res);`
	`5111`	`+ PQclear(res);`
`5119`	`5112`	`PG_RE_THROW();`
`5120`	`5113`	`}`
`5121`	`5114`	`PG_END_TRY();`
`@@ -5496,8 +5489,7 @@ postgresImportForeignSchema(ImportForeignSchemaStmt *stmt, Oid serverOid)`
`5496`	`5489`	`}`
`5497`	`5490`	`PG_FINALLY();`
`5498`	`5491`	`{`
`5499`		`- if (res)`
`5500`		`- PQclear(res);`
	`5492`	`+ PQclear(res);`
`5501`	`5493`	`}`
`5502`	`5494`	`PG_END_TRY();`
`5503`	`5495`
Original file line number	Diff line number	Diff line change
`@@ -3476,8 +3476,7 @@ do_connect(enum trivalue reuse_previous_specification,`
`3476`	`3476`
`3477`	`3477`	`/* Release locally allocated data, whether we succeeded or not */`
`3478`	`3478`	`pg_free(password);`
`3479`		`- if (cinfo)`
`3480`		`- PQconninfoFree(cinfo);`
	`3479`	`+ PQconninfoFree(cinfo);`
`3481`	`3480`
`3482`	`3481`	`if (!success)`
`3483`	`3482`	`{`
Original file line number	Diff line number	Diff line change
`@@ -463,8 +463,7 @@ ClearOrSaveResult(PGresult *result)`
`463`	`463`	`{`
`464`	`464`	`case PGRES_NONFATAL_ERROR:`
`465`	`465`	`case PGRES_FATAL_ERROR:`
`466`		`- if (pset.last_error_result)`
`467`		`- PQclear(pset.last_error_result);`
	`466`	`+ PQclear(pset.last_error_result);`
`468`	`467`	`pset.last_error_result = result;`
`469`	`468`	`break;`
`470`	`469`
Original file line number	Diff line number	Diff line change
`@@ -3492,8 +3492,7 @@ describeOneTableDetails(const char *schemaname,`
`3492`	`3492`
`3493`	`3493`	`free(view_def);`
`3494`	`3494`
`3495`		`- if (res)`
`3496`		`- PQclear(res);`
	`3495`	`+ PQclear(res);`
`3497`	`3496`
`3498`	`3497`	`return retval;`
`3499`	`3498`	`}`
Original file line number	Diff line number	Diff line change
`@@ -85,8 +85,7 @@ executeMaintenanceCommand(PGconn conn, const char query, bool echo)`
`85`	`85`
`86`	`86`	`r = (res && PQresultStatus(res) == PGRES_COMMAND_OK);`
`87`	`87`
`88`		`- if (res)`
`89`		`- PQclear(res);`
	`88`	`+ PQclear(res);`
`90`	`89`
`91`	`90`	`return r;`
`92`	`91`	`}`