[tor/master] Fix a bunch of memory leaks in the unit tests. Found with valgrind - tor-commits

22 Dec 2014

commit 03d2df62f614f97d2b5cf52518565ce91333ba87
Author: Nick Mathewson nickm@torproject.org
Date:   Mon Dec 22 12:27:26 2014 -0500
Fix a bunch of memory leaks in the unit tests. Found with valgrind
---
 src/or/channel.c           |    3 +--
 src/or/channel.h           |    2 ++
 src/or/transports.c        |    4 +---
 src/or/transports.h        |    2 ++
 src/test/fakechans.h       |    1 +
 src/test/test_channel.c    |   45 ++++++++++++++++++++++++++++++++------------
 src/test/test_channeltls.c |    7 ++++---
 src/test/test_config.c     |    3 +++
 src/test/test_dir.c        |   13 +++++++++----
 src/test/test_relay.c      |    9 ++++-----
 10 files changed, 60 insertions(+), 29 deletions(-)

diff --git a/src/or/channel.c b/src/or/channel.c
index 4826bdd..cc609b5 100644
--- a/src/or/channel.c
+++ b/src/or/channel.c
@@ -147,7 +147,6 @@ HT_GENERATE2(channel_idmap, channel_idmap_entry_s, node, channel_idmap_hash,
              channel_idmap_eq, 0.5,  tor_reallocarray_, tor_free_);
static cell_queue_entry_t * cell_queue_entry_dup(cell_queue_entry_t *q);
-static void cell_queue_entry_free(cell_queue_entry_t *q, int handed_off);
 #if 0
 static int cell_queue_entry_is_padding(cell_queue_entry_t *q);
 #endif
@@ -1569,7 +1568,7 @@ cell_queue_entry_dup(cell_queue_entry_t *q)
  * them) or not (we should free).
  */
