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>

diff --git a/src/or/shared_random.c b/src/or/shared_random.c
index 5cdf3d028723eacd7a3113db11a123e3d3a1a952..7a0273d4958a158a8f79ae1fa2cfe84b06db7492 100644 (file)
--- 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 eeffb1479e23808e5e62aa386a7b728400dc4a62..d0cd4607566cb55eeb685e77abe5c65c9e04168d 100644 (file)
--- 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 305797f0ff68993aaaaca461772a76f38e388186..e0b6e461ba6ad9fb79bf4c6eeb2fff13e1ccb77a 100644 (file)
--- 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 b9003502b1f67f9b8b8e245a615ca0cc4a8259f3..20ed0871d784a1ee54e79d0c8b8e659edbafb5f7 100644 (file)
--- 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);