Merge branch bug29706_029_refactor into bug29706_034_refactor

diff --cc src/or/dirauth/shared_random.c
Simple merge

diff --cc src/or/dirauth/shared_random.h
Simple merge

diff --cc src/or/dirauth/shared_random_state.c
Simple merge

diff --cc src/or/dirauth/shared_random_state.h
index 60a326f86c76ff579c5d90bd9491aec316b81c4f,cf027f2d35e16868fb4567d6853a76154cfdfaa4..d31983d18783cccfe1c25cec5f70b68dd539e544
--- 1/src/or/dirauth/shared_random_state.h
--- 2/src/or/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 c04e4660c31d63f36287e6a3f507b9bca6b59f77,c81b6ec2d6c7cc990636296bd29f82dd8064c184..45f0e1db7f733c46b1239f4bc7dcbc61d01f2685
--- 1/src/test/test_shared_random.c
--- 2/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:
 -  sr_state_free();
+   tor_free(cur);
 +  sr_state_free_all();
  }
  
  static void