-static void
+STATIC void
 cell_queue_entry_free(cell_queue_entry_t *q, int handed_off)
 {
   if (!q) return;
diff --git a/src/or/channel.h b/src/or/channel.h
index ec7a15f..c4b909c 100644
--- a/src/or/channel.h
+++ b/src/or/channel.h
@@ -380,6 +380,8 @@ struct cell_queue_entry_s {
/* Cell queue functions for benefit of test suite */
 STATIC int chan_cell_queue_len(const chan_cell_queue_t *queue);
+
+STATIC void cell_queue_entry_free(cell_queue_entry_t *q, int handed_off);
 #endif
/* Channel operations for subclasses and internal use only */
diff --git a/src/or/transports.c b/src/or/transports.c
index 2623f80..7999be3 100644
--- a/src/or/transports.c
+++ b/src/or/transports.c
@@ -112,8 +112,6 @@ static void parse_method_error(const char *line, int is_server_method);
 #define parse_server_method_error(l) parse_method_error(l, 1)
 #define parse_client_method_error(l) parse_method_error(l, 0)
-static INLINE void free_execve_args(char **arg);
-
 /** Managed proxy protocol strings */
 #define PROTO_ENV_ERROR "ENV-ERROR"
 #define PROTO_NEG_SUCCESS "VERSION"
@@ -1502,7 +1500,7 @@ pt_kickstart_proxy, (const smartlist_t *transport_list,
/** Frees the array of pointers in <b>arg</b> used as arguments to
     execve(2). */
-static INLINE void
+STATIC void
 free_execve_args(char **arg)
 {
   char **tmp = arg;
diff --git a/src/or/transports.h b/src/or/transports.h
index 2958d5e..8f60760 100644
--- a/src/or/transports.h
+++ b/src/or/transports.h
@@ -131,6 +131,8 @@ STATIC int configure_proxy(managed_proxy_t *mp);
STATIC char* get_pt_proxy_uri(void);
+STATIC void free_execve_args(char **arg);
+
 #endif
#endif
diff --git a/src/test/fakechans.h b/src/test/fakechans.h
index b129ab4..230abe4 100644
--- a/src/test/fakechans.h
+++ b/src/test/fakechans.h
@@ -12,6 +12,7 @@
 void make_fake_cell(cell_t *c);
 void make_fake_var_cell(var_cell_t *c);
 channel_t * new_fake_channel(void);
+void free_fake_channel(channel_t *c);
/* Also exposes some a mock used by both test_channel.c and test_relay.c */
 void scheduler_channel_has_waiting_cells_mock(channel_t *ch);
diff --git a/src/test/test_channel.c b/src/test/test_channel.c
index b1f1bdb..0766415 100644
--- a/src/test/test_channel.c
+++ b/src/test/test_channel.c
@@ -430,6 +430,27 @@ new_fake_channel(void)
   return chan;
 }
+void
+free_fake_channel(channel_t *chan)
+{
+  cell_queue_entry_t *cell, *cell_tmp;
+
+  if (! chan)
+    return;
+
+  if (chan->cmux)
+    circuitmux_free(chan->cmux);
+
+  TOR_SIMPLEQ_FOREACH_SAFE(cell, &chan->incoming_queue, next, cell_tmp) {
+      cell_queue_entry_free(cell, 0);
+  }
+  TOR_SIMPLEQ_FOREACH_SAFE(cell, &chan->outgoing_queue, next, cell_tmp) {
+      cell_queue_entry_free(cell, 0);
+  }
+
+  tor_free(chan);
+}
+
 /**
  * Counter query for scheduler_channel_has_waiting_cells_mock()
  */
@@ -610,7 +631,7 @@ test_channel_dumpstats(void *arg)
done:
   tor_free(cell);
-  tor_free(ch);
+  free_fake_channel(ch);
UNMOCK(scheduler_channel_doesnt_want_writes);
   UNMOCK(scheduler_release_channel);
@@ -748,6 +769,8 @@ test_channel_flushmux(void *arg)
   test_cmux_cells = 0;
done:
+  if (ch)
+    circuitmux_free(ch->cmux);
   tor_free(ch);
UNMOCK(channel_flush_from_first_active_circuit);
@@ -831,7 +854,7 @@ test_channel_incoming(void *arg)
   ch = NULL;
done:
-  tor_free(ch);
+  free_fake_channel(ch);
   tor_free(cell);
   tor_free(var_cell);
@@ -944,8 +967,8 @@ test_channel_lifecycle(void *arg)
   tt_int_op(test_releases_count, ==, init_releases_count + 4);
done:
-  tor_free(ch1);
-  tor_free(ch2);
+  free_fake_channel(ch1);
+  free_fake_channel(ch2);
UNMOCK(scheduler_channel_doesnt_want_writes);
   UNMOCK(scheduler_release_channel);
@@ -1214,6 +1237,7 @@ test_channel_multi(void *arg)
   cell = tor_malloc_zero(sizeof(cell_t));
   make_fake_cell(cell);
   channel_write_cell(ch2, cell);
+  cell = NULL;
/* Check the estimates */
   channel_update_xmit_queue_size(ch1);
@@ -1248,8 +1272,8 @@ test_channel_multi(void *arg)
   UNMOCK(scheduler_release_channel);
done:
-  tor_free(ch1);
-  tor_free(ch2);
+  free_fake_channel(ch1);
+  free_fake_channel(ch2);
return;
 }
@@ -1279,9 +1303,6 @@ test_channel_queue_impossible(void *arg)
   init_cell_pool();
 #endif /* ENABLE_MEMPOOLS */
-  packed_cell = packed_cell_new();
-  tt_assert(packed_cell);
-
   ch = new_fake_channel();
   tt_assert(ch);
@@ -1403,7 +1424,7 @@ test_channel_queue_impossible(void *arg)
   tt_int_op(chan_cell_queue_len(&(ch->outgoing_queue)), ==, 0);
done:
-  tor_free(ch);
+  free_fake_channel(ch);
 #ifdef ENABLE_MEMPOOLS
   free_cell_pool();
 #endif /* ENABLE_MEMPOOLS */
@@ -1532,7 +1553,7 @@ test_channel_queue_size(void *arg)
   UNMOCK(scheduler_release_channel);
done:
-  tor_free(ch);
+  free_fake_channel(ch);
return;
 }
@@ -1654,7 +1675,7 @@ test_channel_write(void *arg)
 #endif /* ENABLE_MEMPOOLS */
done:
-  tor_free(ch);
+  free_fake_channel(ch);
   tor_free(var_cell);
   tor_free(cell);
   packed_cell_free(packed_cell);
diff --git a/src/test/test_channeltls.c b/src/test/test_channeltls.c
index 45e24df..89c75d8 100644
--- a/src/test/test_channeltls.c
+++ b/src/test/test_channeltls.c
@@ -80,7 +80,7 @@ test_channeltls_create(void *arg)
      */
     ch->close = tlschan_fake_close_method;
     channel_mark_for_close(ch);
-    tor_free(ch);
+    free_fake_channel(ch);
     UNMOCK(scheduler_release_channel);
   }
