commit 36e201dffc5d57d92113e1e68ad4b6a0a7b3471e Author: David Goulet dgoulet@torproject.org Date: Wed Jun 29 15:32:51 2016 -0400

prop250: Add a DEL state action and return const SRVs

The *get* state query functions for the SRVs now only return const pointers and the DEL action needs to be used to delete the SRVs from the state.

Signed-off-by: David Goulet dgoulet@torproject.org --- src/or/shared_random.c | 2 +- src/or/shared_random_state.c | 91 ++++++++++++++++++++++++++++++------------- src/or/shared_random_state.h | 9 +++-- src/test/test_shared_random.c | 6 +-- 4 files changed, 73 insertions(+), 35 deletions(-)

diff --git a/src/or/shared_random.c b/src/or/shared_random.c index 5cdf3d0..7a0273d 100644 --- a/src/or/shared_random.c +++ b/src/or/shared_random.c @@ -589,7 +589,7 @@ STATIC int should_keep_commit(const sr_commit_t *commit, const char *voter_key, sr_phase_t phase) { - sr_commit_t *saved_commit; + const sr_commit_t *saved_commit;

tor_assert(commit); tor_assert(voter_key); diff --git a/src/or/shared_random_state.c b/src/or/shared_random_state.c index eeffb14..d0cd460 100644 --- a/src/or/shared_random_state.c +++ b/src/or/shared_random_state.c @@ -793,20 +793,6 @@ reset_state_for_new_protocol_run(time_t valid_after) sr_state_delete_commits(); }

-/* Rotate SRV value by freeing the previous value, assigning the current - * value to the previous one and nullifying the current one. */ -STATIC void -state_rotate_srv(void) -{ - /* Get a pointer to the previous SRV so we can free it after rotation. */ - sr_srv_t *previous_srv = sr_state_get_previous_srv(); - /* Set previous SRV with the current one. */ - sr_state_set_previous_srv(sr_state_get_current_srv()); - /* Nullify the current srv. */ - sr_state_set_current_srv(NULL); - tor_free(previous_srv); -} - /* This is the first round of the new protocol run starting at * <b>valid_after</b>. Do the necessary housekeeping. */ STATIC void @@ -857,6 +843,7 @@ state_query_get_commit(const char *rsa_fpr) tor_assert(rsa_fpr); return digestmap_get(sr_state->commits, rsa_fpr); } + /* Helper function: This handles the GET state action using an * <b>obj_type</b> and <b>data</b> needed for the action. */ static void * @@ -921,7 +908,7 @@ state_query_put_(sr_state_object_t obj_type, void *data) } }

-/* Helper function: This handles the DEL state action using an +/* Helper function: This handles the DEL_ALL state action using an * <b>obj_type</b> and <b>data</b> needed for the action. */ static void state_query_del_all_(sr_state_object_t obj_type) @@ -947,6 +934,29 @@ state_query_del_all_(sr_state_object_t obj_type) } }

+/* Helper function: This handles the DEL state action using an + * <b>obj_type</b> and <b>data</b> needed for the action. */ +static void +state_query_del_(sr_state_object_t obj_type, void *data) +{ + (void) data; + + switch (obj_type) { + case SR_STATE_OBJ_PREVSRV: + tor_free(sr_state->previous_srv); + break; + case SR_STATE_OBJ_CURSRV: + tor_free(sr_state->current_srv); + break; + case SR_STATE_OBJ_COMMIT: + case SR_STATE_OBJ_COMMITS: + case SR_STATE_OBJ_PHASE: + case SR_STATE_OBJ_VALID_AFTER: + default: + tor_assert(0); + } +} + /* Query state using an <b>action</b> for an object type <b>obj_type</b>. * The <b>data</b> pointer needs to point to an object that the action needs * to use and if anything is required to be returned, it is stored in @@ -969,6 +979,9 @@ state_query(sr_state_action_t action, sr_state_object_t obj_type, case SR_STATE_ACTION_PUT: state_query_put_(obj_type, data); break; + case SR_STATE_ACTION_DEL: + state_query_del_(obj_type, data); + break; case SR_STATE_ACTION_DEL_ALL: state_query_del_all_(obj_type); break; @@ -986,6 +999,35 @@ state_query(sr_state_action_t action, sr_state_object_t obj_type, } }

+/* Delete the current SRV value from the state freeing it and the value is set + * to NULL meaning empty. */ +static void +state_del_current_srv(void) +{ + state_query(SR_STATE_ACTION_DEL, SR_STATE_OBJ_CURSRV, NULL, NULL); +} + +/* Delete the previous SRV value from the state freeing it and the value is + * set to NULL meaning empty. */ +static void +state_del_previous_srv(void) +{ + state_query(SR_STATE_ACTION_DEL, SR_STATE_OBJ_PREVSRV, NULL, NULL); +} + +/* Rotate SRV value by freeing the previous value, assigning the current + * value to the previous one and nullifying the current one. */ +STATIC void +state_rotate_srv(void) +{ + /* First delete previous SRV from the state. Object will be freed. */ + state_del_previous_srv(); + /* Set previous SRV with the current one. */ + sr_state_set_previous_srv(sr_state_get_current_srv()); + /* Nullify the current srv. */ + sr_state_set_current_srv(NULL); +} + /* Set valid after time in the our state. */ void sr_state_set_valid_after(time_t valid_after) @@ -1004,10 +1046,10 @@ sr_state_get_phase(void) }

