Merge remote-tracking branch 'tor-github/pr/834'

changes/bug29823

@@ -0,0 +1,5 @@
+  o Minor bugfixes (unit tests):
+    - Split test_utils_general() to several smaller test functions in
+      test_utils_general(). This makes it easier to perform resource
+      deallocation on assert failure and fixes Coverity warnings CID 1444117
+      and CID 1444118. Fixes bug 29823; bugfix on 0.2.9.1-alpha.

src/test/test_shared_random.c

@@ -1081,14 +1081,12 @@ test_sr_get_majority_srv_from_votes(void *arg)
   smartlist_free(votes);
 }
 
-/* Test utils that don't depend on authority state */
+/* Testing sr_srv_dup(). */
 static void
-test_utils_general(void *arg)
+test_sr_svr_dup(void *arg)
 {
   (void)arg;
 
-  /* Testing sr_srv_dup(). */
-  {
   sr_srv_t *srv = NULL, *dup_srv = NULL;
   const char *srv_value =
     "1BDB7C3E973936E4D13A49F37C859B3DC69C429334CF9412E3FEF6399C52D47A";
@@ -1099,13 +1097,19 @@ test_utils_general(void *arg)
   tt_assert(dup_srv);
   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));
+
+ done:
   tor_free(srv);
   tor_free(dup_srv);
 }
 
 /* Testing commitments_are_the_same(). Currently, the check is to test the
  * value of the encoded commit so let's make sure that actually works. */
+static void
+test_commitments_are_the_same(void *arg)
 {
+  (void)arg;
+
   /* Payload of 57 bytes that is the length of sr_commit_t->encoded_commit.
   * 56 bytes of payload and a NUL terminated byte at the end ('\x00')
   * which comes down to SR_COMMIT_BASE64_LEN + 1. */
@@ -1121,10 +1125,17 @@ test_utils_general(void *arg)
   /* Let's corrupt one of them. */
   memset(commit1.encoded_commit, 'A', sizeof(commit1.encoded_commit));
   tt_int_op(commitments_are_the_same(&commit1, &commit2), OP_EQ, 0);
+
+ done:
+  return;
 }
 
 /* Testing commit_is_authoritative(). */
+static void
+test_commit_is_authoritative(void *arg)
 {
+  (void)arg;
+
   crypto_pk_t *k = crypto_pk_new();
   char digest[DIGEST_LEN];
   sr_commit_t commit;
@@ -1137,14 +1148,18 @@ test_utils_general(void *arg)
   /* Change the pubkey. */
   memset(commit.rsa_identity, 0, sizeof(commit.rsa_identity));
   tt_int_op(commit_is_authoritative(&commit, digest), OP_EQ, 0);
+
+ done:
   crypto_pk_free(k);
 }
 
-  /* Testing get_phase_str(). */
+static void
+test_get_phase_str(void *arg)
 {
+  (void)arg;
+
   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;
@@ -1649,7 +1664,12 @@ struct testcase_t sr_tests[] = {
   { "sr_compute_srv", test_sr_compute_srv, TT_FORK, NULL, NULL },
   { "sr_get_majority_srv_from_votes", test_sr_get_majority_srv_from_votes,
     TT_FORK, NULL, NULL },
-  { "utils_general", test_utils_general, TT_FORK, NULL, NULL },
+  { "sr_svr_dup", test_sr_svr_dup, TT_FORK, NULL, NULL },
+  { "commitments_are_the_same", test_commitments_are_the_same, TT_FORK, NULL,
+    NULL },
+  { "commit_is_authoritative", test_commit_is_authoritative, TT_FORK, NULL,
+    NULL },
+  { "get_phase_str", test_get_phase_str, TT_FORK, NULL, NULL },
   { "utils_auth", test_utils_auth, TT_FORK, NULL, NULL },
   { "state_transition", test_state_transition, TT_FORK, NULL, NULL },
   { "state_update", test_state_update, TT_FORK,