[tor-dev] Remove NULL checks for *_free() calls
Michael McConville
mmcconv1 at sccs.swarthmore.edu
Mon Aug 31 00:13:34 UTC 2015
free() is specified to be NULL-safe, and I don't know of any
implementations that violate this. Tor's *_free() functions conform,
although relaycache_free() prints a warning (which I remove in the below
diff).
I checked every *_free() function for NULL-safety before removing
conditions for it. This includes an OpenSSL function or two.
This is only a first pass. I got bored, and I wanted to make sure that
there's interest in committing this. :)
diff --git a/src/common/backtrace.c b/src/common/backtrace.c
index a2d5378..0aa8a17 100644
--- a/src/common/backtrace.c
+++ b/src/common/backtrace.c
@@ -176,8 +176,7 @@ install_bt_handler(void)
char **symbols;
int depth = backtrace(cb_buf, MAX_DEPTH);
symbols = backtrace_symbols(cb_buf, depth);
- if (symbols)
- free(symbols);
+ free(symbols);
}
return rv;
diff --git a/src/common/torgzip.c b/src/common/torgzip.c
index 4f23407..3158b96 100644
--- a/src/common/torgzip.c
+++ b/src/common/torgzip.c
@@ -403,9 +403,7 @@ tor_gzip_uncompress(char **out, size_t *out_len,
inflateEnd(stream);
tor_free(stream);
}
- if (*out) {
- tor_free(*out);
- }
+ tor_free(*out);
return -1;
}
diff --git a/src/common/tortls.c b/src/common/tortls.c
index 7447822..23b1dcf 100644
--- a/src/common/tortls.c
+++ b/src/common/tortls.c
@@ -625,10 +625,8 @@ tor_tls_create_certificate(crypto_pk_t *rsa,
goto done;
error:
- if (x509) {
- X509_free(x509);
- x509 = NULL;
- }
+ X509_free(x509);
+ x509 = NULL;
done:
tls_log_errors(NULL, LOG_WARN, LD_NET, "generating certificate");
if (sign_pkey)
@@ -722,8 +720,7 @@ tor_x509_cert_free(tor_x509_cert_t *cert)
{
if (! cert)
return;
- if (cert->cert)
- X509_free(cert->cert);
+ X509_free(cert->cert);
tor_free(cert->encoded);
memwipe(cert, 0x03, sizeof(*cert));
tor_free(cert);
@@ -1328,12 +1325,9 @@ tor_tls_context_new(crypto_pk_t *identity, unsigned int key_lifetime,
crypto_pk_free(rsa_auth);
if (result)
tor_tls_context_decref(result);
- if (cert)
- X509_free(cert);
- if (idcert)
- X509_free(idcert);
- if (authcert)
- X509_free(authcert);
+ X509_free(cert);
+ X509_free(idcert);
+ X509_free(authcert);
return NULL;
}
@@ -2035,8 +2029,7 @@ tor_tls_finish_handshake(tor_tls_t *tls)
"a v2 handshake on %p.", tls);
tls->wasV2Handshake = 1;
}
- if (cert)
- X509_free(cert);
+ X509_free(cert);
#endif
if (SSL_set_cipher_list(tls->ssl, SERVER_CIPHER_LIST) == 0) {
tls_log_errors(NULL, LOG_WARN, LD_HANDSHAKE, "re-setting ciphers");
@@ -2319,8 +2312,7 @@ tor_tls_verify(int severity, tor_tls_t *tls, crypto_pk_t **identity_key)
r = 0;
done:
- if (cert)
- X509_free(cert);
+ X509_free(cert);
if (id_pkey)
EVP_PKEY_free(id_pkey);
@@ -2353,8 +2345,7 @@ tor_tls_check_lifetime(int severity, tor_tls_t *tls,
r = 0;
done:
- if (cert)
- X509_free(cert);
+ X509_free(cert);
/* Not expected to get invoked */
tls_log_errors(tls, LOG_WARN, LD_NET, "checking certificate lifetime");
diff --git a/src/common/util.h b/src/common/util.h
index 8bb4505..9ba9d9a 100644
--- a/src/common/util.h
+++ b/src/common/util.h
@@ -107,8 +107,8 @@ extern int dmalloc_free(const char *file, const int line, void *pnt,
STMT_END
#else
/** Release memory allocated by tor_malloc, tor_realloc, tor_strdup, etc.
- * Unlike the free() function, tor_free() will still work on NULL pointers,
- * and it sets the pointer value to NULL after freeing it.
+ * Unlike the free() function, tor_free() sets the pointer value to NULL
+ * after freeing it.
*
* This is a macro. If you need a function pointer to release memory from
* tor_malloc(), use tor_free_().
diff --git a/src/ext/eventdns.c b/src/ext/eventdns.c
index a0c7ff2..cfc3344 100644
--- a/src/ext/eventdns.c
+++ b/src/ext/eventdns.c
@@ -1923,9 +1923,7 @@ server_request_free(struct server_request *req)
rc = --req->port->refcnt;
}
- if (req->response) {
- mm_free(req->response);
- }
+ mm_free(req->response);
server_request_free_answers(req);
@@ -3216,8 +3214,7 @@ load_nameservers_with_getnetworkparams(void)
}
done:
- if (buf)
- mm_free(buf);
+ mm_free(buf);
if (handle)
FreeLibrary(handle);
return status;
diff --git a/src/ext/tinytest_demo.c b/src/ext/tinytest_demo.c
index c07f099..7279216 100644
--- a/src/ext/tinytest_demo.c
+++ b/src/ext/tinytest_demo.c
@@ -169,8 +169,7 @@ test_memcpy(void *ptr)
end:
/* This time our end block has something to do. */
- if (mem)
- free(mem);
+ free(mem);
}
void
diff --git a/src/or/buffers.c b/src/or/buffers.c
index 85fcbc6..e5d2894 100644
--- a/src/or/buffers.c
+++ b/src/or/buffers.c
@@ -1014,7 +1014,7 @@ fetch_var_cell_from_evbuffer(struct evbuffer *buf, var_cell_t **out,
result = 1;
done:
- if (free_hdr && hdr)
+ if (free_hdr)
tor_free(hdr);
return result;
}
@@ -2410,8 +2410,7 @@ generic_buffer_set_to_copy(generic_buffer_t **output,
}
}
#else
- if (*output)
- buf_free(*output);
+ buf_free(*output);
*output = buf_copy(input);
#endif
return 0;
diff --git a/src/or/channel.c b/src/or/channel.c
index af09502..b1dc63e 100644
--- a/src/or/channel.c
+++ b/src/or/channel.c
@@ -917,10 +917,8 @@ channel_force_free(channel_t *chan)
channel_clear_remote_end(chan);
/* Get rid of cmux */
- if (chan->cmux) {
- circuitmux_free(chan->cmux);
- chan->cmux = NULL;
- }
+ circuitmux_free(chan->cmux);
+ chan->cmux = NULL;
/* We might still have a cell queue; kill it */
TOR_SIMPLEQ_FOREACH_SAFE(cell, &chan->incoming_queue, next, cell_tmp) {
diff --git a/src/or/config.c b/src/or/config.c
index 6e782de..e3d7721 100644
--- a/src/or/config.c
+++ b/src/or/config.c
@@ -7263,11 +7263,9 @@ init_cookie_authentication(const char *fname, const char *header,
goto done;
}
- /* If we've already set the cookie, free it before re-setting
- it. This can happen if we previously generated a cookie, but
- couldn't write it to a disk. */
- if (*cookie_out)
- tor_free(*cookie_out);
+ /* Free this cookie before re-setting it. This can happen if we
+ * previously generated a cookie, but couldn't write it to a disk. */
+ tor_free(*cookie_out);
/* Generate the cookie */
*cookie_out = tor_malloc(cookie_len);
diff --git a/src/or/ext_orport.c b/src/or/ext_orport.c
index e8c8aa6..b4f4cab 100644
--- a/src/or/ext_orport.c
+++ b/src/or/ext_orport.c
@@ -476,9 +476,7 @@ connection_ext_or_handle_cmd_useraddr(connection_t *conn,
/* record the address */
tor_addr_copy(&conn->addr, &addr);
conn->port = port;
- if (conn->address) {
- tor_free(conn->address);
- }
+ tor_free(conn->address);
conn->address = tor_dup_addr(&addr);
return 0;
@@ -509,11 +507,8 @@ connection_ext_or_handle_cmd_transport(or_connection_t *conn,
return -1;
}
- /* If ext_or_transport is already occupied (because the PT sent two
- * TRANSPORT commands), deallocate the old name and keep the new
- * one */
- if (conn->ext_or_transport)
- tor_free(conn->ext_or_transport);
+ /* deallocate the ext_or_transport's old name and keep the new one */
+ tor_free(conn->ext_or_transport);
conn->ext_or_transport = transport_str;
return 0;
@@ -643,7 +638,6 @@ connection_ext_or_start_auth(or_connection_t *or_conn)
void
ext_orport_free_all(void)
{
- if (ext_or_auth_cookie) /* Free the auth cookie */
- tor_free(ext_or_auth_cookie);
+ tor_free(ext_or_auth_cookie);
}
diff --git a/src/or/onion_tap.c b/src/or/onion_tap.c
index 487cbee..4645ca4 100644
--- a/src/or/onion_tap.c
+++ b/src/or/onion_tap.c
@@ -75,7 +75,7 @@ onion_skin_TAP_create(crypto_pk_t *dest_router_key,
return 0;
err:
memwipe(challenge, 0, sizeof(challenge));
- if (dh) crypto_dh_free(dh);
+ crypto_dh_free(dh);
return -1;
}
diff --git a/src/or/rendservice.c b/src/or/rendservice.c
index db6bc4b..1fb5245 100644
--- a/src/or/rendservice.c
+++ b/src/or/rendservice.c
@@ -1746,7 +1746,7 @@ rend_service_receive_introduction(origin_circuit_t *circuit,
(unsigned)circuit->base_.n_circ_id);
err:
status = -1;
- if (dh) crypto_dh_free(dh);
+ crypto_dh_free(dh);
if (launched) {
circuit_mark_for_close(TO_CIRCUIT(launched), reason);
}
diff --git a/src/or/replaycache.c b/src/or/replaycache.c
index 569e073..f327fa7 100644
--- a/src/or/replaycache.c
+++ b/src/or/replaycache.c
@@ -18,10 +18,8 @@
void
replaycache_free(replaycache_t *r)
{
- if (!r) {
- log_info(LD_BUG, "replaycache_free() called on NULL");
+ if (!r)
return;
- }
if (r->digests_seen) digestmap_free(r->digests_seen, tor_free_);
diff --git a/src/or/routerparse.c b/src/or/routerparse.c
index c2206f1..71bcb7e 100644
--- a/src/or/routerparse.c
+++ b/src/or/routerparse.c
@@ -3840,8 +3840,7 @@ assert_addr_policy_ok(smartlist_t *lst)
static void
token_clear(directory_token_t *tok)
{
- if (tok->key)
- crypto_pk_free(tok->key);
+ crypto_pk_free(tok->key);
}
#define ALLOC_ZERO(sz) memarea_alloc_zero(area,sz)
diff --git a/src/test/test.c b/src/test/test.c
index e10e260..d978f78 100644
--- a/src/test/test.c
+++ b/src/test/test.c
@@ -572,14 +572,10 @@ test_rend_fns(void *arg)
rend_encoded_v2_service_descriptor_free(smartlist_get(descs, i));
smartlist_free(descs);
}
- if (parsed)
- rend_service_descriptor_free(parsed);
- if (generated)
- rend_service_descriptor_free(generated);
- if (pk1)
- crypto_pk_free(pk1);
- if (pk2)
- crypto_pk_free(pk2);
+ rend_service_descriptor_free(parsed);
+ rend_service_descriptor_free(generated);
+ crypto_pk_free(pk1);
+ crypto_pk_free(pk2);
tor_free(intro_points_encrypted);
}
diff --git a/src/test/test_buffers.c b/src/test/test_buffers.c
index e8fce12..8cc1b61 100644
--- a/src/test/test_buffers.c
+++ b/src/test/test_buffers.c
@@ -189,10 +189,8 @@ test_buffers_basic(void *arg)
}
done:
- if (buf)
- buf_free(buf);
- if (buf2)
- buf_free(buf2);
+ buf_free(buf);
+ buf_free(buf2);
}
static void
@@ -362,10 +360,8 @@ test_buffer_copy(void *arg)
}
done:
- if (buf)
- generic_buffer_free(buf);
- if (buf2)
- generic_buffer_free(buf2);
+ generic_buffer_free(buf);
+ generic_buffer_free(buf2);
}
static void
diff --git a/src/test/test_crypto.c b/src/test/test_crypto.c
index dbaec61..71a94f8 100644
--- a/src/test/test_crypto.c
+++ b/src/test/test_crypto.c
@@ -269,10 +269,8 @@ test_crypto_aes(void *arg)
done:
tor_free(mem_op_hex_tmp);
- if (env1)
- crypto_cipher_free(env1);
- if (env2)
- crypto_cipher_free(env2);
+ crypto_cipher_free(env1);
+ crypto_cipher_free(env2);
tor_free(data1);
tor_free(data2);
tor_free(data3);
@@ -412,10 +410,8 @@ test_crypto_sha(void *arg)
tt_mem_op(d_out1,OP_EQ, d_out2, DIGEST_LEN);
done:
- if (d1)
- crypto_digest_free(d1);
- if (d2)
- crypto_digest_free(d2);
+ crypto_digest_free(d1);
+ crypto_digest_free(d2);
tor_free(mem_op_hex_tmp);
}
diff --git a/src/test/test_dir.c b/src/test/test_dir.c
index 855746e..bf7ce59 100644
--- a/src/test/test_dir.c
+++ b/src/test/test_dir.c
@@ -346,20 +346,17 @@ test_dir_formats(void *arg)
dirserv_free_fingerprint_list();
done:
- if (r1)
- routerinfo_free(r1);
- if (r2)
- routerinfo_free(r2);
- if (rp2)
- routerinfo_free(rp2);
+ routerinfo_free(r1);
+ routerinfo_free(r2);
+ routerinfo_free(rp2);
tor_free(rsa_cc);
tor_free(buf);
tor_free(pk1_str);
tor_free(pk2_str);
- if (pk1) crypto_pk_free(pk1);
- if (pk2) crypto_pk_free(pk2);
- if (rp1) routerinfo_free(rp1);
+ crypto_pk_free(pk1);
+ crypto_pk_free(pk2);
+ routerinfo_free(rp1);
tor_free(dir1); /* XXXX And more !*/
tor_free(dir2); /* And more !*/
}
@@ -2315,32 +2312,19 @@ test_a_networkstatus(
tor_free(consensus_text);
tor_free(consensus_text_md);
- if (vote)
- networkstatus_vote_free(vote);
- if (v1)
- networkstatus_vote_free(v1);
- if (v2)
- networkstatus_vote_free(v2);
- if (v3)
- networkstatus_vote_free(v3);
- if (con)
- networkstatus_vote_free(con);
- if (con_md)
- networkstatus_vote_free(con_md);
- if (sign_skey_1)
- crypto_pk_free(sign_skey_1);
- if (sign_skey_2)
- crypto_pk_free(sign_skey_2);
- if (sign_skey_3)
- crypto_pk_free(sign_skey_3);
- if (sign_skey_leg1)
- crypto_pk_free(sign_skey_leg1);
- if (cert1)
- authority_cert_free(cert1);
- if (cert2)
- authority_cert_free(cert2);
- if (cert3)
- authority_cert_free(cert3);
+ networkstatus_vote_free(vote);
+ networkstatus_vote_free(v1);
+ networkstatus_vote_free(v2);
+ networkstatus_vote_free(v3);
+ networkstatus_vote_free(con);
+ networkstatus_vote_free(con_md);
+ crypto_pk_free(sign_skey_1);
+ crypto_pk_free(sign_skey_2);
+ crypto_pk_free(sign_skey_3);
+ crypto_pk_free(sign_skey_leg1);
+ authority_cert_free(cert1);
+ authority_cert_free(cert2);
+ authority_cert_free(cert3);
tor_free(consensus_text2);
tor_free(consensus_text3);
@@ -2348,18 +2332,12 @@ test_a_networkstatus(
tor_free(consensus_text_md3);
tor_free(detached_text1);
tor_free(detached_text2);
- if (con2)
- networkstatus_vote_free(con2);
- if (con3)
- networkstatus_vote_free(con3);
- if (con_md2)
- networkstatus_vote_free(con_md2);
- if (con_md3)
- networkstatus_vote_free(con_md3);
- if (dsig1)
- ns_detached_signatures_free(dsig1);
- if (dsig2)
- ns_detached_signatures_free(dsig2);
+ networkstatus_vote_free(con2);
+ networkstatus_vote_free(con3);
+ networkstatus_vote_free(con_md2);
+ networkstatus_vote_free(con_md3);
+ ns_detached_signatures_free(dsig1);
+ ns_detached_signatures_free(dsig2);
}
/** Run unit tests for generating and parsing V3 consensus networkstatus
diff --git a/src/test/test_replay.c b/src/test/test_replay.c
index a02c160..538b2df 100644
--- a/src/test/test_replay.c
+++ b/src/test/test_replay.c
@@ -27,7 +27,7 @@ test_replaycache_alloc(void *arg)
tt_assert(r != NULL);
done:
- if (r) replaycache_free(r);
+ replaycache_free(r);
return;
}
@@ -51,7 +51,7 @@ test_replaycache_badalloc(void *arg)
tt_assert(r == NULL);
done:
- if (r) replaycache_free(r);
+ replaycache_free(r);
return;
}
@@ -90,7 +90,7 @@ test_replaycache_miss(void *arg)
tt_int_op(result,OP_EQ, 0);
done:
- if (r) replaycache_free(r);
+ replaycache_free(r);
return;
}
@@ -116,7 +116,7 @@ test_replaycache_hit(void *arg)
tt_int_op(result,OP_EQ, 1);
done:
- if (r) replaycache_free(r);
+ replaycache_free(r);
return;
}
@@ -147,7 +147,7 @@ test_replaycache_age(void *arg)
tt_int_op(result,OP_EQ, 0);
done:
- if (r) replaycache_free(r);
+ replaycache_free(r);
return;
}
@@ -175,7 +175,7 @@ test_replaycache_elapsed(void *arg)
tt_int_op(elapsed,OP_EQ, 100);
done:
- if (r) replaycache_free(r);
+ replaycache_free(r);
return;
}
@@ -206,7 +206,7 @@ test_replaycache_noexpire(void *arg)
tt_int_op(result,OP_EQ, 1);
done:
- if (r) replaycache_free(r);
+ replaycache_free(r);
return;
}
@@ -248,7 +248,7 @@ test_replaycache_scrub(void *arg)
tt_int_op(digestmap_size(r->digests_seen),OP_EQ, 0);
done:
- if (r) replaycache_free(r);
+ replaycache_free(r);
return;
}
@@ -292,7 +292,7 @@ test_replaycache_future(void *arg)
tt_int_op(elapsed,OP_EQ, 0);
done:
- if (r) replaycache_free(r);
+ replaycache_free(r);
return;
}
@@ -335,7 +335,7 @@ test_replaycache_realtime(void *arg)
replaycache_scrub_if_needed(r);
done:
- if (r) replaycache_free(r);
+ replaycache_free(r);
return;
}
diff --git a/src/test/test_routerset.c b/src/test/test_routerset.c
index 9bd0c12..b13ec40 100644
--- a/src/test/test_routerset.c
+++ b/src/test/test_routerset.c
@@ -1144,8 +1144,7 @@ NS(test_main)(void *arg)
tt_int_op(r, OP_EQ, 0);
done:
- if (set != NULL)
- routerset_free(set);
+ routerset_free(set);
}
country_t
@@ -1186,8 +1185,7 @@ NS(test_main)(void *arg)
tt_int_op(smartlist_contains_string(set->list, "{??}"), OP_EQ, 1);
done:
- if (set != NULL)
- routerset_free(set);
+ routerset_free(set);
}
country_t
@@ -1249,8 +1247,7 @@ NS(test_main)(void *arg)
tt_int_op(smartlist_contains_string(set->list, "{a1}"), OP_EQ, 1);
done:
- if (set != NULL)
- routerset_free(set);
+ routerset_free(set);
}
country_t
diff --git a/src/test/test_status.c b/src/test/test_status.c
index cbc8af1..7e18b5d 100644
--- a/src/test/test_status.c
+++ b/src/test/test_status.c
@@ -145,8 +145,7 @@ NS(test_main)(void *arg)
tor_free(actual);
done:
- if (actual != NULL)
- tor_free(actual);
+ tor_free(actual);
}
#undef NS_SUBMODULE
@@ -226,8 +225,7 @@ NS(test_main)(void *arg)
tor_free(actual);
done:
- if (actual != NULL)
- tor_free(actual);
+ tor_free(actual);
}
#undef NS_SUBMODULE
diff --git a/src/test/test_util.c b/src/test/test_util.c
index 0a5783e..a4341d9 100644
--- a/src/test/test_util.c
+++ b/src/test/test_util.c
@@ -1707,8 +1707,7 @@ test_util_gzip(void *arg)
tt_int_op(21,OP_EQ, len2);
done:
- if (state)
- tor_zlib_free(state);
+ tor_zlib_free(state);
tor_free(buf2);
tor_free(buf3);
tor_free(buf1);
More information about the tor-dev
mailing list