Rewrite free_frame_data_sequence to use recursion instead of manually nested

loops.

This allows us to do the calculations to actually free all the frame data
instances correctly - the previous version was missing many of them,
leading to large memory leaks when given lots of frames that made use of
p_add_proto_data.

Fixes the rest of https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=7885

svn path=/trunk/; revision=48030

1 files changed, 75 insertions, 47 deletions

diff --git a/frame_data_sequence.c b/frame_data_sequence.c
index a323832f84..770d1cea34 100644
--- a/frame_data_sequence.c
+++ b/frame_data_sequence.c
@@ -238,60 +238,88 @@ frame_data_sequence_find(frame_data_sequence *fds, guint32 num)
   return &leaf[LEAF_INDEX(num)];
 }
 
+/* recursively frees a frame_data radix level */
+void
+free_frame_data_array(void *array, guint count, guint level, gboolean last)
+{
+  guint i, level_count;
+
+  if (last) {
+    /* if we are the last in our given parent's row, we may not have
+     * exactly a full row, so do the bit twiddling to figure out exactly
+     * how many fields we have */
+    level_count = (count >> ((level - 1) * LOG2_NODES_PER_LEVEL)) &
+                  (NODES_PER_LEVEL - 1);
+    /* the above calculation rounds down, so make sure we count correctly
+     * if count is not an even multiple of NODES_PER_LEVEL */
+    if (count & ((1 << ((level - 1) * LOG2_NODES_PER_LEVEL)) - 1)) {
+      level_count++;
+    }
+  }
+  else {
+    /* if we're not the last in our parent, then we're guaranteed to have
+     * a full array */
+    level_count = NODES_PER_LEVEL;
+  }
+
+
+  if (level > 1) {
+    /* recurse on every sub-array, passing on our own 'last' value
+     * specially to our last child */
+    frame_data **real_array = (frame_data **) array;
+
+    for (i=0; i < level_count-1; i++) {
+      free_frame_data_array(real_array[i], count, level-1, FALSE);
+    }
+
+    free_frame_data_array(real_array[level_count-1], count, level-1, last);
+  }
+  else if (level == 1) {
+    /* bottom level, so just clean up all the frame data */
+    frame_data *real_array = (frame_data *) array;
+
+    for (i=0; i < level_count; i++) {
+      frame_data_cleanup(&real_array[i]);
+    }
+  }
+
+  /* free the array itself */
+  g_free(array);
+}
+
 /*
  * Free a frame_data_sequence and all the frame_data structures in it.
  */
 void
 free_frame_data_sequence(frame_data_sequence *fds)
 {
-  frame_data **level1;
-  frame_data ***level2;
-  frame_data ****level3;
-  guint i, j, k;
+  guint32 count  = fds->count;
+  guint   levels = 0;
 
-  if (fds->count == 0) {
-    /* Nothing to free. */
-  } else if (fds->count <= NODES_PER_LEVEL) {
-    /* It's a 1-level tree. */
-    g_free(fds->ptree_root);
-  } else if (fds->count <= NODES_PER_LEVEL*NODES_PER_LEVEL) {
-    /* It's a 2-level tree. */
-    level1 = (frame_data **)fds->ptree_root;
-    for (i = 0; i < NODES_PER_LEVEL && level1[i] != NULL; i++) {
-      frame_data_cleanup(level1[i]);
-      g_free(level1[i]);
-    }
-    g_free(level1);
-  } else if (fds->count <= NODES_PER_LEVEL*NODES_PER_LEVEL*NODES_PER_LEVEL) {
-    /* It's a 3-level tree. */
-    level2 = (frame_data ***)fds->ptree_root;
-    for (i = 0; i < NODES_PER_LEVEL && level2[i] != NULL; i++) {
-      level1 = level2[i];
-      for (j = 0; j < NODES_PER_LEVEL && level1[j] != NULL; j++) {
-        frame_data_cleanup(level1[j]);
-        g_free(level1[j]);
-      }
-      g_free(level1);
-    }
-    g_free(level2);
-  } else {
-    /* fds->count is 2^32-1 at most, and NODES_PER_LEVEL^4
-       2^(LOG2_NODES_PER_LEVEL*4), and LOG2_NODES_PER_LEVEL is 10,
-       so fds->count is always less < NODES_PER_LEVEL^4. */
-    /* It's a 4-level tree, and is going to stay that way forever. */
-    level3 = (frame_data ****)fds->ptree_root;
-    for (i = 0; i < NODES_PER_LEVEL && level3[i] != NULL; i++) {
-      level2 = level3[i];
-      for (j = 0; j < NODES_PER_LEVEL && level2[j] != NULL; j++) {
-        level1 = level2[j];
-        for (k = 0; k < NODES_PER_LEVEL && level1[k] != NULL; k++) {
-          frame_data_cleanup(level1[k]);
-          g_free(level1[k]);
-        }
-      }
-      g_free(level2);
-    }
-    g_free(level3);
+  /* calculate how many levels we have */
+  while (count) {
+    levels++;
+    count >>= LOG2_NODES_PER_LEVEL;
   }
+
+  /* call the recursive free function */
+  if (levels > 0) {
+    free_frame_data_array(fds->ptree_root, fds->count, levels, TRUE);
+  }
+
+  /* free the header struct */
   g_free(fds);
 }
+
+/*
+ * Editor modelines  -  http://www.wireshark.org/tools/modelines.html
+ *
+ * Local variables:
+ * c-basic-offset: 2
+ * tab-width: 8
+ * indent-tabs-mode: nil
+ * End:
+ *
+ * vi: set shiftwidth=2 tabstop=8 expandtab:
+ * :indentSize=2:tabSize=8:noTabs=true:
+ */