[tor-commits] [tor/master] Assert that memory held by rephist is freed

[nickm at torproject.org]

commit 91ab2ac5aa3a94ebe3d53e9dc2b5cb0ee2b59b3c
Author: cypherpunks <cypherpunks at torproject.org>
Date:   Wed Dec 2 10:11:32 2015 +0100

    Assert that memory held by rephist is freed
    
    The internal memory allocation and history object counters of the
    reputation code can be used to verify the correctness of (part of) the
    code. Using these counters revealed an issue where the memory allocation
    counter is not decreased when the bandwidth arrays are freed.
    
    A new function ensures the memory allocation counter is decreased when a
    bandwidth array is freed.
    
    This commit also removes an unnecessary cast which was found while
    working on the code.
---
 changes/bug17753 |    4 ++++
 src/or/rephist.c |   43 ++++++++++++++++++++++++++++++++++---------
 2 files changed, 38 insertions(+), 9 deletions(-)

diff --git a/changes/bug17753 b/changes/bug17753
new file mode 100644
index 0000000..7d227d8
--- /dev/null
+++ b/changes/bug17753
@@ -0,0 +1,4 @@
+  o Minor bugfixes (code correctness)
+    - Assert that allocated memory held by the reputation code is freed
+      according to its internal counters. Fixes bug 17753; bugfix on
+      tor-0.1.1.1-alpha.
diff --git a/src/or/rephist.c b/src/or/rephist.c
index 4f7aaae..343a066 100644
--- a/src/or/rephist.c
+++ b/src/or/rephist.c
@@ -148,7 +148,7 @@ get_link_history(const char *from_id, const char *to_id)
     return NULL;
   if (tor_digest_is_zero(to_id))
     return NULL;
-  lhist = (link_history_t*) digestmap_get(orhist->link_history_map, to_id);
+  lhist = digestmap_get(orhist->link_history_map, to_id);
   if (!lhist) {
     lhist = tor_malloc_zero(sizeof(link_history_t));
     rephist_total_alloc += sizeof(link_history_t);
@@ -1250,6 +1250,18 @@ bw_array_new(void)
   return b;
 }
 
+/** Free storage held by bandwidth array <b>b</b>. */
+static void
+bw_array_free(bw_array_t *b)
+{
+  if (!b) {
+    return;
+  }
+
+  rephist_total_alloc -= sizeof(bw_array_t);
+  tor_free(b);
+}
+
 /** Recent history of bandwidth observations for read operations. */
 static bw_array_t *read_array = NULL;
 /** Recent history of bandwidth observations for write operations. */
@@ -1266,10 +1278,11 @@ static bw_array_t *dir_write_array = NULL;
 static void
 bw_arrays_init(void)
 {
-  tor_free(read_array);
-  tor_free(write_array);
-  tor_free(dir_read_array);
-  tor_free(dir_write_array);
+  bw_array_free(read_array);
+  bw_array_free(write_array);
+  bw_array_free(dir_read_array);
+  bw_array_free(dir_write_array);
+
   read_array = bw_array_new();
   write_array = bw_array_new();
   dir_read_array = bw_array_new();
@@ -3172,10 +3185,19 @@ rep_hist_free_all(void)
 {
   hs_stats_free(hs_stats);
   digestmap_free(history_map, free_or_history);
-  tor_free(read_array);
-  tor_free(write_array);
-  tor_free(dir_read_array);
-  tor_free(dir_write_array);
+
+  bw_array_free(read_array);
+  read_array = NULL;
+
+  bw_array_free(write_array);
+  write_array = NULL;
+
+  bw_array_free(dir_read_array);
+  dir_read_array = NULL;
+
+  bw_array_free(dir_write_array);
+  dir_write_array = NULL;
+
   tor_free(exit_bytes_read);
   tor_free(exit_bytes_written);
   tor_free(exit_streams);
@@ -3190,5 +3212,8 @@ rep_hist_free_all(void)
   }
   rep_hist_desc_stats_term();
   total_descriptor_downloads = 0;
+
+  tor_assert(rephist_total_alloc == 0);
+  tor_assert(rephist_total_num == 0);
 }