From efcab439564dcadc5bc14609a9205d73d236e966 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 2 Sep 2014 11:55:53 -0400 Subject: [PATCH] Fix a number of clang analyzer false-positives Most of these are in somewhat non-obvious code where it is probably a good idea to initialize variables and add extra assertions anyway. Closes 13036. Patches from "teor". --- changes/ticket13036 | 5 +++++ src/common/address.c | 2 ++ src/or/circuitmux.c | 8 +++++++- src/or/connection.c | 1 + src/or/control.c | 5 +++-- src/or/dirvote.c | 4 ++++ src/or/rephist.c | 2 +- src/or/routerparse.c | 3 ++- 8 files changed, 25 insertions(+), 5 deletions(-) create mode 100644 changes/ticket13036 diff --git a/changes/ticket13036 b/changes/ticket13036 new file mode 100644 index 0000000000..1b4784358a --- /dev/null +++ b/changes/ticket13036 @@ -0,0 +1,5 @@ + o Minor bugfixes: + - Fix a large number of false positive warnings from the clang + analyzer static analysis tool. This should make real warnings + easier for clang analyzer to find. Patch from "teor". Closes + ticket 13036. diff --git a/src/common/address.c b/src/common/address.c index 29d4c0447e..8591f387e6 100644 --- a/src/common/address.c +++ b/src/common/address.c @@ -324,6 +324,8 @@ tor_addr_is_internal_(const tor_addr_t *addr, int for_listening, uint32_t iph4 = 0; uint32_t iph6[4]; sa_family_t v_family; + + tor_assert(addr); v_family = tor_addr_family(addr); if (v_family == AF_INET) { diff --git a/src/or/circuitmux.c b/src/or/circuitmux.c index 55580d5c29..e4571ff944 100644 --- a/src/or/circuitmux.c +++ b/src/or/circuitmux.c @@ -1092,8 +1092,11 @@ circuitmux_detach_circuit,(circuitmux_t *cmux, circuit_t *circ)) /* * Use this to keep track of whether we found it for n_chan or * p_chan for consistency checking. + * + * The 0 initializer is not a valid cell_direction_t value. + * We assert that it has been replaced with a valid value before it is used. */ - cell_direction_t last_searched_direction; + cell_direction_t last_searched_direction = 0; tor_assert(cmux); tor_assert(cmux->chanid_circid_map); @@ -1123,6 +1126,9 @@ circuitmux_detach_circuit,(circuitmux_t *cmux, circuit_t *circ)) } } + tor_assert(last_searched_direction == CELL_DIRECTION_OUT + || last_searched_direction == CELL_DIRECTION_IN); + /* * If hashent isn't NULL, we have a circuit to detach; don't remove it from * the map until later of circuitmux_make_circuit_inactive() breaks. diff --git a/src/or/connection.c b/src/or/connection.c index 4788bdf950..276dca2818 100644 --- a/src/or/connection.c +++ b/src/or/connection.c @@ -1613,6 +1613,7 @@ connection_connect(connection_t *conn, const char *address, } } + tor_assert(options); if (options->ConstrainedSockets) set_constrained_socket_buffers(s, (int)options->ConstrainedSockSize); diff --git a/src/or/control.c b/src/or/control.c index ec63506194..4a6b18d02a 100644 --- a/src/or/control.c +++ b/src/or/control.c @@ -2639,7 +2639,7 @@ handle_control_attachstream(control_connection_t *conn, uint32_t len, /* Is this a single hop circuit? */ if (circ && (circuit_get_cpath_len(circ)<2 || hop==1)) { const node_t *node = NULL; - char *exit_digest; + char *exit_digest = NULL; if (circ->build_state && circ->build_state->chosen_exit && !tor_digest_is_zero(circ->build_state->chosen_exit->identity_digest)) { @@ -2654,6 +2654,7 @@ handle_control_attachstream(control_connection_t *conn, uint32_t len, "551 Can't attach stream to this one-hop circuit.\r\n", conn); return 0; } + tor_assert(exit_digest); ap_conn->chosen_exit_name = tor_strdup(hex_str(exit_digest, DIGEST_LEN)); } @@ -4921,7 +4922,7 @@ MOCK_IMPL(void, or_connection_t *or_conn)) { int status = bootstrap_percent; - const char *tag, *summary; + const char *tag = "", *summary = ""; char buf[BOOTSTRAP_MSG_LEN]; const char *recommendation = "ignore"; int severity; diff --git a/src/or/dirvote.c b/src/or/dirvote.c index c7be343ca2..137d6c1a8c 100644 --- a/src/or/dirvote.c +++ b/src/or/dirvote.c @@ -335,6 +335,9 @@ static int compare_vote_rs(const vote_routerstatus_t *a, const vote_routerstatus_t *b) { int r; + tor_assert(a); + tor_assert(b); + if ((r = fast_memcmp(a->status.identity_digest, b->status.identity_digest, DIGEST_LEN))) return r; @@ -432,6 +435,7 @@ compute_routerstatus_consensus(smartlist_t *votes, int consensus_method, const tor_addr_port_t *most_alt_orport = NULL; SMARTLIST_FOREACH_BEGIN(votes, vote_routerstatus_t *, rs) { + tor_assert(rs); if (compare_vote_rs(most, rs) == 0 && !tor_addr_is_null(&rs->status.ipv6_addr) && rs->status.ipv6_orport) { diff --git a/src/or/rephist.c b/src/or/rephist.c index 5446c25e36..72de54c0c9 100644 --- a/src/or/rephist.c +++ b/src/or/rephist.c @@ -1570,7 +1570,7 @@ rep_hist_load_bwhist_state_section(bw_array_t *b, time_t start; uint64_t v, mv; - int i,ok,ok_m; + int i,ok,ok_m = 0; int have_maxima = s_maxima && s_values && (smartlist_len(s_values) == smartlist_len(s_maxima)); diff --git a/src/or/routerparse.c b/src/or/routerparse.c index 5add728d6d..f990cebd82 100644 --- a/src/or/routerparse.c +++ b/src/or/routerparse.c @@ -923,7 +923,7 @@ router_parse_list_from_string(const char **s, const char *eos, { routerinfo_t *router; extrainfo_t *extrainfo; - signed_descriptor_t *signed_desc; + signed_descriptor_t *signed_desc = NULL; void *elt; const char *end, *start; int have_extrainfo; @@ -980,6 +980,7 @@ router_parse_list_from_string(const char **s, const char *eos, continue; } if (saved_location != SAVED_NOWHERE) { + tor_assert(signed_desc); signed_desc->saved_location = saved_location; signed_desc->saved_offset = *s - start; }