From 02c408e21a6e78ff246ea7a1beb4669634fa9c4c Mon Sep 17 00:00:00 2001 From: Peter Eisentraut Date: Thu, 16 Jun 2022 21:50:56 +0200 Subject: Remove redundant null pointer checks before free() Per applicable standards, free() with a null pointer is a no-op. Systems that don't observe that are ancient and no longer relevant. Some PostgreSQL code already required this behavior, so this change does not introduce any new requirements, just makes the code more consistent. Discussion: https://www.postgresql.org/message-id/flat/dac5d2d0-98f5-94d9-8e69-46da2413593d%40enterprisedb.com --- 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(-) (limited to 'src/interfaces/libpq') diff --git a/src/interfaces/libpq/fe-auth-scram.c b/src/interfaces/libpq/fe-auth-scram.c index e6162007041..5012806fa5e 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 0a072a36dc2..49a1c626f64 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 6e936bbff30..057c9da0ede 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 @@ fail: 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 919cf5741d4..1750d647a8d 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 82fc592f068..783cd9b756f 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("\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 8046fcd884a..cc8a2b85938 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); } -- cgit v1.2.3