/* Return the previous SRV value from our state. Value CAN be NULL. */ -sr_srv_t * +const sr_srv_t * sr_state_get_previous_srv(void) { - sr_srv_t *srv; + const sr_srv_t *srv; state_query(SR_STATE_ACTION_GET, SR_STATE_OBJ_PREVSRV, NULL, (void *) &srv); return srv; @@ -1023,10 +1065,10 @@ sr_state_set_previous_srv(const sr_srv_t *srv) }

/* Return the current SRV value from our state. Value CAN be NULL. */ -sr_srv_t * +const sr_srv_t * sr_state_get_current_srv(void) { - sr_srv_t *srv; + const sr_srv_t *srv; state_query(SR_STATE_ACTION_GET, SR_STATE_OBJ_CURSRV, NULL, (void *) &srv); return srv; @@ -1045,14 +1087,9 @@ sr_state_set_current_srv(const sr_srv_t *srv) void sr_state_clean_srvs(void) { - sr_srv_t *previous_srv = sr_state_get_previous_srv(); - sr_srv_t *current_srv = sr_state_get_current_srv(); - - tor_free(previous_srv); - sr_state_set_previous_srv(NULL); - - tor_free(current_srv); - sr_state_set_current_srv(NULL); + /* Remove SRVs from state. They will be set to NULL as "empty". */ + state_del_previous_srv(); + state_del_current_srv(); }

/* Return a pointer to the commits map from our state. CANNOT be NULL. */ diff --git a/src/or/shared_random_state.h b/src/or/shared_random_state.h index 305797f..e0b6e46 100644 --- a/src/or/shared_random_state.h +++ b/src/or/shared_random_state.h @@ -10,8 +10,9 @@ typedef enum { SR_STATE_ACTION_GET = 1, SR_STATE_ACTION_PUT = 2, - SR_STATE_ACTION_DEL_ALL = 3, - SR_STATE_ACTION_SAVE = 4, + SR_STATE_ACTION_DEL = 3, + SR_STATE_ACTION_DEL_ALL = 4, + SR_STATE_ACTION_SAVE = 5, } sr_state_action_t;

/* Object in the state that can be queried through the state API. */ @@ -101,8 +102,8 @@ void sr_state_update(time_t valid_after);

void sr_state_set_valid_after(time_t valid_after); sr_phase_t sr_state_get_phase(void); -sr_srv_t *sr_state_get_previous_srv(void); -sr_srv_t *sr_state_get_current_srv(void); +const sr_srv_t *sr_state_get_previous_srv(void); +const sr_srv_t *sr_state_get_current_srv(void); void sr_state_set_previous_srv(const sr_srv_t *srv); void sr_state_set_current_srv(const sr_srv_t *srv); void sr_state_clean_srvs(void); diff --git a/src/test/test_shared_random.c b/src/test/test_shared_random.c index b900350..20ed087 100644 --- a/src/test/test_shared_random.c +++ b/src/test/test_shared_random.c @@ -780,7 +780,7 @@ static void test_sr_compute_srv(void *arg) { (void) arg; - sr_srv_t *current_srv = NULL; + const sr_srv_t *current_srv = NULL;

#define SRV_TEST_VECTOR \ "2A9B1D6237DAB312A40F575DA85C147663E7ED3F80E9555395F15B515C74253D" @@ -1026,7 +1026,7 @@ test_state_transition(void *arg)

/* Test SRV rotation in our state. */ { - sr_srv_t *cur, *prev; + const sr_srv_t *cur, *prev; test_sr_setup_srv(1); cur = sr_state_get_current_srv(); tt_assert(cur); @@ -1039,7 +1039,7 @@ test_state_transition(void *arg)

/* New protocol run. */ { - sr_srv_t *cur; + const sr_srv_t *cur; /* Setup some new SRVs so we can confirm that a new protocol run * actually makes them rotate and compute new ones. */ test_sr_setup_srv(1);