commit 89f5f149df984bab00de9868a9305b611c4aa17e Author: Nick Mathewson nickm@torproject.org Date: Mon Nov 28 11:04:28 2016 -0500
Remove guard_selection argument from status-reporting functions
This prevents us from mixing up multiple guard_selections --- src/or/circuitbuild.c | 3 +-- src/or/circuitlist.c | 2 +- src/or/circuituse.c | 2 +- src/or/connection.c | 2 +- src/or/connection_or.c | 6 ++---- src/or/directory.c | 9 ++++----- src/or/entrynodes.c | 31 ++++++++++--------------------- src/or/entrynodes.h | 12 ++++-------- src/test/test_entrynodes.c | 26 +++++++++++++------------- 9 files changed, 37 insertions(+), 56 deletions(-)
diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index 16b53f6..5d0a04f 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -976,8 +976,7 @@ circuit_send_next_onion_skin(origin_circuit_t *circ) } r = 1; } else { - r = entry_guard_succeeded(get_guard_selection_info(), - &circ->guard_state); + r = entry_guard_succeeded(&circ->guard_state); } const int is_usable_for_streams = (r == 1); if (r == 1) { diff --git a/src/or/circuitlist.c b/src/or/circuitlist.c index 0afe2f8..b25f817 100644 --- a/src/or/circuitlist.c +++ b/src/or/circuitlist.c @@ -898,7 +898,7 @@ circuit_free(circuit_t *circ)
/* Cancel before freeing, if we haven't already succeeded or failed. */ if (ocirc->guard_state) { - entry_guard_cancel(get_guard_selection_info(), ô->guard_state); + entry_guard_cancel(ô->guard_state); } circuit_guard_state_free(ocirc->guard_state);
diff --git a/src/or/circuituse.c b/src/or/circuituse.c index b925729..698b158 100644 --- a/src/or/circuituse.c +++ b/src/or/circuituse.c @@ -1653,7 +1653,7 @@ circuit_build_failed(origin_circuit_t *circ) } if (n_chan_id && !already_marked) { if (circ->guard_state) - entry_guard_failed(get_guard_selection_info(), &circ->guard_state); + entry_guard_failed(&circ->guard_state); /* XXXX prop271 -- old API */ entry_guard_register_connect_status(n_chan_id, 0, 1, time(NULL)); /* if there are any one-hop streams waiting on this circuit, fail diff --git a/src/or/connection.c b/src/or/connection.c index 25c75ff..87f0f91 100644 --- a/src/or/connection.c +++ b/src/or/connection.c @@ -636,7 +636,7 @@ connection_free_(connection_t *conn) rend_data_free(dir_conn->rend_data); if (dir_conn->guard_state) { /* Cancel before freeing, if it's still there. */ - entry_guard_cancel(get_guard_selection_info(), &dir_conn->guard_state); + entry_guard_cancel(&dir_conn->guard_state); } circuit_guard_state_free(dir_conn->guard_state); } diff --git a/src/or/connection_or.c b/src/or/connection_or.c index fefcc86..14d5979 100644 --- a/src/or/connection_or.c +++ b/src/or/connection_or.c @@ -735,8 +735,7 @@ connection_or_about_to_close(or_connection_t *or_conn) const or_options_t *options = get_options(); connection_or_note_state_when_broken(or_conn); rep_hist_note_connect_failed(or_conn->identity_digest, now); - entry_guard_chan_failed(get_guard_selection_info(), - TLS_CHAN_TO_BASE(or_conn->chan)); + entry_guard_chan_failed(TLS_CHAN_TO_BASE(or_conn->chan)); /* XXXX prop271 -- old API */ entry_guard_register_connect_status(or_conn->identity_digest,0, !options->HTTPSProxy, now); @@ -1676,8 +1675,7 @@ connection_or_client_learned_peer_id(or_connection_t *conn, "Tried connecting to router at %s:%d, but identity key was not " "as expected: wanted %s but got %s.%s", conn->base_.address, conn->base_.port, expected, seen, extra_log); - entry_guard_chan_failed(get_guard_selection_info(), - TLS_CHAN_TO_BASE(conn->chan)); + entry_guard_chan_failed(TLS_CHAN_TO_BASE(conn->chan)); /* XXXX prop271 old API */ entry_guard_register_connect_status(conn->identity_digest, 0, 1, time(NULL)); diff --git a/src/or/directory.c b/src/or/directory.c index 4164672..6fc8809 100644 --- a/src/or/directory.c +++ b/src/or/directory.c @@ -902,7 +902,7 @@ connection_dir_request_failed(dir_connection_t *conn) if (conn->guard_state) { /* We haven't seen a success on this guard state, so consider it to have * failed. */ - entry_guard_failed(get_guard_selection_info(), &conn->guard_state); + entry_guard_failed(&conn->guard_state); } if (directory_conn_is_self_reachability_test(conn)) { return; /* this was a test fetch. don't retry. */ @@ -1271,7 +1271,7 @@ directory_initiate_command_rend(const tor_addr_port_t *or_addr_port,
// In this case we should not have picked a directory guard. if (BUG(guard_state)) { - entry_guard_cancel(get_guard_selection_info(), &guard_state); + entry_guard_cancel(&guard_state); }
switch (connection_connect(TO_CONN(conn), conn->base_.address, &addr, @@ -1313,7 +1313,7 @@ directory_initiate_command_rend(const tor_addr_port_t *or_addr_port, // In this case we should not have a directory guard; we'll // get a regular guard later when we build the circuit. if (BUG(anonymized_connection && guard_state)) { - entry_guard_cancel(get_guard_selection_info(), &guard_state); + entry_guard_cancel(&guard_state); }
conn->guard_state = guard_state; @@ -2580,8 +2580,7 @@ connection_dir_process_inbuf(dir_connection_t *conn) */ /* XXXXprop271 should we count this as only a partial success somehow? */ - entry_guard_succeeded(get_guard_selection_info(), - &conn->guard_state); + entry_guard_succeeded(&conn->guard_state); circuit_guard_state_free(conn->guard_state); conn->guard_state = NULL; } diff --git a/src/or/entrynodes.c b/src/or/entrynodes.c index e0626cf..0795d19 100644 --- a/src/or/entrynodes.c +++ b/src/or/entrynodes.c @@ -1731,8 +1731,7 @@ entry_guard_pick_for_circuit(guard_selection_t *gs, * XXXXX prop271 tristates are ugly; reconsider that interface. */ int -entry_guard_succeeded(guard_selection_t *gs, - circuit_guard_state_t **guard_state_p) +entry_guard_succeeded(circuit_guard_state_t **guard_state_p) { if (get_options()->UseDeprecatedGuardAlgorithm) return 1; @@ -1741,13 +1740,12 @@ entry_guard_succeeded(guard_selection_t *gs, return -1;
entry_guard_t *guard = entry_guard_handle_get((*guard_state_p)->guard); - if (! guard) + if (! guard || BUG(guard->in_selection == NULL)) return -1;
- tor_assert(gs == guard->in_selection); // XXXX prop271 remove argument - unsigned newstate = - entry_guards_note_guard_success(gs, guard, (*guard_state_p)->state); + entry_guards_note_guard_success(guard->in_selection, guard, + (*guard_state_p)->state);
(*guard_state_p)->state = newstate; (*guard_state_p)->state_set_at = approx_time(); @@ -1763,10 +1761,8 @@ entry_guard_succeeded(guard_selection_t *gs, * success or failure. It is safe to call this function if success or * failure _has_ already been declared. */ void -entry_guard_cancel(guard_selection_t *gs, - circuit_guard_state_t **guard_state_p) +entry_guard_cancel(circuit_guard_state_t **guard_state_p) { - (void) gs; if (get_options()->UseDeprecatedGuardAlgorithm) return; if (BUG(*guard_state_p == NULL)) @@ -1775,8 +1771,6 @@ entry_guard_cancel(guard_selection_t *gs, if (! guard) return;
- tor_assert(gs == guard->in_selection); // XXXX prop271 remove argument - /* XXXX prop271 -- last_tried_to_connect_at will be erroneous here, but this * function will only get called in "bug" cases anyway. */ guard->is_pending = 0; @@ -1790,8 +1784,7 @@ entry_guard_cancel(guard_selection_t *gs, * not working, and advances the state of the guard module. */ void -entry_guard_failed(guard_selection_t *gs, - circuit_guard_state_t **guard_state_p) +entry_guard_failed(circuit_guard_state_t **guard_state_p) { if (get_options()->UseDeprecatedGuardAlgorithm) return; @@ -1800,12 +1793,10 @@ entry_guard_failed(guard_selection_t *gs, return;
entry_guard_t *guard = entry_guard_handle_get((*guard_state_p)->guard); - if (! guard) + if (! guard || BUG(guard->in_selection == NULL)) return;
- tor_assert(gs == guard->in_selection); // XXXX prop271 remove argument - - entry_guards_note_guard_failure(gs, guard); + entry_guards_note_guard_failure(guard->in_selection, guard);
(*guard_state_p)->state = GUARD_CIRC_STATE_DEAD; (*guard_state_p)->state_set_at = approx_time(); @@ -1816,10 +1807,8 @@ entry_guard_failed(guard_selection_t *gs, * pending on <b>chan</b>. */ void -entry_guard_chan_failed(guard_selection_t *gs, - channel_t *chan) +entry_guard_chan_failed(channel_t *chan) { - tor_assert(gs); if (!chan) return; if (get_options()->UseDeprecatedGuardAlgorithm) @@ -1832,7 +1821,7 @@ entry_guard_chan_failed(guard_selection_t *gs, continue;
origin_circuit_t *origin_circ = TO_ORIGIN_CIRCUIT(circ); - entry_guard_failed(gs, &origin_circ->guard_state); + entry_guard_failed(&origin_circ->guard_state); } SMARTLIST_FOREACH_END(circ); smartlist_free(pending); } diff --git a/src/or/entrynodes.h b/src/or/entrynodes.h index 97cc4d2..0abbea8 100644 --- a/src/or/entrynodes.h +++ b/src/or/entrynodes.h @@ -351,14 +351,10 @@ void circuit_guard_state_free(circuit_guard_state_t *state); int entry_guard_pick_for_circuit(guard_selection_t *gs, const node_t **chosen_node_out, circuit_guard_state_t **guard_state_out); -int entry_guard_succeeded(guard_selection_t *gs, - circuit_guard_state_t **guard_state_p); -void entry_guard_failed(guard_selection_t *gs, - circuit_guard_state_t **guard_state_p); -void entry_guard_cancel(guard_selection_t *gs, - circuit_guard_state_t **guard_state_p); -void entry_guard_chan_failed(guard_selection_t *gs, - channel_t *chan); +int entry_guard_succeeded(circuit_guard_state_t **guard_state_p); +void entry_guard_failed(circuit_guard_state_t **guard_state_p); +void entry_guard_cancel(circuit_guard_state_t **guard_state_p); +void entry_guard_chan_failed(channel_t *chan); int entry_guards_update_all(guard_selection_t *gs); int entry_guards_upgrade_waiting_circuits(guard_selection_t *gs, const smartlist_t *all_circuits, diff --git a/src/test/test_entrynodes.c b/src/test/test_entrynodes.c index 4595e5d..bc0862a 100644 --- a/src/test/test_entrynodes.c +++ b/src/test/test_entrynodes.c @@ -2333,7 +2333,7 @@ test_entry_guard_select_for_circuit_highlevel_primary(void *arg)
/* Call that circuit successful. */ update_approx_time(start+15); - r = entry_guard_succeeded(gs, &guard); + r = entry_guard_succeeded(&guard); tt_int_op(r, OP_EQ, 1); /* We can use it now. */ tt_assert(guard); tt_int_op(guard->state, OP_EQ, GUARD_CIRC_STATE_COMPLETE); @@ -2365,7 +2365,7 @@ test_entry_guard_select_for_circuit_highlevel_primary(void *arg)
/* It's failed! What will happen to our poor guard? */ update_approx_time(start+45); - entry_guard_failed(gs, &guard); + entry_guard_failed(&guard); tt_assert(guard); tt_int_op(guard->state, OP_EQ, GUARD_CIRC_STATE_DEAD); tt_i64_op(guard->state_set_at, OP_EQ, start+45); @@ -2401,7 +2401,7 @@ test_entry_guard_select_for_circuit_highlevel_primary(void *arg)
/* Call this one up; watch it get confirmed. */ update_approx_time(start+90); - r = entry_guard_succeeded(gs, &guard); + r = entry_guard_succeeded(&guard); tt_int_op(r, OP_EQ, 1); /* We can use it now. */ tt_assert(guard); tt_int_op(guard->state, OP_EQ, GUARD_CIRC_STATE_COMPLETE); @@ -2441,7 +2441,7 @@ test_entry_guard_select_for_circuit_highlevel_confirm_other(void *arg) tt_assert(guard); tt_assert(r == 0); tt_int_op(guard->state, OP_EQ, GUARD_CIRC_STATE_USABLE_ON_COMPLETION); - entry_guard_failed(gs, &guard); + entry_guard_failed(&guard); circuit_guard_state_free(guard); guard = NULL; node = NULL; @@ -2461,7 +2461,7 @@ test_entry_guard_select_for_circuit_highlevel_confirm_other(void *arg) tt_int_op(g->is_pending, OP_EQ, 1); (void)start;
- r = entry_guard_succeeded(gs, &guard); + r = entry_guard_succeeded(&guard); /* We're on the internet (by fiat), so this guard will get called "confirmed" * and should immediately become primary. * XXXX prop271 -- I don't like that behavior, but it's what is specified @@ -2508,7 +2508,7 @@ test_entry_guard_select_for_circuit_highlevel_primary_retry(void *arg) g = entry_guard_handle_get(guard->guard); make_guard_confirmed(gs, g); tt_int_op(g->is_primary, OP_EQ, 1); - entry_guard_failed(gs, &guard); + entry_guard_failed(&guard); circuit_guard_state_free(guard); tt_int_op(g->is_reachable, OP_EQ, GUARD_REACHABLE_NO); guard = NULL; @@ -2530,7 +2530,7 @@ test_entry_guard_select_for_circuit_highlevel_primary_retry(void *arg) update_approx_time(start + 3600);
/* Say that guard has succeeded! */ - r = entry_guard_succeeded(gs, &guard); + r = entry_guard_succeeded(&guard); tt_int_op(r, OP_EQ, 0); // can't use it yet. tt_int_op(guard->state, OP_EQ, GUARD_CIRC_STATE_WAITING_FOR_BETTER_GUARD); g = entry_guard_handle_get(guard->guard); @@ -2546,7 +2546,7 @@ test_entry_guard_select_for_circuit_highlevel_primary_retry(void *arg) r = entry_guard_pick_for_circuit(gs, &node, &guard2); tt_assert(r == 0); tt_int_op(guard2->state, OP_EQ, GUARD_CIRC_STATE_USABLE_ON_COMPLETION); - r = entry_guard_succeeded(gs, &guard2); + r = entry_guard_succeeded(&guard2); tt_assert(r == 1); tt_int_op(guard2->state, OP_EQ, GUARD_CIRC_STATE_COMPLETE);
@@ -2578,7 +2578,7 @@ test_entry_guard_select_and_cancel(void *arg) tt_int_op(g->is_primary, OP_EQ, 1); tt_int_op(g->is_pending, OP_EQ, 0); make_guard_confirmed(gs, g); - entry_guard_failed(gs, &guard); + entry_guard_failed(&guard); circuit_guard_state_free(guard); guard = NULL; node = NULL; @@ -2597,7 +2597,7 @@ test_entry_guard_select_and_cancel(void *arg) tt_int_op(g->is_pending, OP_EQ, 1);
/* Whoops! We should never have asked for this guard. Cancel the request! */ - entry_guard_cancel(gs, &guard); + entry_guard_cancel(&guard); tt_assert(guard == NULL); tt_int_op(g->is_primary, OP_EQ, 0); tt_int_op(g->is_pending, OP_EQ, 0); @@ -2649,7 +2649,7 @@ upgrade_circuits_setup(const struct testcase_t *testcase) entry_guard_pick_for_circuit(gs, &node, &guard); g = entry_guard_handle_get(guard->guard); make_guard_confirmed(gs, g); - entry_guard_failed(gs, &guard); + entry_guard_failed(&guard); circuit_guard_state_free(guard); }
@@ -2682,14 +2682,14 @@ upgrade_circuits_setup(const struct testcase_t *testcase) int r; update_approx_time(data->start + 32); if (make_circ1_succeed) { - r = entry_guard_succeeded(gs, &data->guard1_state); + r = entry_guard_succeeded(&data->guard1_state); tor_assert(r == 0); tor_assert(data->guard1_state->state == GUARD_CIRC_STATE_WAITING_FOR_BETTER_GUARD); } update_approx_time(data->start + 33); if (make_circ2_succeed) { - r = entry_guard_succeeded(gs, &data->guard2_state); + r = entry_guard_succeeded(&data->guard2_state); tor_assert(r == 0); tor_assert(data->guard2_state->state == GUARD_CIRC_STATE_WAITING_FOR_BETTER_GUARD);