commit f2f729e26b93fe42aeac0c0f99bf9ea8dc62591b Author: Nick Mathewson <nickm@torproject.org> Date: Tue Sep 12 17:46:09 2017 -0400 Clear up dead-assignment warnings from scan-build --- src/common/compat.c | 22 ++++++++++------------ src/common/timers.c | 2 +- src/common/util.c | 25 +++++++++++++------------ src/or/circuitmux.c | 10 ++-------- src/or/connection.c | 10 +++++----- src/or/dirvote.c | 4 ++-- src/or/rendservice.c | 18 +++++++++--------- src/or/statefile.c | 2 -- src/test/test_addr.c | 2 ++ src/test/test_config.c | 1 + src/test/test_crypto.c | 1 + src/test/test_dir.c | 4 ++++ src/test/test_hs_common.c | 3 ++- src/test/test_hs_service.c | 2 ++ src/test/test_options.c | 6 ++++++ src/test/test_rendcache.c | 1 + src/test/test_tortls.c | 1 + src/test/test_util.c | 3 +++ 18 files changed, 65 insertions(+), 52 deletions(-) diff --git a/src/common/compat.c b/src/common/compat.c index 836b3813e..68938ae91 100644 --- a/src/common/compat.c +++ b/src/common/compat.c @@ -2235,35 +2235,33 @@ switch_id(const char *user, const unsigned flags) int tor_disable_debugger_attach(void) { - int r, attempted; - r = -1; - attempted = 0; + int r = -1; log_debug(LD_CONFIG, "Attemping to disable debugger attachment to Tor for " "unprivileged users."); #if defined(__linux__) && defined(HAVE_SYS_PRCTL_H) && defined(HAVE_PRCTL) #ifdef PR_SET_DUMPABLE - attempted = 1; +#define TRIED_TO_DISABLE r = prctl(PR_SET_DUMPABLE, 0); #endif -#endif -#if defined(__APPLE__) && defined(PT_DENY_ATTACH) - if (r < 0) { - attempted = 1; - r = ptrace(PT_DENY_ATTACH, 0, 0, 0); - } +#elif defined(__APPLE__) && defined(PT_DENY_ATTACH) +#define TRIED_TO_DISABLE + r = ptrace(PT_DENY_ATTACH, 0, 0, 0); #endif // XXX: TODO - Mac OS X has dtrace and this may be disabled. // XXX: TODO - Windows probably has something similar - if (r == 0 && attempted) { +#ifdef TRIED_TO_DISABLE + if (r == 0) { log_debug(LD_CONFIG,"Debugger attachment disabled for " "unprivileged users."); return 1; - } else if (attempted) { + } else { log_warn(LD_CONFIG, "Unable to disable debugger attaching: %s", strerror(errno)); } +#endif +#undef TRIED_TO_DISABLE return r; } diff --git a/src/common/timers.c b/src/common/timers.c index c43c49c08..42d368baa 100644 --- a/src/common/timers.c +++ b/src/common/timers.c @@ -191,7 +191,7 @@ timers_initialize(void) if (BUG(global_timeouts)) return; // LCOV_EXCL_LINE - timeout_error_t err; + timeout_error_t err = 0; global_timeouts = timeouts_open(0, &err); if (!global_timeouts) { // LCOV_EXCL_START -- this can only fail on malloc failure. diff --git a/src/common/util.c b/src/common/util.c index 6e08979ff..c8358ea70 100644 --- a/src/common/util.c +++ b/src/common/util.c @@ -3610,7 +3610,7 @@ start_daemon(void) } else { /* Child */ close(daemon_filedes[0]); /* we only write */ - pid = setsid(); /* Detach from controlling terminal */ + (void) setsid(); /* Detach from controlling terminal */ /* * Fork one more time, so the parent (the session group leader) can exit. * This means that we, as a non-session group leader, can never regain a @@ -4308,7 +4308,6 @@ tor_spawn_background(const char *const filename, const char **argv, int stderr_pipe[2]; int stdin_pipe[2]; int fd, retval; - ssize_t nbytes; process_handle_t *process_handle; int status; @@ -4329,7 +4328,7 @@ tor_spawn_background(const char *const filename, const char **argv, and we are not allowed to use unsafe functions between fork and exec */ error_message_length = strlen(error_message); - child_state = CHILD_STATE_PIPE; + // child_state = CHILD_STATE_PIPE; /* Set up pipe for redirecting stdout, stderr, and stdin of child */ retval = pipe(stdout_pipe); @@ -4366,7 +4365,7 @@ tor_spawn_background(const char *const filename, const char **argv, return status; } - child_state = CHILD_STATE_MAXFD; + // child_state = CHILD_STATE_MAXFD; #ifdef _SC_OPEN_MAX if (-1 == max_fd) { @@ -4381,7 +4380,7 @@ tor_spawn_background(const char *const filename, const char **argv, max_fd = DEFAULT_MAX_FD; #endif - child_state = CHILD_STATE_FORK; + // child_state = CHILD_STATE_FORK; pid = fork(); if (0 == pid) { @@ -4417,7 +4416,7 @@ tor_spawn_background(const char *const filename, const char **argv, if (-1 == retval) goto error; - child_state = CHILD_STATE_CLOSEFD; + // child_state = CHILD_STATE_CLOSEFD; close(stderr_pipe[0]); close(stderr_pipe[1]); @@ -4433,7 +4432,7 @@ tor_spawn_background(const char *const filename, const char **argv, close(fd); } - child_state = CHILD_STATE_EXEC; + // child_state = CHILD_STATE_EXEC; /* Call the requested program. We need the cast because execvp doesn't define argv as const, even though it @@ -4452,7 +4451,8 @@ tor_spawn_background(const char *const filename, const char **argv, error: { /* XXX: are we leaking fds from the pipe? */ - int n; + int n, err=0; + ssize_t nbytes; n = format_helper_exit_status(child_state, errno, hex_errno); @@ -4461,13 +4461,14 @@ tor_spawn_background(const char *const filename, const char **argv, value, but there is nothing we can do if it fails */ /* TODO: Don't use STDOUT, use a pipe set up just for this purpose */ nbytes = write(STDOUT_FILENO, error_message, error_message_length); + err = (nbytes < 0); nbytes = write(STDOUT_FILENO, hex_errno, n); + err += (nbytes < 0); } - } - (void) nbytes; + _exit(err?254:255); + } - _exit(255); /* Never reached, but avoids compiler warning */ return status; // LCOV_EXCL_LINE } @@ -4534,7 +4535,7 @@ tor_spawn_background(const char *const filename, const char **argv, } *process_handle_out = process_handle; - return process_handle->status; + return status; #endif // _WIN32 } diff --git a/src/or/circuitmux.c b/src/or/circuitmux.c index ee0d5c718..b52606d12 100644 --- a/src/or/circuitmux.c +++ b/src/or/circuitmux.c @@ -261,13 +261,11 @@ circuitmux_move_active_circ_to_tail(circuitmux_t *cmux, circuit_t *circ, if (circ->n_mux == cmux) { next_p = &(circ->next_active_on_n_chan); prev_p = &(circ->prev_active_on_n_chan); - direction = CELL_DIRECTION_OUT; } else { or_circ = TO_OR_CIRCUIT(circ); tor_assert(or_circ->p_mux == cmux); next_p = &(or_circ->next_active_on_p_chan); prev_p = &(or_circ->prev_active_on_p_chan); - direction = CELL_DIRECTION_IN; } } tor_assert(next_p); @@ -1655,7 +1653,6 @@ circuitmux_assert_okay_pass_one(circuitmux_t *cmux) circid_t circ_id; circuit_t *circ; or_circuit_t *or_circ; - unsigned int circ_is_active; circuit_t **next_p, **prev_p; unsigned int n_circuits, n_active_circuits, n_cells; @@ -1679,8 +1676,6 @@ circuitmux_assert_okay_pass_one(circuitmux_t *cmux) tor_assert(chan); circ = circuit_get_by_circid_channel_even_if_marked(circ_id, chan); tor_assert(circ); - /* Clear the circ_is_active bit to start */ - circ_is_active = 0; /* Assert that we know which direction this is going */ tor_assert((*i)->muxinfo.direction == CELL_DIRECTION_OUT || @@ -1707,7 +1702,7 @@ circuitmux_assert_okay_pass_one(circuitmux_t *cmux) * Should this circuit be active? I.e., does the mux know about > 0 * cells on it? */ - circ_is_active = ((*i)->muxinfo.cell_count > 0); + const int circ_is_active = ((*i)->muxinfo.cell_count > 0); /* It should be in the linked list iff it's active */ if (circ_is_active) { @@ -1759,7 +1754,6 @@ circuitmux_assert_okay_pass_two(circuitmux_t *cmux) circuit_t **next_p, **prev_p; channel_t *chan; unsigned int n_active_circuits = 0; - cell_direction_t direction; chanid_circid_muxinfo_t search, *hashent = NULL; tor_assert(cmux); @@ -1778,7 +1772,7 @@ circuitmux_assert_okay_pass_two(circuitmux_t *cmux) curr_or_circ = NULL; next_circ = NULL; next_p = prev_p = NULL; - direction = 0; + cell_direction_t direction; /* Figure out if this is n_mux or p_mux */ if (cmux == curr_circ->n_mux) { diff --git a/src/or/connection.c b/src/or/connection.c index 5c675acca..ac26d43ff 100644 --- a/src/or/connection.c +++ b/src/or/connection.c @@ -3062,9 +3062,11 @@ connection_buckets_decrement(connection_t *conn, time_t now, (unsigned long)num_read, (unsigned long)num_written, conn_type_to_string(conn->type), conn_state_to_string(conn->type, conn->state)); - if (num_written >= INT_MAX) num_written = 1; - if (num_read >= INT_MAX) num_read = 1; - tor_fragile_assert(); + tor_assert_nonfatal_unreached(); + if (num_written >= INT_MAX) + num_written = 1; + if (num_read >= INT_MAX) + num_read = 1; } record_num_bytes_transferred_impl(conn, now, num_read, num_written); @@ -3593,10 +3595,8 @@ connection_buf_read_from_socket(connection_t *conn, ssize_t *max_to_read, connection_start_reading(conn); } /* we're already reading, one hopes */ - result = 0; break; case TOR_TLS_DONE: /* no data read, so nothing to process */ - result = 0; break; /* so we call bucket_decrement below */ default: break; diff --git a/src/or/dirvote.c b/src/or/dirvote.c index c65945fea..5dd217952 100644 --- a/src/or/dirvote.c +++ b/src/or/dirvote.c @@ -541,8 +541,8 @@ compute_routerstatus_consensus(smartlist_t *votes, int consensus_method, if (cur_n > most_n || (cur && cur_n == most_n && cur->status.published_on > most_published)) { most = cur; - most_n = cur_n; - most_published = cur->status.published_on; + // most_n = cur_n; // unused after this point. + // most_published = cur->status.published_on; // unused after this point. } tor_assert(most); diff --git a/src/or/rendservice.c b/src/or/rendservice.c index 01147c007..02240a7e0 100644 --- a/src/or/rendservice.c +++ b/src/or/rendservice.c @@ -696,7 +696,6 @@ rend_config_service(const config_line_t *line_, * of authorized clients. */ smartlist_t *type_names_split, *clients; const char *authname; - int num_clients; if (service->auth_type != REND_NO_AUTH) { log_warn(LD_CONFIG, "Got multiple HiddenServiceAuthorizeClient " "lines for a single service."); @@ -740,14 +739,15 @@ rend_config_service(const config_line_t *line_, SMARTLIST_FOREACH(type_names_split, char *, cp, tor_free(cp)); smartlist_free(type_names_split); /* Remove duplicate client names. */ - num_clients = smartlist_len(clients); - smartlist_sort_strings(clients); - smartlist_uniq_strings(clients); - if (smartlist_len(clients) < num_clients) { - log_info(LD_CONFIG, "HiddenServiceAuthorizeClient contains %d " - "duplicate client name(s); removing.", - num_clients - smartlist_len(clients)); - num_clients = smartlist_len(clients); + { + int num_clients = smartlist_len(clients); + smartlist_sort_strings(clients); + smartlist_uniq_strings(clients); + if (smartlist_len(clients) < num_clients) { + log_info(LD_CONFIG, "HiddenServiceAuthorizeClient contains %d " + "duplicate client name(s); removing.", + num_clients - smartlist_len(clients)); + } } SMARTLIST_FOREACH_BEGIN(clients, const char *, client_name) { diff --git a/src/or/statefile.c b/src/or/statefile.c index 18111771d..9647aa883 100644 --- a/src/or/statefile.c +++ b/src/or/statefile.c @@ -659,8 +659,6 @@ save_transport_to_state(const char *transport, *next = line = tor_malloc_zero(sizeof(config_line_t)); line->key = tor_strdup("TransportProxy"); tor_asprintf(&line->value, "%s %s", transport, fmt_addrport(addr, port)); - - next = &(line->next); } if (!get_options()->AvoidDiskWrites) diff --git a/src/test/test_addr.c b/src/test/test_addr.c index 53d1c754b..1a3754f08 100644 --- a/src/test/test_addr.c +++ b/src/test/test_addr.c @@ -760,7 +760,9 @@ test_addr_ip6_helpers(void *arg) /* get interface addresses */ r = get_interface_address6(LOG_DEBUG, AF_INET, &t1); + tt_int_op(r, OP_LE, 0); // "it worked or it didn't" i = get_interface_address6(LOG_DEBUG, AF_INET6, &t2); + tt_int_op(i, OP_LE, 0); // "it worked or it didn't" TT_BLATHER(("v4 address: %s (family=%d)", fmt_addr(&t1), tor_addr_family(&t1))); diff --git a/src/test/test_config.c b/src/test/test_config.c index 33ce5a4b3..379e2a0f5 100644 --- a/src/test/test_config.c +++ b/src/test/test_config.c @@ -704,6 +704,7 @@ test_config_parse_transport_plugin_line(void *arg) tt_int_op(r, OP_LT, 0); r = parse_transport_line(options, "transport_1,transport_2 proxy 1.2.3.4:567", 1, 1); + tt_int_op(r, OP_LT, 0); /* No port error exit */ r = parse_transport_line(options, "transport_1 socks5 1.2.3.4", 1, 0); diff --git a/src/test/test_crypto.c b/src/test/test_crypto.c index d8c37bd32..c540aaed6 100644 --- a/src/test/test_crypto.c +++ b/src/test/test_crypto.c @@ -1406,6 +1406,7 @@ test_crypto_digests(void *arg) AUTHORITY_SIGNKEY_A_DIGEST, HEX_DIGEST_LEN); r = crypto_pk_get_common_digests(k, &pkey_digests); + tt_int_op(r, OP_EQ, 0); tt_mem_op(hex_str(pkey_digests.d[DIGEST_SHA1], DIGEST_LEN),OP_EQ, AUTHORITY_SIGNKEY_A_DIGEST, HEX_DIGEST_LEN); diff --git a/src/test/test_dir.c b/src/test/test_dir.c index dd910cd0c..896e16ce0 100644 --- a/src/test/test_dir.c +++ b/src/test/test_dir.c @@ -2252,6 +2252,7 @@ test_dir_networkstatus_compute_bw_weights_v10(void *arg) tt_i64_op(G+M+E+D, OP_EQ, T); ret = networkstatus_compute_bw_weights_v10(chunks, G, M, E, D, T, weight_scale); + tt_assert(ret); tt_str_op(smartlist_get(chunks, 0), OP_EQ, "bandwidth-weights Wbd=883 Wbe=0 " "Wbg=3673 Wbm=10000 Wdb=10000 Web=10000 Wed=8233 Wee=10000 Weg=8233 " "Wem=10000 Wgb=10000 Wgd=883 Wgg=6327 Wgm=6327 Wmb=10000 Wmd=883 Wme=0 " @@ -2268,6 +2269,7 @@ test_dir_networkstatus_compute_bw_weights_v10(void *arg) tt_i64_op(G+M+E+D, OP_EQ, T); ret = networkstatus_compute_bw_weights_v10(chunks, G, M, E, D, T, weight_scale); + tt_assert(ret); tt_str_op(smartlist_get(chunks, 0), OP_EQ, "bandwidth-weights Wbd=0 Wbe=0 " "Wbg=4194 Wbm=10000 Wdb=10000 Web=10000 Wed=10000 Wee=10000 Weg=10000 " "Wem=10000 Wgb=10000 Wgd=0 Wgg=5806 Wgm=5806 Wmb=10000 Wmd=0 Wme=0 " @@ -2284,6 +2286,7 @@ test_dir_networkstatus_compute_bw_weights_v10(void *arg) tt_i64_op(G+M+E+D, OP_EQ, T); ret = networkstatus_compute_bw_weights_v10(chunks, G, M, E, D, T, weight_scale); + tt_assert(ret); tt_str_op(smartlist_get(chunks, 0), OP_EQ, "bandwidth-weights Wbd=317 " "Wbe=5938 Wbg=0 Wbm=10000 Wdb=10000 Web=10000 Wed=9366 Wee=4061 " "Weg=9366 Wem=4061 Wgb=10000 Wgd=317 Wgg=10000 Wgm=10000 Wmb=10000 " @@ -2304,6 +2307,7 @@ test_dir_networkstatus_compute_bw_weights_v10(void *arg) "Wbe=0 Wbg=0 Wbm=10000 Wdb=10000 Web=10000 Wed=3333 Wee=10000 Weg=3333 " "Wem=10000 Wgb=10000 Wgd=3333 Wgg=10000 Wgm=10000 Wmb=10000 Wmd=3333 " "Wme=0 Wmg=0 Wmm=10000\n"); + tt_assert(ret); done: SMARTLIST_FOREACH(chunks, char *, cp, tor_free(cp)); diff --git a/src/test/test_hs_common.c b/src/test/test_hs_common.c index da592eb08..3cacbab0f 100644 --- a/src/test/test_hs_common.c +++ b/src/test/test_hs_common.c @@ -708,6 +708,7 @@ test_hid_serv_request_tracker(void *arg) /* Add another request with very short key */ retval = hs_lookup_last_hid_serv_request(hsdir, "l", now, 1); + tt_int_op(retval, OP_EQ, now); tt_int_op(strmap_size(request_tracker),OP_EQ, 3); /* Try deleting entries with a dummy key. Check that our previous requests @@ -1511,7 +1512,7 @@ helper_test_hsdir_sync(networkstatus_t *ns, tt_int_op(smartlist_len(desc->previous_hsdirs), OP_EQ, 6); /* 3) Initialize client time */ - now = helper_set_consensus_and_system_time(ns, client_between_srv_and_tp); + helper_set_consensus_and_system_time(ns, client_between_srv_and_tp); cleanup_nodelist(); SMARTLIST_FOREACH(ns->routerstatus_list, diff --git a/src/test/test_hs_service.c b/src/test/test_hs_service.c index 8a6a6c328..b57bcac29 100644 --- a/src/test/test_hs_service.c +++ b/src/test/test_hs_service.c @@ -1269,6 +1269,7 @@ test_upload_descriptors(void *arg) ret = parse_rfc1123_time("Sat, 26 Oct 1985 13:00:00 UTC", &mock_ns.valid_after); + tt_int_op(ret, OP_EQ, 0); ret = parse_rfc1123_time("Sat, 26 Oct 1985 14:00:00 UTC", &mock_ns.fresh_until); tt_int_op(ret, OP_EQ, 0); @@ -1344,6 +1345,7 @@ test_revision_counter_state(void *arg) &desc_two->blinded_kp.pubkey, &service_found); tt_int_op(service_found, OP_EQ, 0); + tt_u64_op(cached_rev_counter, OP_EQ, 0); /* Now let's try with the right pubkeys */ cached_rev_counter =check_state_line_for_service_rev_counter(state_line_one, diff --git a/src/test/test_options.c b/src/test/test_options.c index 65b678cf5..6e1e25249 100644 --- a/src/test/test_options.c +++ b/src/test/test_options.c @@ -659,6 +659,7 @@ test_options_validate__logs(void *ignored) tt_str_op(tdata->opt->Logs->key, OP_EQ, "Log"); tt_str_op(tdata->opt->Logs->value, OP_EQ, "notice stdout"); tor_free(msg); + tt_int_op(ret, OP_EQ, -1); free_options_test_data(tdata); tdata = get_options_test_data(""); @@ -669,6 +670,7 @@ test_options_validate__logs(void *ignored) tt_str_op(tdata->opt->Logs->key, OP_EQ, "Log"); tt_str_op(tdata->opt->Logs->value, OP_EQ, "warn stdout"); tor_free(msg); + tt_int_op(ret, OP_EQ, -1); free_options_test_data(tdata); tdata = get_options_test_data(""); @@ -678,6 +680,7 @@ test_options_validate__logs(void *ignored) ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); tt_assert(!tdata->opt->Logs); tor_free(msg); + tt_int_op(ret, OP_EQ, -1); free_options_test_data(tdata); tdata = get_options_test_data(""); @@ -686,6 +689,7 @@ test_options_validate__logs(void *ignored) ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 1, &msg); tt_assert(!tdata->opt->Logs); tor_free(msg); + tt_int_op(ret, OP_EQ, -1); free_options_test_data(tdata); tdata = get_options_test_data(""); @@ -694,6 +698,7 @@ test_options_validate__logs(void *ignored) ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); tt_assert(!tdata->opt->Logs); tor_free(msg); + tt_int_op(ret, OP_EQ, -1); free_options_test_data(tdata); tdata = get_options_test_data(""); @@ -703,6 +708,7 @@ test_options_validate__logs(void *ignored) tdata->opt->Logs = cl; ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); tt_int_op((intptr_t)tdata->opt->Logs, OP_EQ, (intptr_t)cl); + tt_int_op(ret, OP_EQ, -1); done: quiet_level = orig_quiet_level; diff --git a/src/test/test_rendcache.c b/src/test/test_rendcache.c index 0db52292c..9354dd048 100644 --- a/src/test/test_rendcache.c +++ b/src/test/test_rendcache.c @@ -58,6 +58,7 @@ test_rend_cache_lookup_entry(void *data) tt_int_op(ret, OP_EQ, 0); ret = rend_cache_lookup_entry(service_id, 2, &entry); + tt_int_op(ret, OP_EQ, 0); tt_assert(entry); tt_int_op(entry->len, OP_EQ, strlen(desc_holder->desc_str)); tt_str_op(entry->desc, OP_EQ, desc_holder->desc_str); diff --git a/src/test/test_tortls.c b/src/test/test_tortls.c index ce21bb28e..0e89a66b5 100644 --- a/src/test/test_tortls.c +++ b/src/test/test_tortls.c @@ -1378,6 +1378,7 @@ test_tortls_evaluate_ecgroup_for_tls(void *ignored) ret = evaluate_ecgroup_for_tls("P224"); // tt_int_op(ret, OP_EQ, 1); This varies between machines + tt_assert(ret == 0 || ret == 1); done: (void)0; diff --git a/src/test/test_util.c b/src/test/test_util.c index 6e54c8bf5..6bed23fc7 100644 --- a/src/test/test_util.c +++ b/src/test/test_util.c @@ -2179,10 +2179,13 @@ test_util_parse_integer(void *arg) tt_int_op(1,OP_EQ, i); tt_assert(DBL_TO_U64(d) == 0); d = tor_parse_double(" ", 0, (double)UINT64_MAX,&i,NULL); + tt_double_op(fabs(d), OP_LT, 1e-10); tt_int_op(0,OP_EQ, i); d = tor_parse_double(".0a", 0, (double)UINT64_MAX,&i,NULL); + tt_double_op(fabs(d), OP_LT, 1e-10); tt_int_op(0,OP_EQ, i); d = tor_parse_double(".0a", 0, (double)UINT64_MAX,&i,&cp); + tt_double_op(fabs(d), OP_LT, 1e-10); tt_int_op(1,OP_EQ, i); d = tor_parse_double("-.0", 0, (double)UINT64_MAX,&i,NULL); tt_int_op(1,OP_EQ, i);