[tor-commits] [tor/maint-0.4.0] Merge branch bug29706_029_refactor into bug29706_034_refactor
nickm at torproject.org
nickm at torproject.org
Tue Mar 12 15:03:57 UTC 2019
commit c7854933e9d8a6634beb7079e7c8af3266a362a8
Merge: 155b0f552 9eeff921a
Author: teor <teor at torproject.org>
Date: Tue Mar 12 11:31:52 2019 +1000
Merge branch bug29706_029_refactor into bug29706_034_refactor
changes/bug29706_minimal | 4 +
changes/bug29706_refactor | 4 +
src/or/dirauth/shared_random.c | 8 +-
src/or/dirauth/shared_random.h | 2 +-
src/or/dirauth/shared_random_state.c | 60 +++++++--
src/or/dirauth/shared_random_state.h | 2 +
src/test/test_shared_random.c | 245 ++++++++++++++++++++++++++++++++---
7 files changed, 290 insertions(+), 35 deletions(-)
diff --cc src/or/dirauth/shared_random_state.h
index 60a326f86,cf027f2d3..d31983d18
--- a/src/or/dirauth/shared_random_state.h
+++ b/src/or/dirauth/shared_random_state.h
@@@ -140,8 -140,10 +140,10 @@@ STATIC int is_phase_transition(sr_phase
STATIC void set_sr_phase(sr_phase_t phase);
STATIC sr_state_t *get_sr_state(void);
+ STATIC void state_del_previous_srv(void);
+ STATIC void state_del_current_srv(void);
-#endif /* TOR_UNIT_TESTS */
+#endif /* defined(TOR_UNIT_TESTS) */
-#endif /* TOR_SHARED_RANDOM_STATE_H */
+#endif /* !defined(TOR_SHARED_RANDOM_STATE_H) */
diff --cc src/test/test_shared_random.c
index c04e4660c,c81b6ec2d..45f0e1db7
--- a/src/test/test_shared_random.c
+++ b/src/test/test_shared_random.c
@@@ -1031,9 -944,9 +1039,9 @@@ test_utils_general(void *arg
srv = tor_malloc_zero(sizeof(*srv));
srv->num_reveals = 42;
memcpy(srv->value, srv_value, sizeof(srv->value));
- dup_srv = srv_dup(srv);
+ dup_srv = sr_srv_dup(srv);
tt_assert(dup_srv);
- tt_u64_op(dup_srv->num_reveals, ==, srv->num_reveals);
+ tt_u64_op(dup_srv->num_reveals, OP_EQ, srv->num_reveals);
tt_mem_op(dup_srv->value, OP_EQ, srv->value, sizeof(srv->value));
tor_free(srv);
tor_free(dup_srv);
@@@ -1078,25 -991,220 +1086,220 @@@
/* Testing get_phase_str(). */
{
- tt_str_op(get_phase_str(SR_PHASE_REVEAL), ==, "reveal");
- tt_str_op(get_phase_str(SR_PHASE_COMMIT), ==, "commit");
+ tt_str_op(get_phase_str(SR_PHASE_REVEAL), OP_EQ, "reveal");
+ tt_str_op(get_phase_str(SR_PHASE_COMMIT), OP_EQ, "commit");
}
+ done:
+ return;
+ }
+
+ /* Test utils that depend on authority state */
+ static void
+ test_utils_auth(void *arg)
+ {
+ (void)arg;
+ init_authority_state();
+
/* Testing phase transition */
{
- init_authority_state();
set_sr_phase(SR_PHASE_COMMIT);
- tt_int_op(is_phase_transition(SR_PHASE_REVEAL), ==, 1);
- tt_int_op(is_phase_transition(SR_PHASE_COMMIT), ==, 0);
+ tt_int_op(is_phase_transition(SR_PHASE_REVEAL), OP_EQ, 1);
+ tt_int_op(is_phase_transition(SR_PHASE_COMMIT), OP_EQ, 0);
set_sr_phase(SR_PHASE_REVEAL);
- tt_int_op(is_phase_transition(SR_PHASE_REVEAL), ==, 0);
- tt_int_op(is_phase_transition(SR_PHASE_COMMIT), ==, 1);
+ tt_int_op(is_phase_transition(SR_PHASE_REVEAL), OP_EQ, 0);
+ tt_int_op(is_phase_transition(SR_PHASE_COMMIT), OP_EQ, 1);
/* Junk. */
- tt_int_op(is_phase_transition(42), ==, 1);
+ tt_int_op(is_phase_transition(42), OP_EQ, 1);
}
+ /* Testing get, set, delete, clean SRVs */
+
+ {
+ /* Just set the previous SRV */
+ test_sr_setup_srv(0);
+ tt_ptr_op(sr_state_get_previous_srv(), OP_NE, NULL);
+ tt_ptr_op(sr_state_get_current_srv(), OP_EQ, NULL);
+ state_del_previous_srv();
+ tt_ptr_op(sr_state_get_previous_srv(), OP_EQ, NULL);
+ tt_ptr_op(sr_state_get_current_srv(), OP_EQ, NULL);
+ }
+
+ {
+ /* Delete the SRVs one at a time */
+ test_sr_setup_srv(1);
+ tt_ptr_op(sr_state_get_previous_srv(), OP_NE, NULL);
+ tt_ptr_op(sr_state_get_current_srv(), OP_NE, NULL);
+ state_del_current_srv();
+ tt_ptr_op(sr_state_get_previous_srv(), OP_NE, NULL);
+ tt_ptr_op(sr_state_get_current_srv(), OP_EQ, NULL);
+ state_del_previous_srv();
+ tt_ptr_op(sr_state_get_previous_srv(), OP_EQ, NULL);
+ tt_ptr_op(sr_state_get_current_srv(), OP_EQ, NULL);
+
+ /* And in the opposite order */
+ test_sr_setup_srv(1);
+ tt_ptr_op(sr_state_get_previous_srv(), OP_NE, NULL);
+ tt_ptr_op(sr_state_get_current_srv(), OP_NE, NULL);
+ state_del_previous_srv();
+ tt_ptr_op(sr_state_get_previous_srv(), OP_EQ, NULL);
+ tt_ptr_op(sr_state_get_current_srv(), OP_NE, NULL);
+ state_del_current_srv();
+ tt_ptr_op(sr_state_get_previous_srv(), OP_EQ, NULL);
+ tt_ptr_op(sr_state_get_current_srv(), OP_EQ, NULL);
+
+ /* And both at once */
+ test_sr_setup_srv(1);
+ tt_ptr_op(sr_state_get_previous_srv(), OP_NE, NULL);
+ tt_ptr_op(sr_state_get_current_srv(), OP_NE, NULL);
+ sr_state_clean_srvs();
+ tt_ptr_op(sr_state_get_previous_srv(), OP_EQ, NULL);
+ tt_ptr_op(sr_state_get_current_srv(), OP_EQ, NULL);
+
+ /* And do the gets and sets multiple times */
+ test_sr_setup_srv(1);
+ tt_ptr_op(sr_state_get_previous_srv(), OP_NE, NULL);
+ tt_ptr_op(sr_state_get_current_srv(), OP_NE, NULL);
+ tt_ptr_op(sr_state_get_previous_srv(), OP_NE, NULL);
+ tt_ptr_op(sr_state_get_current_srv(), OP_NE, NULL);
+ state_del_previous_srv();
+ tt_ptr_op(sr_state_get_previous_srv(), OP_EQ, NULL);
+ tt_ptr_op(sr_state_get_current_srv(), OP_NE, NULL);
+ state_del_previous_srv();
+ tt_ptr_op(sr_state_get_previous_srv(), OP_EQ, NULL);
+ tt_ptr_op(sr_state_get_current_srv(), OP_NE, NULL);
+ tt_ptr_op(sr_state_get_previous_srv(), OP_EQ, NULL);
+ tt_ptr_op(sr_state_get_current_srv(), OP_NE, NULL);
+ sr_state_clean_srvs();
+ tt_ptr_op(sr_state_get_previous_srv(), OP_EQ, NULL);
+ tt_ptr_op(sr_state_get_current_srv(), OP_EQ, NULL);
+ state_del_current_srv();
+ tt_ptr_op(sr_state_get_previous_srv(), OP_EQ, NULL);
+ tt_ptr_op(sr_state_get_current_srv(), OP_EQ, NULL);
+ sr_state_clean_srvs();
+ tt_ptr_op(sr_state_get_previous_srv(), OP_EQ, NULL);
+ tt_ptr_op(sr_state_get_current_srv(), OP_EQ, NULL);
+ state_del_current_srv();
+ tt_ptr_op(sr_state_get_previous_srv(), OP_EQ, NULL);
+ tt_ptr_op(sr_state_get_current_srv(), OP_EQ, NULL);
+ tt_ptr_op(sr_state_get_previous_srv(), OP_EQ, NULL);
+ tt_ptr_op(sr_state_get_current_srv(), OP_EQ, NULL);
+ }
+
+ {
+ /* Now set the SRVs to NULL instead */
+ test_sr_setup_srv(1);
+ tt_ptr_op(sr_state_get_previous_srv(), OP_NE, NULL);
+ tt_ptr_op(sr_state_get_current_srv(), OP_NE, NULL);
+ sr_state_set_current_srv(NULL);
+ tt_ptr_op(sr_state_get_previous_srv(), OP_NE, NULL);
+ tt_ptr_op(sr_state_get_current_srv(), OP_EQ, NULL);
+ sr_state_set_previous_srv(NULL);
+ tt_ptr_op(sr_state_get_previous_srv(), OP_EQ, NULL);
+ tt_ptr_op(sr_state_get_current_srv(), OP_EQ, NULL);
+
+ /* And in the opposite order */
+ test_sr_setup_srv(1);
+ tt_ptr_op(sr_state_get_previous_srv(), OP_NE, NULL);
+ tt_ptr_op(sr_state_get_current_srv(), OP_NE, NULL);
+ sr_state_set_previous_srv(NULL);
+ tt_ptr_op(sr_state_get_previous_srv(), OP_EQ, NULL);
+ tt_ptr_op(sr_state_get_current_srv(), OP_NE, NULL);
+ sr_state_set_current_srv(NULL);
+ tt_ptr_op(sr_state_get_previous_srv(), OP_EQ, NULL);
+ tt_ptr_op(sr_state_get_current_srv(), OP_EQ, NULL);
+
+ /* And both at once */
+ test_sr_setup_srv(1);
+ tt_ptr_op(sr_state_get_previous_srv(), OP_NE, NULL);
+ tt_ptr_op(sr_state_get_current_srv(), OP_NE, NULL);
+ sr_state_clean_srvs();
+ tt_ptr_op(sr_state_get_previous_srv(), OP_EQ, NULL);
+ tt_ptr_op(sr_state_get_current_srv(), OP_EQ, NULL);
+
+ /* And do the gets and sets multiple times */
+ test_sr_setup_srv(1);
+ tt_ptr_op(sr_state_get_previous_srv(), OP_NE, NULL);
+ tt_ptr_op(sr_state_get_current_srv(), OP_NE, NULL);
+ sr_state_set_previous_srv(NULL);
+ sr_state_set_previous_srv(NULL);
+ tt_ptr_op(sr_state_get_previous_srv(), OP_EQ, NULL);
+ tt_ptr_op(sr_state_get_current_srv(), OP_NE, NULL);
+ tt_ptr_op(sr_state_get_previous_srv(), OP_EQ, NULL);
+ tt_ptr_op(sr_state_get_current_srv(), OP_NE, NULL);
+ sr_state_set_current_srv(NULL);
+ sr_state_set_previous_srv(NULL);
+ sr_state_set_current_srv(NULL);
+ tt_ptr_op(sr_state_get_previous_srv(), OP_EQ, NULL);
+ tt_ptr_op(sr_state_get_current_srv(), OP_EQ, NULL);
+ }
+
+ {
+ /* Now copy the values across */
+ test_sr_setup_srv(1);
+ /* Check that the pointers are non-NULL, and different from each other */
+ tt_ptr_op(sr_state_get_previous_srv(), OP_NE, NULL);
+ tt_ptr_op(sr_state_get_current_srv(), OP_NE, NULL);
+ tt_ptr_op(sr_state_get_previous_srv(), OP_NE,
+ sr_state_get_current_srv());
+ /* Check that the content is different */
+ tt_mem_op(sr_state_get_previous_srv(), OP_NE,
+ sr_state_get_current_srv(), sizeof(sr_srv_t));
+ /* Set the current to the previous: the protocol goes the other way */
+ sr_state_set_current_srv(sr_srv_dup(sr_state_get_previous_srv()));
+ /* Check that the pointers are non-NULL, and different from each other */
+ tt_ptr_op(sr_state_get_previous_srv(), OP_NE, NULL);
+ tt_ptr_op(sr_state_get_current_srv(), OP_NE, NULL);
+ tt_ptr_op(sr_state_get_previous_srv(), OP_NE,
+ sr_state_get_current_srv());
+ /* Check that the content is the same */
+ tt_mem_op(sr_state_get_previous_srv(), OP_EQ,
+ sr_state_get_current_srv(), sizeof(sr_srv_t));
+ }
+
+ {
+ /* Now copy a value onto itself */
+ test_sr_setup_srv(1);
+ /* Check that the pointers are non-NULL, and different from each other */
+ tt_ptr_op(sr_state_get_previous_srv(), OP_NE, NULL);
+ tt_ptr_op(sr_state_get_current_srv(), OP_NE, NULL);
+ tt_ptr_op(sr_state_get_previous_srv(), OP_NE,
+ sr_state_get_current_srv());
+ /* Take a copy of the old value */
+ sr_srv_t old_current_srv;
+ memcpy(&old_current_srv, sr_state_get_current_srv(), sizeof(sr_srv_t));
+ /* Check that the content is different */
+ tt_mem_op(sr_state_get_previous_srv(), OP_NE,
+ sr_state_get_current_srv(), sizeof(sr_srv_t));
+ /* Set the current to the current: the protocol never replaces an SRV with
+ * the same value */
+ sr_state_set_current_srv(sr_srv_dup(sr_state_get_current_srv()));
+ /* Check that the pointers are non-NULL, and different from each other */
+ tt_ptr_op(sr_state_get_previous_srv(), OP_NE, NULL);
+ tt_ptr_op(sr_state_get_current_srv(), OP_NE, NULL);
+ tt_ptr_op(sr_state_get_previous_srv(), OP_NE,
+ sr_state_get_current_srv());
+ /* Check that the content is different between current and previous */
+ tt_mem_op(sr_state_get_previous_srv(), OP_NE,
+ sr_state_get_current_srv(), sizeof(sr_srv_t));
+ /* Check that the content is the same as the old content */
+ tt_mem_op(&old_current_srv, OP_EQ,
+ sr_state_get_current_srv(), sizeof(sr_srv_t));
+ }
+
+ /* I don't think we can say "expect a BUG()" in our tests. */
+ #if 0
+ {
+ /* Now copy a value onto itself without sr_srv_dup().
+ * This should fail with a BUG() warning. */
+ test_sr_setup_srv(1);
+ sr_state_set_current_srv(sr_state_get_current_srv());
+ sr_state_set_previous_srv(sr_state_get_previous_srv());
+ }
+ #endif
+
done:
- return;
+ sr_state_free();
}
static void
@@@ -1142,16 -1251,18 +1346,18 @@@ test_state_transition(void *arg
/* Test SRV rotation in our state. */
{
- const sr_srv_t *cur, *prev;
test_sr_setup_srv(1);
- cur = sr_state_get_current_srv();
+ tt_assert(sr_state_get_current_srv());
+ /* Take a copy of the data, because the state owns the pointer */
+ cur = sr_srv_dup(sr_state_get_current_srv());
tt_assert(cur);
- /* After, current srv should be the previous and then set to NULL. */
+ /* After, the previous SRV should be the same as the old current SRV, and
+ * the current SRV should be set to NULL */
state_rotate_srv();
- prev = sr_state_get_previous_srv();
- tt_assert(prev == cur);
+ tt_mem_op(sr_state_get_previous_srv(), OP_EQ, cur, sizeof(sr_srv_t));
- tt_assert(!sr_state_get_current_srv());
+ tt_ptr_op(sr_state_get_current_srv(), OP_EQ, NULL);
sr_state_clean_srvs();
+ tor_free(cur);
}
/* New protocol run. */
@@@ -1173,13 -1284,14 +1379,14 @@@
* compute a new SRV. */
tt_assert(sr_state_get_current_srv());
/* Also, make sure we did change the current. */
- tt_assert(sr_state_get_current_srv() != cur);
+ tt_mem_op(sr_state_get_current_srv(), OP_NE, cur, sizeof(sr_srv_t));
/* We should have our commitment alone. */
- tt_int_op(digestmap_size(state->commits), ==, 1);
- tt_int_op(state->n_reveal_rounds, ==, 0);
- tt_int_op(state->n_commit_rounds, ==, 0);
+ tt_int_op(digestmap_size(state->commits), OP_EQ, 1);
+ tt_int_op(state->n_reveal_rounds, OP_EQ, 0);
+ tt_int_op(state->n_commit_rounds, OP_EQ, 0);
/* 46 here since we were at 45 just before. */
- tt_u64_op(state->n_protocol_runs, ==, 46);
+ tt_u64_op(state->n_protocol_runs, OP_EQ, 46);
+ tor_free(cur);
}
/* Cleanup of SRVs. */
@@@ -1190,7 -1302,8 +1397,8 @@@
}
done:
+ tor_free(cur);
- sr_state_free();
+ sr_state_free_all();
}
static void
More information about the tor-commits
mailing list