@@ -167,7 +167,7 @@ test_channeltls_num_bytes_queued(void *arg)
      */
     ch->close = tlschan_fake_close_method;
     channel_mark_for_close(ch);
-    tor_free(ch);
+    free_fake_channel(ch);
     UNMOCK(scheduler_release_channel);
   }
@@ -241,7 +241,7 @@ test_channeltls_overhead_estimate(void *arg)
      */
     ch->close = tlschan_fake_close_method;
     channel_mark_for_close(ch);
-    tor_free(ch);
+    free_fake_channel(ch);
     UNMOCK(scheduler_release_channel);
   }
@@ -304,6 +304,7 @@ tlschan_fake_close_method(channel_t *chan)
   tt_assert(tlschan != NULL);
/* Just free the fake orconn */
+  tor_free(tlschan->conn->base_.address);
   tor_free(tlschan->conn);
channel_closed(chan);
diff --git a/src/test/test_config.c b/src/test/test_config.c
index dcb4e92..ea0c397 100644
--- a/src/test/test_config.c
+++ b/src/test/test_config.c
@@ -6,6 +6,7 @@
 #include "orconfig.h"
#define CONFIG_PRIVATE
+#define PT_PRIVATE
 #include "or.h"
 #include "addressmap.h"
 #include "config.h"
@@ -578,6 +579,8 @@ pt_kickstart_proxy_mock(const smartlist_t *transport_list,
   /* XXXX check that args are as expected. */
++pt_kickstart_proxy_mock_call_count;
+
+  free_execve_args(proxy_argv);
 }
static int
diff --git a/src/test/test_dir.c b/src/test/test_dir.c
index 2659fbc..4cd8aa8 100644
--- a/src/test/test_dir.c
+++ b/src/test/test_dir.c
@@ -388,6 +388,12 @@ test_dir_routerparse_bad(void *arg)
 #include "example_extrainfo.inc"
static void
+routerinfo_free_wrapper_(void *arg)
+{
+  routerinfo_free(arg);
+}
+
+static void
 test_dir_extrainfo_parsing(void *arg)
 {
   (void) arg;
@@ -455,9 +461,9 @@ test_dir_extrainfo_parsing(void *arg)
 #undef CHECK_FAIL
done:
+  extrainfo_free(ei);
   routerinfo_free(ri);
-  /* XXXX elements should get freed too */
-  digestmap_free((digestmap_t*)map, NULL);
+  digestmap_free((digestmap_t*)map, routerinfo_free_wrapper_);
 }
static void
@@ -552,9 +558,8 @@ test_dir_parse_router_list(void *arg)
   SMARTLIST_FOREACH(chunks, char *, cp, tor_free(cp));
   smartlist_free(chunks);
   routerinfo_free(ri);
-  /* XXXX this leaks: */
   if (map) {
-    digestmap_free((digestmap_t*)map, NULL);
+    digestmap_free((digestmap_t*)map, routerinfo_free_wrapper_);
     router_get_routerlist()->identity_map =
       (struct digest_ri_map_t*)digestmap_new();
   }
diff --git a/src/test/test_relay.c b/src/test/test_relay.c
index 38d1d96..fbe7faf 100644
--- a/src/test/test_relay.c
+++ b/src/test/test_relay.c
@@ -111,15 +111,14 @@ test_relay_append_cell_to_circuit_queue(void *arg)
/* Shut down channels */
   channel_free_all();
-  nchan = pchan = NULL;
done:
   tor_free(cell);
+  cell_queue_clear(&orcirc->base_.n_chan_cells);
+  cell_queue_clear(&orcirc->p_chan_cells);
   tor_free(orcirc);
-  if (nchan && nchan->cmux) circuitmux_free(nchan->cmux);
-  tor_free(nchan);
-  if (pchan && pchan->cmux) circuitmux_free(pchan->cmux);
-  tor_free(pchan);
+  free_fake_channel(nchan);
+  free_fake_channel(pchan);
 #ifdef ENABLE_MEMPOOLS
   free_cell_pool();
 #endif /* ENABLE_MEMPOOLS */

    