libpq: Remove redundant null pointer checks before free()
libpq contains a lot of
if (foo)
free(foo);
calls, where the "if" part is unnecessary. This is of course pretty
harmless, but some functions like scram_free() and freePGconn() have
become so bulky that it becomes annoying. So while I was doing some
work in that area I undertook to simplify this.
Attachments:
0001-libpq-Remove-redundant-null-pointer-checks-before-fr.patchtext/plain; charset=UTF-8; name=0001-libpq-Remove-redundant-null-pointer-checks-before-fr.patchDownload
From f7a10c1fca10c76b89dbf402ad9418ed8f0675d7 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Thu, 16 Jun 2022 21:50:56 +0200
Subject: [PATCH] libpq: Remove redundant null pointer checks before free()
---
src/interfaces/libpq/fe-auth-scram.c | 33 ++--
src/interfaces/libpq/fe-auth.c | 18 +-
src/interfaces/libpq/fe-connect.c | 211 ++++++++----------------
src/interfaces/libpq/fe-exec.c | 6 +-
src/interfaces/libpq/fe-print.c | 23 +--
src/interfaces/libpq/fe-secure-common.c | 3 +-
6 files changed, 97 insertions(+), 197 deletions(-)
diff --git a/src/interfaces/libpq/fe-auth-scram.c b/src/interfaces/libpq/fe-auth-scram.c
index e616200704..5012806fa5 100644
--- a/src/interfaces/libpq/fe-auth-scram.c
+++ b/src/interfaces/libpq/fe-auth-scram.c
@@ -174,30 +174,21 @@ scram_free(void *opaq)
{
fe_scram_state *state = (fe_scram_state *) opaq;
- if (state->password)
- free(state->password);
- if (state->sasl_mechanism)
- free(state->sasl_mechanism);
+ free(state->password);
+ free(state->sasl_mechanism);
/* client messages */
- if (state->client_nonce)
- free(state->client_nonce);
- if (state->client_first_message_bare)
- free(state->client_first_message_bare);
- if (state->client_final_message_without_proof)
- free(state->client_final_message_without_proof);
+ free(state->client_nonce);
+ free(state->client_first_message_bare);
+ free(state->client_final_message_without_proof);
/* first message from server */
- if (state->server_first_message)
- free(state->server_first_message);
- if (state->salt)
- free(state->salt);
- if (state->nonce)
- free(state->nonce);
+ free(state->server_first_message);
+ free(state->salt);
+ free(state->nonce);
/* final message from server */
- if (state->server_final_message)
- free(state->server_final_message);
+ free(state->server_final_message);
free(state);
}
@@ -941,8 +932,7 @@ pg_fe_scram_build_secret(const char *password, const char **errstr)
if (!pg_strong_random(saltbuf, SCRAM_DEFAULT_SALT_LEN))
{
*errstr = _("failed to generate random salt");
- if (prep_password)
- free(prep_password);
+ free(prep_password);
return NULL;
}
@@ -950,8 +940,7 @@ pg_fe_scram_build_secret(const char *password, const char **errstr)
SCRAM_DEFAULT_ITERATIONS, password,
errstr);
- if (prep_password)
- free(prep_password);
+ free(prep_password);
return result;
}
diff --git a/src/interfaces/libpq/fe-auth.c b/src/interfaces/libpq/fe-auth.c
index 0a072a36dc..49a1c626f6 100644
--- a/src/interfaces/libpq/fe-auth.c
+++ b/src/interfaces/libpq/fe-auth.c
@@ -107,8 +107,7 @@ pg_GSS_continue(PGconn *conn, int payloadlen)
NULL,
NULL);
- if (ginbuf.value)
- free(ginbuf.value);
+ free(ginbuf.value);
if (goutbuf.length != 0)
{
@@ -270,8 +269,7 @@ pg_SSPI_continue(PGconn *conn, int payloadlen)
NULL);
/* we don't need the input anymore */
- if (inputbuf)
- free(inputbuf);
+ free(inputbuf);
if (r != SEC_E_OK && r != SEC_I_CONTINUE_NEEDED)
{
@@ -604,21 +602,18 @@ pg_SASL_init(PGconn *conn, int payloadlen)
goto error;
termPQExpBuffer(&mechanism_buf);
- if (initialresponse)
- free(initialresponse);
+ free(initialresponse);
return STATUS_OK;
error:
termPQExpBuffer(&mechanism_buf);
- if (initialresponse)
- free(initialresponse);
+ free(initialresponse);
return STATUS_ERROR;
oom_error:
termPQExpBuffer(&mechanism_buf);
- if (initialresponse)
- free(initialresponse);
+ free(initialresponse);
appendPQExpBufferStr(&conn->errorMessage,
libpq_gettext("out of memory\n"));
return STATUS_ERROR;
@@ -831,8 +826,7 @@ pg_password_sendauth(PGconn *conn, const char *password, AuthRequest areq)
return STATUS_ERROR;
}
ret = pqPacketSend(conn, 'p', pwd_to_send, strlen(pwd_to_send) + 1);
- if (crypt_pwd)
- free(crypt_pwd);
+ free(crypt_pwd);
return ret;
}
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index 6e936bbff3..057c9da0ed 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -540,8 +540,7 @@ pqFreeCommandQueue(PGcmdQueueEntry *queue)
PGcmdQueueEntry *cur = queue;
queue = cur->next;
- if (cur->query)
- free(cur->query);
+ free(cur->query);
free(cur);
}
}
@@ -593,8 +592,7 @@ pqDropServerData(PGconn *conn)
conn->sversion = 0;
/* Drop large-object lookup data */
- if (conn->lobjfuncs)
- free(conn->lobjfuncs);
+ free(conn->lobjfuncs);
conn->lobjfuncs = NULL;
/* Reset assorted other per-connection state */
@@ -602,8 +600,7 @@ pqDropServerData(PGconn *conn)
conn->auth_req_received = false;
conn->password_needed = false;
conn->write_failed = false;
- if (conn->write_err_msg)
- free(conn->write_err_msg);
+ free(conn->write_err_msg);
conn->write_err_msg = NULL;
conn->be_pid = 0;
conn->be_key = 0;
@@ -898,8 +895,7 @@ fillPGconn(PGconn *conn, PQconninfoOption *connOptions)
{
char **connmember = (char **) ((char *) conn + option->connofs);
- if (*connmember)
- free(*connmember);
+ free(*connmember);
*connmember = strdup(tmp);
if (*connmember == NULL)
{
@@ -1113,8 +1109,7 @@ connectOptions2(PGconn *conn)
}
else
{
- if (ch->host)
- free(ch->host);
+ free(ch->host);
/*
* This bit selects the default host location. If you change
@@ -1186,8 +1181,7 @@ connectOptions2(PGconn *conn)
*/
if (conn->pguser == NULL || conn->pguser[0] == '\0')
{
- if (conn->pguser)
- free(conn->pguser);
+ free(conn->pguser);
conn->pguser = pg_fe_getauthname(&conn->errorMessage);
if (!conn->pguser)
{
@@ -1201,8 +1195,7 @@ connectOptions2(PGconn *conn)
*/
if (conn->dbName == NULL || conn->dbName[0] == '\0')
{
- if (conn->dbName)
- free(conn->dbName);
+ free(conn->dbName);
conn->dbName = strdup(conn->pguser);
if (!conn->dbName)
goto oom_error;
@@ -1221,8 +1214,7 @@ connectOptions2(PGconn *conn)
if (pqGetHomeDirectory(homedir, sizeof(homedir)))
{
- if (conn->pgpassfile)
- free(conn->pgpassfile);
+ free(conn->pgpassfile);
conn->pgpassfile = malloc(MAXPGPATH);
if (!conn->pgpassfile)
goto oom_error;
@@ -1548,8 +1540,7 @@ PQsetdbLogin(const char *pghost, const char *pgport, const char *pgoptions,
/* Insert dbName parameter value into struct */
if (dbName && dbName[0] != '\0')
{
- if (conn->dbName)
- free(conn->dbName);
+ free(conn->dbName);
conn->dbName = strdup(dbName);
if (!conn->dbName)
goto oom_error;
@@ -1562,8 +1553,7 @@ PQsetdbLogin(const char *pghost, const char *pgport, const char *pgoptions,
*/
if (pghost && pghost[0] != '\0')
{
- if (conn->pghost)
- free(conn->pghost);
+ free(conn->pghost);
conn->pghost = strdup(pghost);
if (!conn->pghost)
goto oom_error;
@@ -1571,8 +1561,7 @@ PQsetdbLogin(const char *pghost, const char *pgport, const char *pgoptions,
if (pgport && pgport[0] != '\0')
{
- if (conn->pgport)
- free(conn->pgport);
+ free(conn->pgport);
conn->pgport = strdup(pgport);
if (!conn->pgport)
goto oom_error;
@@ -1580,8 +1569,7 @@ PQsetdbLogin(const char *pghost, const char *pgport, const char *pgoptions,
if (pgoptions && pgoptions[0] != '\0')
{
- if (conn->pgoptions)
- free(conn->pgoptions);
+ free(conn->pgoptions);
conn->pgoptions = strdup(pgoptions);
if (!conn->pgoptions)
goto oom_error;
@@ -1589,8 +1577,7 @@ PQsetdbLogin(const char *pghost, const char *pgport, const char *pgoptions,
if (login && login[0] != '\0')
{
- if (conn->pguser)
- free(conn->pguser);
+ free(conn->pguser);
conn->pguser = strdup(login);
if (!conn->pguser)
goto oom_error;
@@ -1598,8 +1585,7 @@ PQsetdbLogin(const char *pghost, const char *pgport, const char *pgoptions,
if (pwd && pwd[0] != '\0')
{
- if (conn->pgpass)
- free(conn->pgpass);
+ free(conn->pgpass);
conn->pgpass = strdup(pwd);
if (!conn->pgpass)
goto oom_error;
@@ -4044,10 +4030,8 @@ makeEmptyPGconn(void)
static void
freePGconn(PGconn *conn)
{
- int i;
-
/* let any event procs clean up their state data */
- for (i = 0; i < conn->nEvents; i++)
+ for (int i = 0; i < conn->nEvents; i++)
{
PGEventConnDestroy evt;
@@ -4058,114 +4042,69 @@ freePGconn(PGconn *conn)
}
/* clean up pg_conn_host structures */
- if (conn->connhost != NULL)
+ for (int i = 0; i < conn->nconnhost; ++i)
{
- for (i = 0; i < conn->nconnhost; ++i)
+ free(conn->connhost[i].host);
+ free(conn->connhost[i].hostaddr);
+ free(conn->connhost[i].port);
+ if (conn->connhost[i].password != NULL)
{
- if (conn->connhost[i].host != NULL)
- free(conn->connhost[i].host);
- if (conn->connhost[i].hostaddr != NULL)
- free(conn->connhost[i].hostaddr);
- if (conn->connhost[i].port != NULL)
- free(conn->connhost[i].port);
- if (conn->connhost[i].password != NULL)
- {
- explicit_bzero(conn->connhost[i].password, strlen(conn->connhost[i].password));
- free(conn->connhost[i].password);
- }
+ explicit_bzero(conn->connhost[i].password, strlen(conn->connhost[i].password));
+ free(conn->connhost[i].password);
}
- free(conn->connhost);
}
-
- if (conn->client_encoding_initial)
- free(conn->client_encoding_initial);
- if (conn->events)
- free(conn->events);
- if (conn->pghost)
- free(conn->pghost);
- if (conn->pghostaddr)
- free(conn->pghostaddr);
- if (conn->pgport)
- free(conn->pgport);
- if (conn->connect_timeout)
- free(conn->connect_timeout);
- if (conn->pgtcp_user_timeout)
- free(conn->pgtcp_user_timeout);
- if (conn->pgoptions)
- free(conn->pgoptions);
- if (conn->appname)
- free(conn->appname);
- if (conn->fbappname)
- free(conn->fbappname);
- if (conn->dbName)
- free(conn->dbName);
- if (conn->replication)
- free(conn->replication);
- if (conn->pguser)
- free(conn->pguser);
+ free(conn->connhost);
+
+ free(conn->client_encoding_initial);
+ free(conn->events);
+ free(conn->pghost);
+ free(conn->pghostaddr);
+ free(conn->pgport);
+ free(conn->connect_timeout);
+ free(conn->pgtcp_user_timeout);
+ free(conn->pgoptions);
+ free(conn->appname);
+ free(conn->fbappname);
+ free(conn->dbName);
+ free(conn->replication);
+ free(conn->pguser);
if (conn->pgpass)
{
explicit_bzero(conn->pgpass, strlen(conn->pgpass));
free(conn->pgpass);
}
- if (conn->pgpassfile)
- free(conn->pgpassfile);
- if (conn->channel_binding)
- free(conn->channel_binding);
- if (conn->keepalives)
- free(conn->keepalives);
- if (conn->keepalives_idle)
- free(conn->keepalives_idle);
- if (conn->keepalives_interval)
- free(conn->keepalives_interval);
- if (conn->keepalives_count)
- free(conn->keepalives_count);
- if (conn->sslmode)
- free(conn->sslmode);
- if (conn->sslcert)
- free(conn->sslcert);
- if (conn->sslkey)
- free(conn->sslkey);
+ free(conn->pgpassfile);
+ free(conn->channel_binding);
+ free(conn->keepalives);
+ free(conn->keepalives_idle);
+ free(conn->keepalives_interval);
+ free(conn->keepalives_count);
+ free(conn->sslmode);
+ free(conn->sslcert);
+ free(conn->sslkey);
if (conn->sslpassword)
{
explicit_bzero(conn->sslpassword, strlen(conn->sslpassword));
free(conn->sslpassword);
}
- if (conn->sslrootcert)
- free(conn->sslrootcert);
- if (conn->sslcrl)
- free(conn->sslcrl);
- if (conn->sslcrldir)
- free(conn->sslcrldir);
- if (conn->sslcompression)
- free(conn->sslcompression);
- if (conn->sslsni)
- free(conn->sslsni);
- if (conn->requirepeer)
- free(conn->requirepeer);
- if (conn->ssl_min_protocol_version)
- free(conn->ssl_min_protocol_version);
- if (conn->ssl_max_protocol_version)
- free(conn->ssl_max_protocol_version);
- if (conn->gssencmode)
- free(conn->gssencmode);
- if (conn->krbsrvname)
- free(conn->krbsrvname);
- if (conn->gsslib)
- free(conn->gsslib);
- if (conn->connip)
- free(conn->connip);
+ free(conn->sslrootcert);
+ free(conn->sslcrl);
+ free(conn->sslcrldir);
+ free(conn->sslcompression);
+ free(conn->sslsni);
+ free(conn->requirepeer);
+ free(conn->ssl_min_protocol_version);
+ free(conn->ssl_max_protocol_version);
+ free(conn->gssencmode);
+ free(conn->krbsrvname);
+ free(conn->gsslib);
+ free(conn->connip);
/* Note that conn->Pfdebug is not ours to close or free */
- if (conn->write_err_msg)
- free(conn->write_err_msg);
- if (conn->inBuffer)
- free(conn->inBuffer);
- if (conn->outBuffer)
- free(conn->outBuffer);
- if (conn->rowBuf)
- free(conn->rowBuf);
- if (conn->target_session_attrs)
- free(conn->target_session_attrs);
+ free(conn->write_err_msg);
+ free(conn->inBuffer);
+ free(conn->outBuffer);
+ free(conn->rowBuf);
+ free(conn->target_session_attrs);
termPQExpBuffer(&conn->errorMessage);
termPQExpBuffer(&conn->workBuffer);
@@ -4433,8 +4372,7 @@ PQgetCancel(PGconn *conn)
void
PQfreeCancel(PGcancel *cancel)
{
- if (cancel)
- free(cancel);
+ free(cancel);
}
@@ -5883,8 +5821,7 @@ conninfo_array_parse(const char *const *keywords, const char *const *values,
{
if (strcmp(options[k].keyword, str_option->keyword) == 0)
{
- if (options[k].val)
- free(options[k].val);
+ free(options[k].val);
options[k].val = strdup(str_option->val);
if (!options[k].val)
{
@@ -5912,8 +5849,7 @@ conninfo_array_parse(const char *const *keywords, const char *const *values,
/*
* Store the value, overriding previous settings
*/
- if (option->val)
- free(option->val);
+ free(option->val);
option->val = strdup(pvalue);
if (!option->val)
{
@@ -6344,8 +6280,7 @@ conninfo_uri_parse_options(PQconninfoOption *options, const char *uri,
cleanup:
termPQExpBuffer(&hostbuf);
termPQExpBuffer(&portbuf);
- if (buf)
- free(buf);
+ free(buf);
return retval;
}
@@ -6655,8 +6590,7 @@ conninfo_storeval(PQconninfoOption *connOptions,
}
}
- if (option->val)
- free(option->val);
+ free(option->val);
option->val = value_copy;
return option;
@@ -6735,16 +6669,11 @@ PQconninfo(PGconn *conn)
void
PQconninfoFree(PQconninfoOption *connOptions)
{
- PQconninfoOption *option;
-
if (connOptions == NULL)
return;
- for (option = connOptions; option->keyword != NULL; option++)
- {
- if (option->val != NULL)
- free(option->val);
- }
+ for (PQconninfoOption *option = connOptions; option->keyword != NULL; option++)
+ free(option->val);
free(connOptions);
}
diff --git a/src/interfaces/libpq/fe-exec.c b/src/interfaces/libpq/fe-exec.c
index 919cf5741d..1750d647a8 100644
--- a/src/interfaces/libpq/fe-exec.c
+++ b/src/interfaces/libpq/fe-exec.c
@@ -742,8 +742,7 @@ PQclear(PGresult *res)
free(res->events[i].name);
}
- if (res->events)
- free(res->events);
+ free(res->events);
/* Free all the subsidiary blocks */
while ((block = res->curBlock) != NULL)
@@ -753,8 +752,7 @@ PQclear(PGresult *res)
}
/* Free the top-level tuple pointer array */
- if (res->tuples)
- free(res->tuples);
+ free(res->tuples);
/* zero out the pointer fields to catch programming errors */
res->attDescs = NULL;
diff --git a/src/interfaces/libpq/fe-print.c b/src/interfaces/libpq/fe-print.c
index 82fc592f06..783cd9b756 100644
--- a/src/interfaces/libpq/fe-print.c
+++ b/src/interfaces/libpq/fe-print.c
@@ -303,26 +303,19 @@ PQprint(FILE *fout, const PGresult *res, const PQprintOpt *po)
fputs("</table>\n", fout);
exit:
- if (fieldMax)
- free(fieldMax);
- if (fieldNotNum)
- free(fieldNotNum);
- if (border)
- free(border);
+ free(fieldMax);
+ free(fieldNotNum);
+ free(border);
if (fields)
{
/* if calloc succeeded, this shouldn't overflow size_t */
size_t numfields = ((size_t) nTups + 1) * (size_t) nFields;
while (numfields-- > 0)
- {
- if (fields[numfields])
- free(fields[numfields]);
- }
+ free(fields[numfields]);
free(fields);
}
- if (fieldNames)
- free((void *) fieldNames);
+ free(fieldNames);
if (usePipe)
{
#ifdef WIN32
@@ -679,8 +672,7 @@ PQdisplayTuples(const PGresult *res,
fflush(fp);
- if (fLength)
- free(fLength);
+ free(fLength);
}
@@ -763,8 +755,7 @@ PQprintTuples(const PGresult *res,
}
}
- if (tborder)
- free(tborder);
+ free(tborder);
}
diff --git a/src/interfaces/libpq/fe-secure-common.c b/src/interfaces/libpq/fe-secure-common.c
index 8046fcd884..cc8a2b8593 100644
--- a/src/interfaces/libpq/fe-secure-common.c
+++ b/src/interfaces/libpq/fe-secure-common.c
@@ -309,8 +309,7 @@ pq_verify_peer_name_matches_certificate(PGconn *conn)
}
/* clean up */
- if (first_name)
- free(first_name);
+ free(first_name);
return (rc == 1);
}
--
2.36.1
On Thu, Jun 16, 2022 at 10:07:33PM +0200, Peter Eisentraut wrote:
calls, where the "if" part is unnecessary. This is of course pretty
harmless, but some functions like scram_free() and freePGconn() have become
so bulky that it becomes annoying. So while I was doing some work in that
area I undertook to simplify this.
Seems fine. Would some of the buildfarm dinosaurs hiccup on that?
gaur is one that comes into mind.
--
Michael
Michael Paquier <michael@paquier.xyz> writes:
On Thu, Jun 16, 2022 at 10:07:33PM +0200, Peter Eisentraut wrote:
calls, where the "if" part is unnecessary. This is of course pretty
harmless, but some functions like scram_free() and freePGconn() have become
so bulky that it becomes annoying. So while I was doing some work in that
area I undertook to simplify this.
Seems fine. Would some of the buildfarm dinosaurs hiccup on that?
gaur is one that comes into mind.
Doubt it. (In any case, gaur/pademelon are unlikely to be seen
again after a hardware failure --- I'm working on resurrecting that
machine using modern NetBSD on an external drive, but its HPUX
installation probably isn't coming back.)
POSIX has required free(NULL) to be a no-op since at least SUSv2 (1997).
Even back then, the machines that failed on it were legacy devices,
like then-decade-old SunOS versions. So I don't think that Peter's
proposal has any portability risk today.
Having said that, the pattern "if (x) free(x);" is absolutely
ubiquitous across our code, and so I'm not sure that I'm on
board with undoing it only in libpq. I'd be happier if we made
a push to get rid of it everywhere. Notably, I think the choice
that pfree(NULL) is disallowed traces directly to worries about
coding-pattern-compatibility with pre-POSIX free(). Should we
revisit that?
Independently of that concern, how much of a back-patch hazard
might we create with such changes?
regards, tom lane
On 17.06.22 05:25, Michael Paquier wrote:
On Thu, Jun 16, 2022 at 10:07:33PM +0200, Peter Eisentraut wrote:
calls, where the "if" part is unnecessary. This is of course pretty
harmless, but some functions like scram_free() and freePGconn() have become
so bulky that it becomes annoying. So while I was doing some work in that
area I undertook to simplify this.Seems fine. Would some of the buildfarm dinosaurs hiccup on that?
gaur is one that comes into mind.
I'm pretty sure PostgreSQL code already depends on this behavior anyway.
The code just isn't consistent about it.
On 17.06.22 07:11, Tom Lane wrote:
Having said that, the pattern "if (x) free(x);" is absolutely
ubiquitous across our code, and so I'm not sure that I'm on
board with undoing it only in libpq. I'd be happier if we made
a push to get rid of it everywhere.
Sure, here is a more comprehensive patch set. (It still looks like
libpq is the largest chunk.)
Notably, I think the choice
that pfree(NULL) is disallowed traces directly to worries about
coding-pattern-compatibility with pre-POSIX free(). Should we
revisit that?
Yes please, and also repalloc().
Attachments:
v2-0001-Remove-redundant-null-pointer-checks-before-free.patchtext/plain; charset=UTF-8; name=v2-0001-Remove-redundant-null-pointer-checks-before-free.patchDownload
From b8b24ade0f08077fe5a43a2373e86886f7e36abc Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Thu, 16 Jun 2022 21:50:56 +0200
Subject: [PATCH v2 1/3] Remove redundant null pointer checks before free()
---
.../pg_stat_statements/pg_stat_statements.c | 12 +-
contrib/uuid-ossp/uuid-ossp.c | 6 +-
src/backend/bootstrap/bootstrap.c | 3 +-
src/backend/libpq/auth.c | 5 +-
src/backend/postmaster/postmaster.c | 3 +-
src/backend/regex/regc_pg_locale.c | 6 +-
src/backend/tcop/postgres.c | 3 +-
src/backend/utils/adt/pg_locale.c | 30 +--
src/backend/utils/error/elog.c | 3 +-
src/backend/utils/init/miscinit.c | 3 +-
src/backend/utils/misc/guc.c | 24 +-
src/bin/pg_basebackup/pg_basebackup.c | 3 +-
src/bin/pg_basebackup/streamutil.c | 3 +-
src/bin/pg_dump/dumputils.c | 21 +-
src/bin/pg_dump/pg_backup_archiver.c | 60 ++---
src/bin/pg_dump/pg_backup_custom.c | 6 +-
src/bin/pg_dump/pg_backup_db.c | 3 +-
src/bin/pg_dump/pg_backup_tar.c | 3 +-
src/bin/pg_dump/pg_dump.c | 30 +--
src/bin/pg_dump/pg_dumpall.c | 6 +-
src/bin/pgbench/pgbench.c | 6 +-
src/bin/psql/command.c | 66 ++----
src/bin/psql/copy.c | 3 +-
src/bin/psql/describe.c | 6 +-
src/bin/psql/input.c | 3 +-
src/bin/psql/tab-complete.c | 15 +-
src/common/fe_memutils.c | 3 +-
src/fe_utils/connect_utils.c | 3 +-
src/fe_utils/string_utils.c | 6 +-
src/interfaces/ecpg/pgtypeslib/numeric.c | 6 +-
src/interfaces/ecpg/preproc/descriptor.c | 3 +-
src/interfaces/libpq/fe-auth-scram.c | 33 +--
src/interfaces/libpq/fe-auth.c | 18 +-
src/interfaces/libpq/fe-connect.c | 211 ++++++------------
src/interfaces/libpq/fe-exec.c | 6 +-
src/interfaces/libpq/fe-print.c | 23 +-
src/interfaces/libpq/fe-secure-common.c | 3 +-
src/port/getaddrinfo.c | 3 +-
38 files changed, 214 insertions(+), 436 deletions(-)
diff --git a/contrib/pg_stat_statements/pg_stat_statements.c b/contrib/pg_stat_statements/pg_stat_statements.c
index 768cedd91a..4acfddcdb8 100644
--- a/contrib/pg_stat_statements/pg_stat_statements.c
+++ b/contrib/pg_stat_statements/pg_stat_statements.c
@@ -809,8 +809,7 @@ pgss_shmem_shutdown(int code, Datum arg)
(errcode_for_file_access(),
errmsg("could not write file \"%s\": %m",
PGSS_DUMP_FILE ".tmp")));
- if (qbuffer)
- free(qbuffer);
+ free(qbuffer);
if (file)
FreeFile(file);
unlink(PGSS_DUMP_FILE ".tmp");
@@ -1657,8 +1656,7 @@ pg_stat_statements_internal(FunctionCallInfo fcinfo,
pgss->extent != extent ||
pgss->gc_count != gc_count)
{
- if (qbuffer)
- free(qbuffer);
+ free(qbuffer);
qbuffer = qtext_load_file(&qbuffer_size);
}
}
@@ -1842,8 +1840,7 @@ pg_stat_statements_internal(FunctionCallInfo fcinfo,
LWLockRelease(pgss->lock);
- if (qbuffer)
- free(qbuffer);
+ free(qbuffer);
}
/* Number of output arguments (columns) for pg_stat_statements_info */
@@ -2446,8 +2443,7 @@ gc_qtexts(void)
/* clean up resources */
if (qfile)
FreeFile(qfile);
- if (qbuffer)
- free(qbuffer);
+ free(qbuffer);
/*
* Since the contents of the external file are now uncertain, mark all
diff --git a/contrib/uuid-ossp/uuid-ossp.c b/contrib/uuid-ossp/uuid-ossp.c
index 7c0fb812fd..b868812358 100644
--- a/contrib/uuid-ossp/uuid-ossp.c
+++ b/contrib/uuid-ossp/uuid-ossp.c
@@ -292,8 +292,7 @@ uuid_generate_internal(int v, unsigned char *ns, const char *ptr, int len)
if (ptr && len <= 36)
strcpy(strbuf + (36 - len), ptr);
}
- if (str)
- free(str);
+ free(str);
}
if (status != uuid_s_ok)
@@ -366,8 +365,7 @@ uuid_generate_internal(int v, unsigned char *ns, const char *ptr, int len)
if (status == uuid_s_ok)
strlcpy(strbuf, str, 37);
- if (str)
- free(str);
+ free(str);
if (status != uuid_s_ok)
ereport(ERROR,
diff --git a/src/backend/bootstrap/bootstrap.c b/src/backend/bootstrap/bootstrap.c
index 9a610d41ad..088556ab54 100644
--- a/src/backend/bootstrap/bootstrap.c
+++ b/src/backend/bootstrap/bootstrap.c
@@ -288,8 +288,7 @@ BootstrapModeMain(int argc, char *argv[], bool check_only)
SetConfigOption(name, value, PGC_POSTMASTER, PGC_S_ARGV);
free(name);
- if (value)
- free(value);
+ free(value);
break;
}
default:
diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c
index efc53f3135..2d9ab7edce 100644
--- a/src/backend/libpq/auth.c
+++ b/src/backend/libpq/auth.c
@@ -2020,10 +2020,7 @@ pam_passwd_conv_proc(int num_msg, const struct pam_message **msg,
fail:
/* free up whatever we allocated */
for (i = 0; i < num_msg; i++)
- {
- if (reply[i].resp != NULL)
- free(reply[i].resp);
- }
+ free(reply[i].resp);
free(reply);
return PAM_CONV_ERR;
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index dde4bc25b1..d7257e4056 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -854,8 +854,7 @@ PostmasterMain(int argc, char *argv[])
SetConfigOption(name, value, PGC_POSTMASTER, PGC_S_ARGV);
free(name);
- if (value)
- free(value);
+ free(value);
break;
}
diff --git a/src/backend/regex/regc_pg_locale.c b/src/backend/regex/regc_pg_locale.c
index e1f9df0918..02d462a659 100644
--- a/src/backend/regex/regc_pg_locale.c
+++ b/src/backend/regex/regc_pg_locale.c
@@ -930,10 +930,8 @@ pg_ctype_get_cache(pg_wc_probefunc probefunc, int cclasscode)
* Failure, clean up
*/
out_of_memory:
- if (pcc->cv.chrs)
- free(pcc->cv.chrs);
- if (pcc->cv.ranges)
- free(pcc->cv.ranges);
+ free(pcc->cv.chrs);
+ free(pcc->cv.ranges);
free(pcc);
return NULL;
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 8b6b5bbaaa..495cbf2006 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -3896,8 +3896,7 @@ process_postgres_switches(int argc, char *argv[], GucContext ctx,
}
SetConfigOption(name, value, ctx, gucsource);
free(name);
- if (value)
- free(value);
+ free(value);
break;
}
diff --git a/src/backend/utils/adt/pg_locale.c b/src/backend/utils/adt/pg_locale.c
index a0490a7522..ea42e70e43 100644
--- a/src/backend/utils/adt/pg_locale.c
+++ b/src/backend/utils/adt/pg_locale.c
@@ -374,26 +374,16 @@ assign_locale_messages(const char *newval, void *extra)
static void
free_struct_lconv(struct lconv *s)
{
- if (s->decimal_point)
- free(s->decimal_point);
- if (s->thousands_sep)
- free(s->thousands_sep);
- if (s->grouping)
- free(s->grouping);
- if (s->int_curr_symbol)
- free(s->int_curr_symbol);
- if (s->currency_symbol)
- free(s->currency_symbol);
- if (s->mon_decimal_point)
- free(s->mon_decimal_point);
- if (s->mon_thousands_sep)
- free(s->mon_thousands_sep);
- if (s->mon_grouping)
- free(s->mon_grouping);
- if (s->positive_sign)
- free(s->positive_sign);
- if (s->negative_sign)
- free(s->negative_sign);
+ free(s->decimal_point);
+ free(s->thousands_sep);
+ free(s->grouping);
+ free(s->int_curr_symbol);
+ free(s->currency_symbol);
+ free(s->mon_decimal_point);
+ free(s->mon_thousands_sep);
+ free(s->mon_grouping);
+ free(s->positive_sign);
+ free(s->negative_sign);
}
/*
diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index 55ee5423af..59124bd9cc 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -1985,8 +1985,7 @@ set_syslog_parameters(const char *ident, int facility)
closelog();
openlog_done = false;
}
- if (syslog_ident)
- free(syslog_ident);
+ free(syslog_ident);
syslog_ident = strdup(ident);
/* if the strdup fails, we will cope in write_syslog() */
syslog_facility = facility;
diff --git a/src/backend/utils/init/miscinit.c b/src/backend/utils/init/miscinit.c
index b25bd0e583..eb43b2c5e5 100644
--- a/src/backend/utils/init/miscinit.c
+++ b/src/backend/utils/init/miscinit.c
@@ -417,8 +417,7 @@ SetDataDir(const char *dir)
/* If presented path is relative, convert to absolute */
new = make_absolute_path(dir);
- if (DataDir)
- free(DataDir);
+ free(DataDir);
DataDir = new;
}
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index a7cc49898b..0b3186eead 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -5483,8 +5483,7 @@ build_guc_variables(void)
for (i = 0; ConfigureNamesEnum[i].gen.name; i++)
guc_vars[num_vars++] = &ConfigureNamesEnum[i].gen;
- if (guc_variables)
- free(guc_variables);
+ free(guc_variables);
guc_variables = guc_vars;
num_guc_variables = num_vars;
size_guc_variables = size_vars;
@@ -6880,8 +6879,7 @@ ReportGUCOption(struct config_generic *record)
* set last_reported to NULL and thereby possibly make a duplicate
* report later.
*/
- if (record->last_reported)
- free(record->last_reported);
+ free(record->last_reported);
record->last_reported = strdup(val);
}
@@ -8356,8 +8354,7 @@ set_config_sourcefile(const char *name, char *sourcefile, int sourceline)
return;
sourcefile = guc_strdup(elevel, sourcefile);
- if (record->sourcefile)
- free(record->sourcefile);
+ free(record->sourcefile);
record->sourcefile = sourcefile;
record->sourceline = sourceline;
}
@@ -8877,8 +8874,7 @@ AlterSystemSetConfigFile(AlterSystemStmt *altersysstmt)
if (record->vartype == PGC_STRING && newval.stringval != NULL)
free(newval.stringval);
- if (newextra)
- free(newextra);
+ free(newextra);
/*
* We must also reject values containing newlines, because the
@@ -11225,12 +11221,9 @@ RestoreGUCState(void *gucstate)
* pointers.
*/
Assert(gconf->stack == NULL);
- if (gconf->extra)
- free(gconf->extra);
- if (gconf->last_reported) /* probably can't happen */
- free(gconf->last_reported);
- if (gconf->sourcefile)
- free(gconf->sourcefile);
+ free(gconf->extra);
+ free(gconf->last_reported);
+ free(gconf->sourcefile);
switch (gconf->vartype)
{
case PGC_BOOL:
@@ -11261,8 +11254,7 @@ RestoreGUCState(void *gucstate)
{
struct config_string *conf = (struct config_string *) gconf;
- if (*conf->variable)
- free(*conf->variable);
+ free(*conf->variable);
if (conf->reset_val && conf->reset_val != *conf->variable)
free(conf->reset_val);
if (conf->reset_extra && conf->reset_extra != gconf->extra)
diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c
index 4adb170d46..b66eac707a 100644
--- a/src/bin/pg_basebackup/pg_basebackup.c
+++ b/src/bin/pg_basebackup/pg_basebackup.c
@@ -775,8 +775,7 @@ progress_update_filename(const char *filename)
/* We needn't maintain this variable if not doing verbose reports. */
if (showprogress && verbose)
{
- if (progress_filename)
- free(progress_filename);
+ free(progress_filename);
if (filename)
progress_filename = pg_strdup(filename);
else
diff --git a/src/bin/pg_basebackup/streamutil.c b/src/bin/pg_basebackup/streamutil.c
index 299b9b7621..8e820c0c71 100644
--- a/src/bin/pg_basebackup/streamutil.c
+++ b/src/bin/pg_basebackup/streamutil.c
@@ -154,8 +154,7 @@ GetConnection(void)
/* Get a new password if appropriate */
if (need_password)
{
- if (password)
- free(password);
+ free(password);
password = simple_prompt("Password: ", false);
need_password = false;
}
diff --git a/src/bin/pg_dump/dumputils.c b/src/bin/pg_dump/dumputils.c
index 3e68dfc78f..6e501a5413 100644
--- a/src/bin/pg_dump/dumputils.c
+++ b/src/bin/pg_dump/dumputils.c
@@ -98,18 +98,15 @@ buildACLCommands(const char *name, const char *subname, const char *nspname,
/* Parse the acls array */
if (!parsePGArray(acls, &aclitems, &naclitems))
{
- if (aclitems)
- free(aclitems);
+ free(aclitems);
return false;
}
/* Parse the baseacls too */
if (!parsePGArray(baseacls, &baseitems, &nbaseitems))
{
- if (aclitems)
- free(aclitems);
- if (baseitems)
- free(baseitems);
+ free(aclitems);
+ free(baseitems);
return false;
}
@@ -298,14 +295,10 @@ buildACLCommands(const char *name, const char *subname, const char *nspname,
destroyPQExpBuffer(firstsql);
destroyPQExpBuffer(secondsql);
- if (aclitems)
- free(aclitems);
- if (baseitems)
- free(baseitems);
- if (grantitems)
- free(grantitems);
- if (revokeitems)
- free(revokeitems);
+ free(aclitems);
+ free(baseitems);
+ free(grantitems);
+ free(revokeitems);
return ok;
}
diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c
index 77fe51a3a5..233198afc0 100644
--- a/src/bin/pg_dump/pg_backup_archiver.c
+++ b/src/bin/pg_dump/pg_backup_archiver.c
@@ -640,8 +640,7 @@ RestoreArchive(Archive *AHX)
* If we treated users as pg_dump'able objects then we'd need to reset
* currUser here too.
*/
- if (AH->currSchema)
- free(AH->currSchema);
+ free(AH->currSchema);
AH->currSchema = NULL;
}
@@ -2067,8 +2066,7 @@ _discoverArchiveFormat(ArchiveHandle *AH)
pg_log_debug("attempting to ascertain archive format");
- if (AH->lookahead)
- free(AH->lookahead);
+ free(AH->lookahead);
AH->readHeader = 0;
AH->lookaheadSize = 512;
@@ -3178,21 +3176,17 @@ _reconnectToDB(ArchiveHandle *AH, const char *dbname)
* NOTE: currUser keeps track of what the imaginary session user in our
* script is. It's now effectively reset to the original userID.
*/
- if (AH->currUser)
- free(AH->currUser);
+ free(AH->currUser);
AH->currUser = NULL;
/* don't assume we still know the output schema, tablespace, etc either */
- if (AH->currSchema)
- free(AH->currSchema);
+ free(AH->currSchema);
AH->currSchema = NULL;
- if (AH->currTableAm)
- free(AH->currTableAm);
+ free(AH->currTableAm);
AH->currTableAm = NULL;
- if (AH->currTablespace)
- free(AH->currTablespace);
+ free(AH->currTablespace);
AH->currTablespace = NULL;
/* re-establish fixed state */
@@ -3219,8 +3213,7 @@ _becomeUser(ArchiveHandle *AH, const char *user)
* NOTE: currUser keeps track of what the imaginary session user in our
* script is
*/
- if (AH->currUser)
- free(AH->currUser);
+ free(AH->currUser);
AH->currUser = pg_strdup(user);
}
@@ -3285,8 +3278,7 @@ _selectOutputSchema(ArchiveHandle *AH, const char *schemaName)
else
ahprintf(AH, "%s;\n\n", qry->data);
- if (AH->currSchema)
- free(AH->currSchema);
+ free(AH->currSchema);
AH->currSchema = pg_strdup(schemaName);
destroyPQExpBuffer(qry);
@@ -3347,8 +3339,7 @@ _selectTablespace(ArchiveHandle *AH, const char *tablespace)
else
ahprintf(AH, "%s;\n\n", qry->data);
- if (AH->currTablespace)
- free(AH->currTablespace);
+ free(AH->currTablespace);
AH->currTablespace = pg_strdup(want);
destroyPQExpBuffer(qry);
@@ -3399,8 +3390,7 @@ _selectTableAccessMethod(ArchiveHandle *AH, const char *tableam)
destroyPQExpBuffer(cmd);
- if (AH->currTableAm)
- free(AH->currTableAm);
+ free(AH->currTableAm);
AH->currTableAm = pg_strdup(want);
}
@@ -3659,8 +3649,7 @@ _printTocEntry(ArchiveHandle *AH, TocEntry *te, bool isData)
*/
if (_tocEntryIsACL(te))
{
- if (AH->currUser)
- free(AH->currUser);
+ free(AH->currUser);
AH->currUser = NULL;
}
}
@@ -3991,17 +3980,13 @@ restore_toc_entries_prefork(ArchiveHandle *AH, TocEntry *pending_list)
DisconnectDatabase(&AH->public);
/* blow away any transient state from the old connection */
- if (AH->currUser)
- free(AH->currUser);
+ free(AH->currUser);
AH->currUser = NULL;
- if (AH->currSchema)
- free(AH->currSchema);
+ free(AH->currSchema);
AH->currSchema = NULL;
- if (AH->currTablespace)
- free(AH->currTablespace);
+ free(AH->currTablespace);
AH->currTablespace = NULL;
- if (AH->currTableAm)
- free(AH->currTableAm);
+ free(AH->currTableAm);
AH->currTableAm = NULL;
}
@@ -4842,16 +4827,11 @@ DeCloneArchive(ArchiveHandle *AH)
destroyPQExpBuffer(AH->sqlparse.curCmd);
/* Clear any connection-local state */
- if (AH->currUser)
- free(AH->currUser);
- if (AH->currSchema)
- free(AH->currSchema);
- if (AH->currTablespace)
- free(AH->currTablespace);
- if (AH->currTableAm)
- free(AH->currTableAm);
- if (AH->savedPassword)
- free(AH->savedPassword);
+ free(AH->currUser);
+ free(AH->currSchema);
+ free(AH->currTablespace);
+ free(AH->currTableAm);
+ free(AH->savedPassword);
free(AH);
}
diff --git a/src/bin/pg_dump/pg_backup_custom.c b/src/bin/pg_dump/pg_backup_custom.c
index 3443eef6b0..1023fea01b 100644
--- a/src/bin/pg_dump/pg_backup_custom.c
+++ b/src/bin/pg_dump/pg_backup_custom.c
@@ -632,8 +632,7 @@ _skipData(ArchiveHandle *AH)
{
if (blkLen > buflen)
{
- if (buf)
- free(buf);
+ free(buf);
buf = (char *) pg_malloc(blkLen);
buflen = blkLen;
}
@@ -649,8 +648,7 @@ _skipData(ArchiveHandle *AH)
blkLen = ReadInt(AH);
}
- if (buf)
- free(buf);
+ free(buf);
}
/*
diff --git a/src/bin/pg_dump/pg_backup_db.c b/src/bin/pg_dump/pg_backup_db.c
index 89cdbf80e0..28baa68fd4 100644
--- a/src/bin/pg_dump/pg_backup_db.c
+++ b/src/bin/pg_dump/pg_backup_db.c
@@ -204,8 +204,7 @@ ConnectDatabase(Archive *AHX,
*/
if (PQconnectionUsedPassword(AH->connection))
{
- if (AH->savedPassword)
- free(AH->savedPassword);
+ free(AH->savedPassword);
AH->savedPassword = pg_strdup(PQpass(AH->connection));
}
diff --git a/src/bin/pg_dump/pg_backup_tar.c b/src/bin/pg_dump/pg_backup_tar.c
index 39d71badb7..bfc49b66d2 100644
--- a/src/bin/pg_dump/pg_backup_tar.c
+++ b/src/bin/pg_dump/pg_backup_tar.c
@@ -412,8 +412,7 @@ tarClose(ArchiveHandle *AH, TAR_MEMBER *th)
* handle, and we don't use temp files.
*/
- if (th->targetFile)
- free(th->targetFile);
+ free(th->targetFile);
th->nFH = NULL;
}
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 7cc9c72e49..c871cb727d 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -3339,8 +3339,7 @@ dumpSearchPath(Archive *AH)
/* Also save it in AH->searchpath, in case we're doing plain text dump */
AH->searchpath = pg_strdup(qry->data);
- if (schemanames)
- free(schemanames);
+ free(schemanames);
PQclear(res);
destroyPQExpBuffer(qry);
destroyPQExpBuffer(path);
@@ -4614,8 +4613,7 @@ dumpSubscription(Archive *fout, const SubscriptionInfo *subinfo)
subinfo->dobj.catId, 0, subinfo->dobj.dumpId);
destroyPQExpBuffer(publications);
- if (pubnames)
- free(pubnames);
+ free(pubnames);
destroyPQExpBuffer(delq);
destroyPQExpBuffer(query);
@@ -11908,12 +11906,10 @@ dumpFunc(Archive *fout, const FuncInfo *finfo)
destroyPQExpBuffer(delqry);
destroyPQExpBuffer(asPart);
free(funcsig);
- if (funcfullsig)
- free(funcfullsig);
+ free(funcfullsig);
free(funcsig_tag);
free(qual_funcsig);
- if (configitems)
- free(configitems);
+ free(configitems);
}
@@ -13658,8 +13654,7 @@ dumpAgg(Archive *fout, const AggInfo *agginfo)
agginfo->aggfn.rolname, &agginfo->aggfn.dacl);
free(aggsig);
- if (aggfullsig)
- free(aggfullsig);
+ free(aggfullsig);
free(aggsig_tag);
PQclear(res);
@@ -15713,12 +15708,9 @@ dumpTableSchema(Archive *fout, const TableInfo *tbinfo)
tbinfo->attfdwoptions[j]);
} /* end loop over columns */
- if (partkeydef)
- free(partkeydef);
- if (ftoptions)
- free(ftoptions);
- if (srvname)
- free(srvname);
+ free(partkeydef);
+ free(ftoptions);
+ free(srvname);
}
/*
@@ -16105,10 +16097,8 @@ dumpIndex(Archive *fout, const IndxInfo *indxinfo)
.createStmt = q->data,
.dropStmt = delq->data));
- if (indstatcolsarray)
- free(indstatcolsarray);
- if (indstatvalsarray)
- free(indstatvalsarray);
+ free(indstatcolsarray);
+ free(indstatvalsarray);
}
/* Dump Index Comments */
diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c
index ae41a652d7..da5cf85272 100644
--- a/src/bin/pg_dump/pg_dumpall.c
+++ b/src/bin/pg_dump/pg_dumpall.c
@@ -1500,10 +1500,8 @@ connectDatabase(const char *dbname, const char *connection_string,
char *err_msg = NULL;
int i = 0;
- if (keywords)
- free(keywords);
- if (values)
- free(values);
+ free(keywords);
+ free(values);
if (conn_opts)
PQconninfoFree(conn_opts);
diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index fbb74bdc4c..36b5c8fff7 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -1852,8 +1852,7 @@ putVariable(Variables *variables, const char *context, char *name,
/* dup then free, in case value is pointing at this variable */
val = pg_strdup(value);
- if (var->svalue)
- free(var->svalue);
+ free(var->svalue);
var->svalue = val;
var->value.type = PGBT_NO_VALUE;
@@ -1872,8 +1871,7 @@ putVariableValue(Variables *variables, const char *context, char *name,
if (!var)
return false;
- if (var->svalue)
- free(var->svalue);
+ free(var->svalue);
var->svalue = NULL;
var->value = *value;
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index b51d28780b..4495f7a907 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -595,8 +595,7 @@ exec_command_cd(PsqlScanState scan_state, bool active_branch, const char *cmd)
success = false;
}
- if (opt)
- free(opt);
+ free(opt);
}
else
ignore_slash_options(scan_state);
@@ -769,8 +768,7 @@ exec_command_d(PsqlScanState scan_state, bool active_branch, const char *cmd)
break;
}
- if (pattern2)
- free(pattern2);
+ free(pattern2);
}
break;
case 'a':
@@ -881,8 +879,7 @@ exec_command_d(PsqlScanState scan_state, bool active_branch, const char *cmd)
OT_NORMAL, NULL, true);
success = listDbRoleSettings(pattern, pattern2);
- if (pattern2)
- free(pattern2);
+ free(pattern2);
}
else
status = PSQL_CMD_UNKNOWN;
@@ -963,8 +960,7 @@ exec_command_d(PsqlScanState scan_state, bool active_branch, const char *cmd)
status = PSQL_CMD_UNKNOWN;
}
- if (pattern)
- free(pattern);
+ free(pattern);
}
else
ignore_slash_options(scan_state);
@@ -1092,10 +1088,8 @@ exec_command_edit(PsqlScanState scan_state, bool active_branch,
else
status = PSQL_CMD_ERROR;
}
- if (fname)
- free(fname);
- if (ln)
- free(ln);
+ free(fname);
+ free(ln);
}
}
else
@@ -1204,8 +1198,7 @@ exec_command_ef_ev(PsqlScanState scan_state, bool active_branch,
status = PSQL_CMD_NEWEDIT;
}
- if (obj_desc)
- free(obj_desc);
+ free(obj_desc);
}
else
ignore_slash_whole_line(scan_state);
@@ -1920,8 +1913,7 @@ exec_command_list(PsqlScanState scan_state, bool active_branch, const char *cmd)
success = listAllDbs(pattern, show_verbose);
- if (pattern)
- free(pattern);
+ free(pattern);
}
else
ignore_slash_options(scan_state);
@@ -2136,10 +2128,8 @@ exec_command_password(PsqlScanState scan_state, bool active_branch)
}
free(user);
- if (pw1)
- free(pw1);
- if (pw2)
- free(pw2);
+ free(pw1);
+ free(pw2);
termPQExpBuffer(&buf);
}
else
@@ -2214,10 +2204,8 @@ exec_command_prompt(PsqlScanState scan_state, bool active_branch,
(result && !SetVariable(pset.vars, opt, result)))
success = false;
- if (result)
- free(result);
- if (prompt_text)
- free(prompt_text);
+ free(result);
+ free(prompt_text);
free(opt);
}
}
@@ -2522,8 +2510,7 @@ exec_command_sf_sv(PsqlScanState scan_state, bool active_branch,
ClosePager(output);
}
- if (obj_desc)
- free(obj_desc);
+ free(obj_desc);
destroyPQExpBuffer(buf);
}
else
@@ -2802,8 +2789,7 @@ exec_command_z(PsqlScanState scan_state, bool active_branch)
OT_NORMAL, NULL, true);
success = permissionsList(pattern);
- if (pattern)
- free(pattern);
+ free(pattern);
}
else
ignore_slash_options(scan_state);
@@ -2853,8 +2839,7 @@ exec_command_slash_command_help(PsqlScanState scan_state, bool active_branch)
else
slashUsage(pset.popt.topt.pager);
- if (opt0)
- free(opt0);
+ free(opt0);
}
else
ignore_slash_options(scan_state);
@@ -2994,8 +2979,7 @@ ignore_slash_filepipe(PsqlScanState scan_state)
char *arg = psql_scan_slash_option(scan_state,
OT_FILEPIPE, NULL, false);
- if (arg)
- free(arg);
+ free(arg);
}
/*
@@ -3011,8 +2995,7 @@ ignore_slash_whole_line(PsqlScanState scan_state)
char *arg = psql_scan_slash_option(scan_state,
OT_WHOLE_LINE, NULL, false);
- if (arg)
- free(arg);
+ free(arg);
}
/*
@@ -4780,16 +4763,11 @@ restorePsetInfo(printQueryOpt *popt, printQueryOpt *save)
/* Free all the old data we're about to overwrite the pointers to. */
/* topt.line_style points to const data that need not be duplicated */
- if (popt->topt.fieldSep.separator)
- free(popt->topt.fieldSep.separator);
- if (popt->topt.recordSep.separator)
- free(popt->topt.recordSep.separator);
- if (popt->topt.tableAttr)
- free(popt->topt.tableAttr);
- if (popt->nullPrint)
- free(popt->nullPrint);
- if (popt->title)
- free(popt->title);
+ free(popt->topt.fieldSep.separator);
+ free(popt->topt.recordSep.separator);
+ free(popt->topt.tableAttr);
+ free(popt->nullPrint);
+ free(popt->title);
/*
* footers and translate_columns are never set in psql's print settings,
diff --git a/src/bin/psql/copy.c b/src/bin/psql/copy.c
index 424a429e1e..c181682a13 100644
--- a/src/bin/psql/copy.c
+++ b/src/bin/psql/copy.c
@@ -389,8 +389,7 @@ do_copy(const char *args)
pg_log_error("%s: %s", options->file,
reason ? reason : "");
- if (reason)
- free(reason);
+ free(reason);
}
success = false;
}
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index 1a5d924a23..7a21777d9e 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -1786,8 +1786,7 @@ describeOneTableDetails(const char *schemaname,
printQuery(res, &myopt, pset.queryFout, false, pset.logfile);
- if (footers[0])
- free(footers[0]);
+ free(footers[0]);
retval = true;
goto error_return; /* not an error, just return early */
@@ -3491,8 +3490,7 @@ describeOneTableDetails(const char *schemaname,
termPQExpBuffer(&title);
termPQExpBuffer(&tmpbuf);
- if (view_def)
- free(view_def);
+ free(view_def);
if (res)
PQclear(res);
diff --git a/src/bin/psql/input.c b/src/bin/psql/input.c
index 416185d659..6cc7ddda71 100644
--- a/src/bin/psql/input.c
+++ b/src/bin/psql/input.c
@@ -158,8 +158,7 @@ pg_send_history(PQExpBuffer history_buf)
else
{
/* Save each previous line for ignoredups processing */
- if (prev_hist)
- free(prev_hist);
+ free(prev_hist);
prev_hist = pg_strdup(s);
/* And send it to readline */
add_history(s);
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index e1cc753489..8452b3dad1 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -4759,11 +4759,9 @@ psql_completion(const char *text, int start, int end)
free(previous_words);
free(words_buffer);
free(text_copy);
- if (completion_ref_object)
- free(completion_ref_object);
+ free(completion_ref_object);
completion_ref_object = NULL;
- if (completion_ref_schema)
- free(completion_ref_schema);
+ free(completion_ref_schema);
completion_ref_schema = NULL;
/* Return our Grand List O' Matches */
@@ -5165,12 +5163,9 @@ _complete_from_query(const char *simple_query,
/* Clean up */
termPQExpBuffer(&query_buffer);
free(e_object_like);
- if (e_schemaname)
- free(e_schemaname);
- if (e_ref_object)
- free(e_ref_object);
- if (e_ref_schema)
- free(e_ref_schema);
+ free(e_schemaname);
+ free(e_ref_object);
+ free(e_ref_schema);
}
/* Return the next result, if any, but not if the query failed */
diff --git a/src/common/fe_memutils.c b/src/common/fe_memutils.c
index 21fcce5430..b1e6c65576 100644
--- a/src/common/fe_memutils.c
+++ b/src/common/fe_memutils.c
@@ -104,8 +104,7 @@ pg_strdup(const char *in)
void
pg_free(void *ptr)
{
- if (ptr != NULL)
- free(ptr);
+ free(ptr);
}
/*
diff --git a/src/fe_utils/connect_utils.c b/src/fe_utils/connect_utils.c
index f2e583f9fa..1cc97b72f7 100644
--- a/src/fe_utils/connect_utils.c
+++ b/src/fe_utils/connect_utils.c
@@ -99,8 +99,7 @@ connectDatabase(const ConnParams *cparams, const char *progname,
cparams->prompt_password != TRI_NO)
{
PQfinish(conn);
- if (password)
- free(password);
+ free(password);
password = simple_prompt("Password: ", false);
new_pass = true;
}
diff --git a/src/fe_utils/string_utils.c b/src/fe_utils/string_utils.c
index c3ea4fc186..f9ea08705a 100644
--- a/src/fe_utils/string_utils.c
+++ b/src/fe_utils/string_utils.c
@@ -810,8 +810,7 @@ appendReloptionsArray(PQExpBuffer buffer, const char *reloptions,
if (!parsePGArray(reloptions, &options, &noptions))
{
- if (options)
- free(options);
+ free(options);
return false;
}
@@ -854,8 +853,7 @@ appendReloptionsArray(PQExpBuffer buffer, const char *reloptions,
appendStringLiteral(buffer, value, encoding, std_strings);
}
- if (options)
- free(options);
+ free(options);
return true;
}
diff --git a/src/interfaces/ecpg/pgtypeslib/numeric.c b/src/interfaces/ecpg/pgtypeslib/numeric.c
index 34efc9045f..a97b3300cb 100644
--- a/src/interfaces/ecpg/pgtypeslib/numeric.c
+++ b/src/interfaces/ecpg/pgtypeslib/numeric.c
@@ -16,11 +16,7 @@
#define init_var(v) memset(v,0,sizeof(numeric))
#define digitbuf_alloc(size) ((NumericDigit *) pgtypes_alloc(size))
-#define digitbuf_free(buf) \
- do { \
- if ((buf) != NULL) \
- free(buf); \
- } while (0)
+#define digitbuf_free(buf) free(buf)
/* ----------
diff --git a/src/interfaces/ecpg/preproc/descriptor.c b/src/interfaces/ecpg/preproc/descriptor.c
index 35d94711d5..f4b1878289 100644
--- a/src/interfaces/ecpg/preproc/descriptor.c
+++ b/src/interfaces/ecpg/preproc/descriptor.c
@@ -113,8 +113,7 @@ drop_descriptor(char *name, char *connection)
&& strcmp(connection, i->connection) == 0))
{
*lastptr = i->next;
- if (i->connection)
- free(i->connection);
+ free(i->connection);
free(i->name);
free(i);
return;
diff --git a/src/interfaces/libpq/fe-auth-scram.c b/src/interfaces/libpq/fe-auth-scram.c
index e616200704..5012806fa5 100644
--- a/src/interfaces/libpq/fe-auth-scram.c
+++ b/src/interfaces/libpq/fe-auth-scram.c
@@ -174,30 +174,21 @@ scram_free(void *opaq)
{
fe_scram_state *state = (fe_scram_state *) opaq;
- if (state->password)
- free(state->password);
- if (state->sasl_mechanism)
- free(state->sasl_mechanism);
+ free(state->password);
+ free(state->sasl_mechanism);
/* client messages */
- if (state->client_nonce)
- free(state->client_nonce);
- if (state->client_first_message_bare)
- free(state->client_first_message_bare);
- if (state->client_final_message_without_proof)
- free(state->client_final_message_without_proof);
+ free(state->client_nonce);
+ free(state->client_first_message_bare);
+ free(state->client_final_message_without_proof);
/* first message from server */
- if (state->server_first_message)
- free(state->server_first_message);
- if (state->salt)
- free(state->salt);
- if (state->nonce)
- free(state->nonce);
+ free(state->server_first_message);
+ free(state->salt);
+ free(state->nonce);
/* final message from server */
- if (state->server_final_message)
- free(state->server_final_message);
+ free(state->server_final_message);
free(state);
}
@@ -941,8 +932,7 @@ pg_fe_scram_build_secret(const char *password, const char **errstr)
if (!pg_strong_random(saltbuf, SCRAM_DEFAULT_SALT_LEN))
{
*errstr = _("failed to generate random salt");
- if (prep_password)
- free(prep_password);
+ free(prep_password);
return NULL;
}
@@ -950,8 +940,7 @@ pg_fe_scram_build_secret(const char *password, const char **errstr)
SCRAM_DEFAULT_ITERATIONS, password,
errstr);
- if (prep_password)
- free(prep_password);
+ free(prep_password);
return result;
}
diff --git a/src/interfaces/libpq/fe-auth.c b/src/interfaces/libpq/fe-auth.c
index 0a072a36dc..49a1c626f6 100644
--- a/src/interfaces/libpq/fe-auth.c
+++ b/src/interfaces/libpq/fe-auth.c
@@ -107,8 +107,7 @@ pg_GSS_continue(PGconn *conn, int payloadlen)
NULL,
NULL);
- if (ginbuf.value)
- free(ginbuf.value);
+ free(ginbuf.value);
if (goutbuf.length != 0)
{
@@ -270,8 +269,7 @@ pg_SSPI_continue(PGconn *conn, int payloadlen)
NULL);
/* we don't need the input anymore */
- if (inputbuf)
- free(inputbuf);
+ free(inputbuf);
if (r != SEC_E_OK && r != SEC_I_CONTINUE_NEEDED)
{
@@ -604,21 +602,18 @@ pg_SASL_init(PGconn *conn, int payloadlen)
goto error;
termPQExpBuffer(&mechanism_buf);
- if (initialresponse)
- free(initialresponse);
+ free(initialresponse);
return STATUS_OK;
error:
termPQExpBuffer(&mechanism_buf);
- if (initialresponse)
- free(initialresponse);
+ free(initialresponse);
return STATUS_ERROR;
oom_error:
termPQExpBuffer(&mechanism_buf);
- if (initialresponse)
- free(initialresponse);
+ free(initialresponse);
appendPQExpBufferStr(&conn->errorMessage,
libpq_gettext("out of memory\n"));
return STATUS_ERROR;
@@ -831,8 +826,7 @@ pg_password_sendauth(PGconn *conn, const char *password, AuthRequest areq)
return STATUS_ERROR;
}
ret = pqPacketSend(conn, 'p', pwd_to_send, strlen(pwd_to_send) + 1);
- if (crypt_pwd)
- free(crypt_pwd);
+ free(crypt_pwd);
return ret;
}
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index 6e936bbff3..057c9da0ed 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -540,8 +540,7 @@ pqFreeCommandQueue(PGcmdQueueEntry *queue)
PGcmdQueueEntry *cur = queue;
queue = cur->next;
- if (cur->query)
- free(cur->query);
+ free(cur->query);
free(cur);
}
}
@@ -593,8 +592,7 @@ pqDropServerData(PGconn *conn)
conn->sversion = 0;
/* Drop large-object lookup data */
- if (conn->lobjfuncs)
- free(conn->lobjfuncs);
+ free(conn->lobjfuncs);
conn->lobjfuncs = NULL;
/* Reset assorted other per-connection state */
@@ -602,8 +600,7 @@ pqDropServerData(PGconn *conn)
conn->auth_req_received = false;
conn->password_needed = false;
conn->write_failed = false;
- if (conn->write_err_msg)
- free(conn->write_err_msg);
+ free(conn->write_err_msg);
conn->write_err_msg = NULL;
conn->be_pid = 0;
conn->be_key = 0;
@@ -898,8 +895,7 @@ fillPGconn(PGconn *conn, PQconninfoOption *connOptions)
{
char **connmember = (char **) ((char *) conn + option->connofs);
- if (*connmember)
- free(*connmember);
+ free(*connmember);
*connmember = strdup(tmp);
if (*connmember == NULL)
{
@@ -1113,8 +1109,7 @@ connectOptions2(PGconn *conn)
}
else
{
- if (ch->host)
- free(ch->host);
+ free(ch->host);
/*
* This bit selects the default host location. If you change
@@ -1186,8 +1181,7 @@ connectOptions2(PGconn *conn)
*/
if (conn->pguser == NULL || conn->pguser[0] == '\0')
{
- if (conn->pguser)
- free(conn->pguser);
+ free(conn->pguser);
conn->pguser = pg_fe_getauthname(&conn->errorMessage);
if (!conn->pguser)
{
@@ -1201,8 +1195,7 @@ connectOptions2(PGconn *conn)
*/
if (conn->dbName == NULL || conn->dbName[0] == '\0')
{
- if (conn->dbName)
- free(conn->dbName);
+ free(conn->dbName);
conn->dbName = strdup(conn->pguser);
if (!conn->dbName)
goto oom_error;
@@ -1221,8 +1214,7 @@ connectOptions2(PGconn *conn)
if (pqGetHomeDirectory(homedir, sizeof(homedir)))
{
- if (conn->pgpassfile)
- free(conn->pgpassfile);
+ free(conn->pgpassfile);
conn->pgpassfile = malloc(MAXPGPATH);
if (!conn->pgpassfile)
goto oom_error;
@@ -1548,8 +1540,7 @@ PQsetdbLogin(const char *pghost, const char *pgport, const char *pgoptions,
/* Insert dbName parameter value into struct */
if (dbName && dbName[0] != '\0')
{
- if (conn->dbName)
- free(conn->dbName);
+ free(conn->dbName);
conn->dbName = strdup(dbName);
if (!conn->dbName)
goto oom_error;
@@ -1562,8 +1553,7 @@ PQsetdbLogin(const char *pghost, const char *pgport, const char *pgoptions,
*/
if (pghost && pghost[0] != '\0')
{
- if (conn->pghost)
- free(conn->pghost);
+ free(conn->pghost);
conn->pghost = strdup(pghost);
if (!conn->pghost)
goto oom_error;
@@ -1571,8 +1561,7 @@ PQsetdbLogin(const char *pghost, const char *pgport, const char *pgoptions,
if (pgport && pgport[0] != '\0')
{
- if (conn->pgport)
- free(conn->pgport);
+ free(conn->pgport);
conn->pgport = strdup(pgport);
if (!conn->pgport)
goto oom_error;
@@ -1580,8 +1569,7 @@ PQsetdbLogin(const char *pghost, const char *pgport, const char *pgoptions,
if (pgoptions && pgoptions[0] != '\0')
{
- if (conn->pgoptions)
- free(conn->pgoptions);
+ free(conn->pgoptions);
conn->pgoptions = strdup(pgoptions);
if (!conn->pgoptions)
goto oom_error;
@@ -1589,8 +1577,7 @@ PQsetdbLogin(const char *pghost, const char *pgport, const char *pgoptions,
if (login && login[0] != '\0')
{
- if (conn->pguser)
- free(conn->pguser);
+ free(conn->pguser);
conn->pguser = strdup(login);
if (!conn->pguser)
goto oom_error;
@@ -1598,8 +1585,7 @@ PQsetdbLogin(const char *pghost, const char *pgport, const char *pgoptions,
if (pwd && pwd[0] != '\0')
{
- if (conn->pgpass)
- free(conn->pgpass);
+ free(conn->pgpass);
conn->pgpass = strdup(pwd);
if (!conn->pgpass)
goto oom_error;
@@ -4044,10 +4030,8 @@ makeEmptyPGconn(void)
static void
freePGconn(PGconn *conn)
{
- int i;
-
/* let any event procs clean up their state data */
- for (i = 0; i < conn->nEvents; i++)
+ for (int i = 0; i < conn->nEvents; i++)
{
PGEventConnDestroy evt;
@@ -4058,114 +4042,69 @@ freePGconn(PGconn *conn)
}
/* clean up pg_conn_host structures */
- if (conn->connhost != NULL)
+ for (int i = 0; i < conn->nconnhost; ++i)
{
- for (i = 0; i < conn->nconnhost; ++i)
+ free(conn->connhost[i].host);
+ free(conn->connhost[i].hostaddr);
+ free(conn->connhost[i].port);
+ if (conn->connhost[i].password != NULL)
{
- if (conn->connhost[i].host != NULL)
- free(conn->connhost[i].host);
- if (conn->connhost[i].hostaddr != NULL)
- free(conn->connhost[i].hostaddr);
- if (conn->connhost[i].port != NULL)
- free(conn->connhost[i].port);
- if (conn->connhost[i].password != NULL)
- {
- explicit_bzero(conn->connhost[i].password, strlen(conn->connhost[i].password));
- free(conn->connhost[i].password);
- }
+ explicit_bzero(conn->connhost[i].password, strlen(conn->connhost[i].password));
+ free(conn->connhost[i].password);
}
- free(conn->connhost);
}
-
- if (conn->client_encoding_initial)
- free(conn->client_encoding_initial);
- if (conn->events)
- free(conn->events);
- if (conn->pghost)
- free(conn->pghost);
- if (conn->pghostaddr)
- free(conn->pghostaddr);
- if (conn->pgport)
- free(conn->pgport);
- if (conn->connect_timeout)
- free(conn->connect_timeout);
- if (conn->pgtcp_user_timeout)
- free(conn->pgtcp_user_timeout);
- if (conn->pgoptions)
- free(conn->pgoptions);
- if (conn->appname)
- free(conn->appname);
- if (conn->fbappname)
- free(conn->fbappname);
- if (conn->dbName)
- free(conn->dbName);
- if (conn->replication)
- free(conn->replication);
- if (conn->pguser)
- free(conn->pguser);
+ free(conn->connhost);
+
+ free(conn->client_encoding_initial);
+ free(conn->events);
+ free(conn->pghost);
+ free(conn->pghostaddr);
+ free(conn->pgport);
+ free(conn->connect_timeout);
+ free(conn->pgtcp_user_timeout);
+ free(conn->pgoptions);
+ free(conn->appname);
+ free(conn->fbappname);
+ free(conn->dbName);
+ free(conn->replication);
+ free(conn->pguser);
if (conn->pgpass)
{
explicit_bzero(conn->pgpass, strlen(conn->pgpass));
free(conn->pgpass);
}
- if (conn->pgpassfile)
- free(conn->pgpassfile);
- if (conn->channel_binding)
- free(conn->channel_binding);
- if (conn->keepalives)
- free(conn->keepalives);
- if (conn->keepalives_idle)
- free(conn->keepalives_idle);
- if (conn->keepalives_interval)
- free(conn->keepalives_interval);
- if (conn->keepalives_count)
- free(conn->keepalives_count);
- if (conn->sslmode)
- free(conn->sslmode);
- if (conn->sslcert)
- free(conn->sslcert);
- if (conn->sslkey)
- free(conn->sslkey);
+ free(conn->pgpassfile);
+ free(conn->channel_binding);
+ free(conn->keepalives);
+ free(conn->keepalives_idle);
+ free(conn->keepalives_interval);
+ free(conn->keepalives_count);
+ free(conn->sslmode);
+ free(conn->sslcert);
+ free(conn->sslkey);
if (conn->sslpassword)
{
explicit_bzero(conn->sslpassword, strlen(conn->sslpassword));
free(conn->sslpassword);
}
- if (conn->sslrootcert)
- free(conn->sslrootcert);
- if (conn->sslcrl)
- free(conn->sslcrl);
- if (conn->sslcrldir)
- free(conn->sslcrldir);
- if (conn->sslcompression)
- free(conn->sslcompression);
- if (conn->sslsni)
- free(conn->sslsni);
- if (conn->requirepeer)
- free(conn->requirepeer);
- if (conn->ssl_min_protocol_version)
- free(conn->ssl_min_protocol_version);
- if (conn->ssl_max_protocol_version)
- free(conn->ssl_max_protocol_version);
- if (conn->gssencmode)
- free(conn->gssencmode);
- if (conn->krbsrvname)
- free(conn->krbsrvname);
- if (conn->gsslib)
- free(conn->gsslib);
- if (conn->connip)
- free(conn->connip);
+ free(conn->sslrootcert);
+ free(conn->sslcrl);
+ free(conn->sslcrldir);
+ free(conn->sslcompression);
+ free(conn->sslsni);
+ free(conn->requirepeer);
+ free(conn->ssl_min_protocol_version);
+ free(conn->ssl_max_protocol_version);
+ free(conn->gssencmode);
+ free(conn->krbsrvname);
+ free(conn->gsslib);
+ free(conn->connip);
/* Note that conn->Pfdebug is not ours to close or free */
- if (conn->write_err_msg)
- free(conn->write_err_msg);
- if (conn->inBuffer)
- free(conn->inBuffer);
- if (conn->outBuffer)
- free(conn->outBuffer);
- if (conn->rowBuf)
- free(conn->rowBuf);
- if (conn->target_session_attrs)
- free(conn->target_session_attrs);
+ free(conn->write_err_msg);
+ free(conn->inBuffer);
+ free(conn->outBuffer);
+ free(conn->rowBuf);
+ free(conn->target_session_attrs);
termPQExpBuffer(&conn->errorMessage);
termPQExpBuffer(&conn->workBuffer);
@@ -4433,8 +4372,7 @@ PQgetCancel(PGconn *conn)
void
PQfreeCancel(PGcancel *cancel)
{
- if (cancel)
- free(cancel);
+ free(cancel);
}
@@ -5883,8 +5821,7 @@ conninfo_array_parse(const char *const *keywords, const char *const *values,
{
if (strcmp(options[k].keyword, str_option->keyword) == 0)
{
- if (options[k].val)
- free(options[k].val);
+ free(options[k].val);
options[k].val = strdup(str_option->val);
if (!options[k].val)
{
@@ -5912,8 +5849,7 @@ conninfo_array_parse(const char *const *keywords, const char *const *values,
/*
* Store the value, overriding previous settings
*/
- if (option->val)
- free(option->val);
+ free(option->val);
option->val = strdup(pvalue);
if (!option->val)
{
@@ -6344,8 +6280,7 @@ conninfo_uri_parse_options(PQconninfoOption *options, const char *uri,
cleanup:
termPQExpBuffer(&hostbuf);
termPQExpBuffer(&portbuf);
- if (buf)
- free(buf);
+ free(buf);
return retval;
}
@@ -6655,8 +6590,7 @@ conninfo_storeval(PQconninfoOption *connOptions,
}
}
- if (option->val)
- free(option->val);
+ free(option->val);
option->val = value_copy;
return option;
@@ -6735,16 +6669,11 @@ PQconninfo(PGconn *conn)
void
PQconninfoFree(PQconninfoOption *connOptions)
{
- PQconninfoOption *option;
-
if (connOptions == NULL)
return;
- for (option = connOptions; option->keyword != NULL; option++)
- {
- if (option->val != NULL)
- free(option->val);
- }
+ for (PQconninfoOption *option = connOptions; option->keyword != NULL; option++)
+ free(option->val);
free(connOptions);
}
diff --git a/src/interfaces/libpq/fe-exec.c b/src/interfaces/libpq/fe-exec.c
index 919cf5741d..1750d647a8 100644
--- a/src/interfaces/libpq/fe-exec.c
+++ b/src/interfaces/libpq/fe-exec.c
@@ -742,8 +742,7 @@ PQclear(PGresult *res)
free(res->events[i].name);
}
- if (res->events)
- free(res->events);
+ free(res->events);
/* Free all the subsidiary blocks */
while ((block = res->curBlock) != NULL)
@@ -753,8 +752,7 @@ PQclear(PGresult *res)
}
/* Free the top-level tuple pointer array */
- if (res->tuples)
- free(res->tuples);
+ free(res->tuples);
/* zero out the pointer fields to catch programming errors */
res->attDescs = NULL;
diff --git a/src/interfaces/libpq/fe-print.c b/src/interfaces/libpq/fe-print.c
index 82fc592f06..783cd9b756 100644
--- a/src/interfaces/libpq/fe-print.c
+++ b/src/interfaces/libpq/fe-print.c
@@ -303,26 +303,19 @@ PQprint(FILE *fout, const PGresult *res, const PQprintOpt *po)
fputs("</table>\n", fout);
exit:
- if (fieldMax)
- free(fieldMax);
- if (fieldNotNum)
- free(fieldNotNum);
- if (border)
- free(border);
+ free(fieldMax);
+ free(fieldNotNum);
+ free(border);
if (fields)
{
/* if calloc succeeded, this shouldn't overflow size_t */
size_t numfields = ((size_t) nTups + 1) * (size_t) nFields;
while (numfields-- > 0)
- {
- if (fields[numfields])
- free(fields[numfields]);
- }
+ free(fields[numfields]);
free(fields);
}
- if (fieldNames)
- free((void *) fieldNames);
+ free(fieldNames);
if (usePipe)
{
#ifdef WIN32
@@ -679,8 +672,7 @@ PQdisplayTuples(const PGresult *res,
fflush(fp);
- if (fLength)
- free(fLength);
+ free(fLength);
}
@@ -763,8 +755,7 @@ PQprintTuples(const PGresult *res,
}
}
- if (tborder)
- free(tborder);
+ free(tborder);
}
diff --git a/src/interfaces/libpq/fe-secure-common.c b/src/interfaces/libpq/fe-secure-common.c
index 8046fcd884..cc8a2b8593 100644
--- a/src/interfaces/libpq/fe-secure-common.c
+++ b/src/interfaces/libpq/fe-secure-common.c
@@ -309,8 +309,7 @@ pq_verify_peer_name_matches_certificate(PGconn *conn)
}
/* clean up */
- if (first_name)
- free(first_name);
+ free(first_name);
return (rc == 1);
}
diff --git a/src/port/getaddrinfo.c b/src/port/getaddrinfo.c
index 3284c6eb52..bea7b520f0 100644
--- a/src/port/getaddrinfo.c
+++ b/src/port/getaddrinfo.c
@@ -270,8 +270,7 @@ freeaddrinfo(struct addrinfo *res)
}
#endif
- if (res->ai_addr)
- free(res->ai_addr);
+ free(res->ai_addr);
free(res);
}
}
--
2.36.1
v2-0002-Remove-redundant-null-pointer-checks-before-pg_fr.patchtext/plain; charset=UTF-8; name=v2-0002-Remove-redundant-null-pointer-checks-before-pg_fr.patchDownload
From 6fc396a32347edf4fec7f772300ff13360641700 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Fri, 17 Jun 2022 11:51:38 +0200
Subject: [PATCH v2 2/3] Remove redundant null pointer checks before pg_free()
This deserves shaming in a separate commit because the whole point of
pg_free() was to do that very check before calling free().
Logically, pg_free() should be removed, but I'm keeping it here to
keep the API consistent.
---
src/bin/pg_basebackup/walmethods.c | 3 +--
src/bin/pg_upgrade/parallel.c | 18 ++++++------------
src/bin/pgbench/pgbench.c | 9 +++------
src/bin/psql/command.c | 3 +--
src/bin/psql/variables.c | 5 ++---
5 files changed, 13 insertions(+), 25 deletions(-)
diff --git a/src/bin/pg_basebackup/walmethods.c b/src/bin/pg_basebackup/walmethods.c
index cc292718da..ef4c11277a 100644
--- a/src/bin/pg_basebackup/walmethods.c
+++ b/src/bin/pg_basebackup/walmethods.c
@@ -500,8 +500,7 @@ dir_close(Walfile f, WalCloseMethod method)
pg_free(df->pathname);
pg_free(df->fullpath);
- if (df->temp_suffix)
- pg_free(df->temp_suffix);
+ pg_free(df->temp_suffix);
pg_free(df);
return r;
diff --git a/src/bin/pg_upgrade/parallel.c b/src/bin/pg_upgrade/parallel.c
index ca40df7b4c..5a3df84f01 100644
--- a/src/bin/pg_upgrade/parallel.c
+++ b/src/bin/pg_upgrade/parallel.c
@@ -130,14 +130,11 @@ parallel_exec_prog(const char *log_file, const char *opt_log_file,
new_arg = exec_thread_args[parallel_jobs - 1];
/* Can only pass one pointer into the function, so use a struct */
- if (new_arg->log_file)
- pg_free(new_arg->log_file);
+ pg_free(new_arg->log_file);
new_arg->log_file = pg_strdup(log_file);
- if (new_arg->opt_log_file)
- pg_free(new_arg->opt_log_file);
+ pg_free(new_arg->opt_log_file);
new_arg->opt_log_file = opt_log_file ? pg_strdup(opt_log_file) : NULL;
- if (new_arg->cmd)
- pg_free(new_arg->cmd);
+ pg_free(new_arg->cmd);
new_arg->cmd = pg_strdup(cmd);
child = (HANDLE) _beginthreadex(NULL, 0, (void *) win32_exec_prog,
@@ -243,14 +240,11 @@ parallel_transfer_all_new_dbs(DbInfoArr *old_db_arr, DbInfoArr *new_db_arr,
/* Can only pass one pointer into the function, so use a struct */
new_arg->old_db_arr = old_db_arr;
new_arg->new_db_arr = new_db_arr;
- if (new_arg->old_pgdata)
- pg_free(new_arg->old_pgdata);
+ pg_free(new_arg->old_pgdata);
new_arg->old_pgdata = pg_strdup(old_pgdata);
- if (new_arg->new_pgdata)
- pg_free(new_arg->new_pgdata);
+ pg_free(new_arg->new_pgdata);
new_arg->new_pgdata = pg_strdup(new_pgdata);
- if (new_arg->old_tablespace)
- pg_free(new_arg->old_tablespace);
+ pg_free(new_arg->old_tablespace);
new_arg->old_tablespace = old_tablespace ? pg_strdup(old_tablespace) : NULL;
child = (HANDLE) _beginthreadex(NULL, 0, (void *) win32_transfer_all_new_dbs,
diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index 36b5c8fff7..bcaea8f5ea 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -5473,12 +5473,10 @@ static void
free_command(Command *command)
{
termPQExpBuffer(&command->lines);
- if (command->first_line)
- pg_free(command->first_line);
+ pg_free(command->first_line);
for (int i = 0; i < command->argc; i++)
pg_free(command->argv[i]);
- if (command->varprefix)
- pg_free(command->varprefix);
+ pg_free(command->varprefix);
/*
* It should also free expr recursively, but this is currently not needed
@@ -6635,8 +6633,7 @@ main(int argc, char **argv)
is_init_mode = true;
break;
case 'I':
- if (initialize_steps)
- pg_free(initialize_steps);
+ pg_free(initialize_steps);
initialize_steps = pg_strdup(optarg);
checkInitSteps(initialize_steps);
initialization_option_set = true;
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 4495f7a907..f3c5196c90 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -3475,8 +3475,7 @@ do_connect(enum trivalue reuse_previous_specification,
} /* end retry loop */
/* Release locally allocated data, whether we succeeded or not */
- if (password)
- pg_free(password);
+ pg_free(password);
if (cinfo)
PQconninfoFree(cinfo);
diff --git a/src/bin/psql/variables.c b/src/bin/psql/variables.c
index 47c58d2be9..5cc4bc0cb7 100644
--- a/src/bin/psql/variables.c
+++ b/src/bin/psql/variables.c
@@ -255,8 +255,7 @@ SetVariable(VariableSpace space, const char *name, const char *value)
if (confirmed)
{
- if (current->value)
- pg_free(current->value);
+ pg_free(current->value);
current->value = new_value;
/*
@@ -272,7 +271,7 @@ SetVariable(VariableSpace space, const char *name, const char *value)
free(current);
}
}
- else if (new_value)
+ else
pg_free(new_value); /* current->value is left unchanged */
return confirmed;
--
2.36.1
v2-0003-Remove-redundant-null-pointer-checks-before-PQcle.patchtext/plain; charset=UTF-8; name=v2-0003-Remove-redundant-null-pointer-checks-before-PQcle.patchDownload
From 355ef1a68be690d9bb8ee0524226abd648733ce0 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
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 @@ <title>Main Functions</title>
<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 @@ <title>Miscellaneous Functions</title>
<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
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
Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:
On 17.06.22 07:11, Tom Lane wrote:
Notably, I think the choice
that pfree(NULL) is disallowed traces directly to worries about
coding-pattern-compatibility with pre-POSIX free(). Should we
revisit that?
Yes please, and also repalloc().
repalloc no, because you wouldn't know which context to do the
allocation in.
regards, tom lane
On Fri, Jun 17, 2022 at 09:03:23PM +0200, Peter Eisentraut wrote:
I'm pretty sure PostgreSQL code already depends on this behavior anyway.
The code just isn't consistent about it.
In the frontend, I'd like to think that you are right and that we have
already some places doing that. The backend is a different story,
like in GetMemoryChunkContext(). That should be easy enough to check
with some LD_PRELOAD wizardry, at least.
--
Michael
Michael Paquier <michael@paquier.xyz> writes:
On Fri, Jun 17, 2022 at 09:03:23PM +0200, Peter Eisentraut wrote:
I'm pretty sure PostgreSQL code already depends on this behavior anyway.
The code just isn't consistent about it.
In the frontend, I'd like to think that you are right and that we have
already some places doing that.
We do, indeed.
The backend is a different story,
like in GetMemoryChunkContext(). That should be easy enough to check
with some LD_PRELOAD wizardry, at least.
Huh? The proposal is to accept the fact that free() tolerates NULL,
and then maybe make pfree() tolerate it as well. I don't think that
that needs to encompass any other functions.
regards, tom lane
On 2022-Jun-17, Peter Eisentraut wrote:
From 355ef1a68be690d9bb8ee0524226abd648733ce0 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Fri, 17 Jun 2022 12:09:32 +0200
Subject: [PATCH v2 3/3] Remove redundant null pointer checks before PQclear
and PQconninfofreeThese 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.
For PQclear() specifically, one thing that I thought a few days ago
would be useful would to have it return (PGresult *) NULL. Then the
very common pattern of doing "PQclear(res); res = NULL;" could be
simplified to "res = PQclear(res);", which is nicely compact and is
learned instantly.
I've not seen this convention used anywhere else though, and I'm not
sure I'd advocate it for other functions where we use similar patterns
such as pfree/pg_free, so perhaps it'd become too much of a special
case.
--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
For PQclear() specifically, one thing that I thought a few days ago
would be useful would to have it return (PGresult *) NULL. Then the
very common pattern of doing "PQclear(res); res = NULL;" could be
simplified to "res = PQclear(res);", which is nicely compact and is
learned instantly.
That's a public API unfortunately, and so some people would demand
a libpq.so major version bump if we changed it.
regards, tom lane