From 355ef1a68be690d9bb8ee0524226abd648733ce0 Mon Sep 17 00:00:00 2001 From: Peter Eisentraut Date: Fri, 17 Jun 2022 12:09:32 +0200 Subject: [PATCH v2 3/3] Remove redundant null pointer checks before PQclear and PQconninfofree 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. --- contrib/dblink/dblink.c | 6 ++---- contrib/postgres_fdw/postgres_fdw.c | 24 ++++++++---------------- doc/src/sgml/libpq.sgml | 5 +++++ src/bin/pg_basebackup/streamutil.c | 6 ++---- src/bin/pg_dump/pg_dumpall.c | 3 +-- src/bin/psql/command.c | 3 +-- src/bin/psql/common.c | 3 +-- src/bin/psql/describe.c | 3 +-- src/fe_utils/query_utils.c | 3 +-- src/interfaces/ecpg/ecpglib/descriptor.c | 3 +-- src/interfaces/ecpg/ecpglib/execute.c | 3 +-- src/interfaces/libpq/fe-connect.c | 6 ++---- src/interfaces/libpq/fe-exec.c | 9 +++------ 13 files changed, 29 insertions(+), 48 deletions(-) diff --git a/contrib/dblink/dblink.c b/contrib/dblink/dblink.c index a561d1d652..e323fdd0e6 100644 --- 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 index d56951153b..955a428e3d 100644 --- 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 index 37ec3cb4e5..74456aa69d 100644 --- a/doc/src/sgml/libpq.sgml +++ b/doc/src/sgml/libpq.sgml @@ -3628,6 +3628,9 @@ Main Functions void PQclear(PGresult *res); + + If the argument is a NULL pointer, no operation is + performed. @@ -6670,6 +6673,8 @@ Miscellaneous Functions void PQconninfoFree(PQconninfoOption *connOptions); + If the argument is a NULL pointer, no operation is + performed. diff --git a/src/bin/pg_basebackup/streamutil.c b/src/bin/pg_basebackup/streamutil.c index 8e820c0c71..919ec9c29c 100644 --- 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 index da5cf85272..26d3d53809 100644 --- 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 index f3c5196c90..c562c04afe 100644 --- 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 index 974959c595..9f95869eca 100644 --- 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 index 7a21777d9e..67a75b9c81 100644 --- 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 index 2fc6e2405b..6575b24c78 100644 --- 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 index 369c2f0867..b35179ada0 100644 --- a/src/interfaces/ecpg/ecpglib/descriptor.c +++ b/src/interfaces/ecpg/ecpglib/descriptor.c @@ -918,8 +918,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 index 6a7ef0bbf6..32af5d63f1 100644 --- a/src/interfaces/ecpg/ecpglib/execute.c +++ b/src/interfaces/ecpg/ecpglib/execute.c @@ -1717,8 +1717,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 index 057c9da0ed..dc49387d6c 100644 --- 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 index 1750d647a8..51e9a362f6 100644 --- 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 || -- 2.36.1