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.From f7a10c1fca10c76b89dbf402ad9418ed8f0675d7 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@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
