EDT structures now own their ep_ memory pools. This should finally clean

up the last little bits of:
- https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=5284
- https://www.wireshark.org/lists/wireshark-dev/201208/msg00128.html

and possibly part of:
- https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=7775

This is a fairly invasive change that required some funky work with linked
lists to avoid changing any of the public ep_* APIs, so if something breaks
blame me :)

svn path=/trunk/; revision=45389

diff --git a/epan/emem.c b/epan/emem.c
index 1076f8f842d78bd98a89a280b5561345baf7ce7f..fb38d7553225228132808228b706c65cc4f9a7ee 100644 (file)
--- a/epan/emem.c
+++ b/epan/emem.c
@@ -152,7 +152,12 @@ typedef struct _emem_header_t {
 
 } emem_header_t;
 
-static emem_header_t ep_packet_mem;
+static GSList *ep_pool_stack = NULL;
+/* Some functions use ep_ calls when there isn't actually a packet in scope.
+ * These should perhaps be fixed, but in the meantime, ep_fake_pool is used
+ * to handle those cases. */
+static emem_header_t ep_fake_pool;
+static emem_header_t *ep_packet_mem = &ep_fake_pool;
 static emem_header_t se_packet_mem;
 
 /*
@@ -259,11 +264,11 @@ ep_check_canary_integrity(const char* fmt, ...)
        g_vsnprintf(here, sizeof(here), fmt, ap);
        va_end(ap);
 
-       for (npc = ep_packet_mem.free_list; npc != NULL; npc = npc->next) {
+       for (npc = ep_packet_mem->free_list; npc != NULL; npc = npc->next) {
                void *canary_next = npc->canary_last;
 
                while (canary_next != NULL) {
-                       canary_next = emem_canary_next(ep_packet_mem.canary, canary_next, NULL);
+                       canary_next = emem_canary_next(ep_packet_mem->canary, canary_next, NULL);
                        /* XXX, check if canary_next is inside allocated memory? */
 
                        if (canary_next == (void *) -1)
@@ -293,21 +298,21 @@ emem_init_chunk(emem_header_t *mem)
  * up.
  */
 static void
-ep_init_chunk(void)
+ep_init_chunk(emem_header_t *mem)
 {
-       ep_packet_mem.free_list=NULL;
-       ep_packet_mem.used_list=NULL;
-       ep_packet_mem.trees=NULL;       /* not used by this allocator */
+       mem->free_list=NULL;
+       mem->used_list=NULL;
+       mem->trees=NULL;        /* not used by this allocator */
 
-       ep_packet_mem.debug_use_chunks = (getenv("WIRESHARK_DEBUG_EP_NO_CHUNKS") == NULL);
-       ep_packet_mem.debug_use_canary = ep_packet_mem.debug_use_chunks && (getenv("WIRESHARK_DEBUG_EP_NO_CANARY") == NULL);
-       ep_packet_mem.debug_verify_pointers = (getenv("WIRESHARK_EP_VERIFY_POINTERS") != NULL);
+       mem->debug_use_chunks = (getenv("WIRESHARK_DEBUG_EP_NO_CHUNKS") == NULL);
+       mem->debug_use_canary = mem->debug_use_chunks && (getenv("WIRESHARK_DEBUG_EP_NO_CANARY") == NULL);
+       mem->debug_verify_pointers = (getenv("WIRESHARK_EP_VERIFY_POINTERS") != NULL);
 
 #ifdef DEBUG_INTENSE_CANARY_CHECKS
        intense_canary_checking = (getenv("WIRESHARK_DEBUG_EP_INTENSE_CANARY") != NULL);
 #endif
 
-       emem_init_chunk(&ep_packet_mem);
+       emem_init_chunk(mem);
 }
 
 /* Initialize the capture-lifetime memory allocation pool.
@@ -335,8 +340,8 @@ se_init_chunk(void)
 void
 emem_init(void)
 {
-       ep_init_chunk();
        se_init_chunk();
+       ep_init_chunk(&ep_fake_pool);
 
        if (getenv("WIRESHARK_DEBUG_SCRUB_MEMORY"))
                debug_use_memory_scrubber  = TRUE;
@@ -387,21 +392,21 @@ print_alloc_stats()
 
        fprintf(stderr, "\n-------- EP allocator statistics --------\n");
        fprintf(stderr, "%s chunks, %s canaries, %s memory scrubber\n",
-              ep_packet_mem.debug_use_chunks ? "Using" : "Not using",
-              ep_packet_mem.debug_use_canary ? "using" : "not using",
+              ep_packet_mem->debug_use_chunks ? "Using" : "Not using",
+              ep_packet_mem->debug_use_canary ? "using" : "not using",
               debug_use_memory_scrubber ? "using" : "not using");
 
-       if (! (ep_packet_mem.free_list || !ep_packet_mem.used_list)) {
+       if (! (ep_packet_mem->free_list || !ep_packet_mem->used_list)) {
                fprintf(stderr, "No memory allocated\n");
                ep_stat = FALSE;
        }
-       if (ep_packet_mem.debug_use_chunks && ep_stat) {
+       if (ep_packet_mem->debug_use_chunks && ep_stat) {
                /* Nothing interesting without chunks */
                /*  Only look at the used_list since those chunks are fully
                 *  used.  Looking at the free list would skew our view of what
                 *  we have wasted.
                 */
-               for (chunk = ep_packet_mem.used_list; chunk; chunk = chunk->next) {
+               for (chunk = ep_packet_mem->used_list; chunk; chunk = chunk->next) {
                        num_chunks++;
                        total_used += (chunk->amount_free_init - chunk->amount_free);
                        total_allocation += chunk->amount_free_init;
@@ -555,8 +560,8 @@ emem_verify_pointer(const emem_header_t *hdr, const void *ptr)
 gboolean
 ep_verify_pointer(const void *ptr)
 {
-       if (ep_packet_mem.debug_verify_pointers)
-               return emem_verify_pointer(&ep_packet_mem, ptr);
+       if (ep_packet_mem->debug_verify_pointers)
+               return emem_verify_pointer(ep_packet_mem, ptr);
        else
                return FALSE;
 }
@@ -864,7 +869,7 @@ emem_alloc(size_t size, emem_header_t *mem)
 void *
 ep_alloc(size_t size)
 {
-       return emem_alloc(size, &ep_packet_mem);
+       return emem_alloc(size, ep_packet_mem);
 }
 
 /* allocate 'size' amount of memory with an allocation lifetime until the
@@ -1237,11 +1242,39 @@ emem_free_all(emem_header_t *mem)
        }
 }
 
-/* release all allocated memory back to the pool. */
-void
-ep_free_all(void)
+emem_header_t *ep_create_pool(void)
+{
+       emem_header_t *mem;
+       
+       mem = g_malloc(sizeof(emem_header_t));
+
+       ep_init_chunk(mem);
+
+       if (ep_pool_stack == NULL) {
+               emem_free_all(&ep_fake_pool);
+       }
+
+       ep_pool_stack = g_slist_prepend(ep_pool_stack, mem);
+
+       ep_packet_mem = mem;
+
+       return mem;
+}
+
+void ep_free_pool(emem_header_t *mem)
 {
-       emem_free_all(&ep_packet_mem);
+       ep_pool_stack = g_slist_remove(ep_pool_stack, mem);
+
+       emem_free_all(mem);
+
+       g_free(mem);
+
+       if (ep_pool_stack == NULL) {
+               ep_packet_mem = &ep_fake_pool;
+       }
+       else {
+               ep_packet_mem = ep_pool_stack->data;
+       }
 }
 
 /* release all allocated memory back to the pool. */

diff --git a/epan/emem.h b/epan/emem.h
index e3b5f37c6f1fc878e0f6fae65d65657eee9ffc90..d61e7b968f035b9d087bf2c60f9acce19ff4f2c1 100644 (file)
--- a/epan/emem.h
+++ b/epan/emem.h
@@ -34,6 +34,8 @@
  */
 void emem_init(void);
 
+typedef struct _emem_header_t emem_header_t;
+
 /* Functions for handling memory allocation and garbage collection with
  * a packet lifetime scope.
  * These functions are used to allocate memory that will only remain persistent
@@ -46,6 +48,9 @@ void emem_init(void);
  * the previous packet is freed.
  */
 
+emem_header_t *ep_create_pool(void);
+void ep_free_pool(emem_header_t *ep_packet_mem);
+
 /** Allocate memory with a packet lifetime scope */
 void *ep_alloc(size_t size) G_GNUC_MALLOC;
 #define ep_new(type) ((type*)ep_alloc(sizeof(type)))
@@ -87,9 +92,6 @@ gchar *ep_strconcat(const gchar *string, ...) G_GNUC_MALLOC G_GNUC_NULL_TERMINAT
  */
 gchar** ep_strsplit(const gchar* string, const gchar* delimiter, int max_tokens);
 
-/** release all memory allocated in the previous packet dissection */
-void ep_free_all(void);
-
 
 /** a stack implemented using ephemeral allocators */
 

diff --git a/epan/epan.c b/epan/epan.c
index ffbc53797478d35192e39c72df70dbf0116ee0fe..3bb6eda09517b4bf19dc0766b1de9b62a6c454e3 100644 (file)
--- a/epan/epan.c
+++ b/epan/epan.c
@@ -65,13 +65,6 @@
 #include <ares_version.h>
 #endif
 
-/*
- * Refcount the edt:s and don't free ep memory until refcount = 0
- * See https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=5284
- *
- */
-static guint edt_refs = 0;
-
 const gchar*
 epan_get_version(void) {
        return VERSION;
@@ -171,7 +164,7 @@ epan_dissect_init(epan_dissect_t *edt, const gboolean create_proto_tree, const g
 
        edt->pi.dependent_frames = NULL;
 
-       edt_refs++;
+       edt->mem = ep_create_pool();
 
        return edt;
 }
@@ -217,10 +210,7 @@ epan_dissect_cleanup(epan_dissect_t* edt)
                proto_tree_free(edt->tree);
        }
 
-       edt_refs--;
-
-       if (edt_refs == 0)
-               ep_free_all();
+       ep_free_pool(edt->mem);
 }
 
 void
@@ -362,3 +352,16 @@ _U_
        g_string_append_printf(str, ", Gcrypt %s", gcry_check_version(NULL));
 #endif /* HAVE_LIBGCRYPT */
 }
+
+/*
+ * Editor modelines  -  http://www.wireshark.org/tools/modelines.html
+ *
+ * Local variables:
+ * c-basic-offset: 8
+ * tab-width: 8
+ * indent-tabs-mode: t
+ * End:
+ *
+ * vi: set shiftwidth=8 tabstop=8 noexpandtab:
+ * :indentSize=8:tabSize=8:noTabs=false:
+ */

diff --git a/epan/epan_dissect.h b/epan/epan_dissect.h
index 7faabef9473a51c4ad00066c92d7ae7de2a84b49..8a95bfe8080cd29a1586235385e5beeea8b521aa 100644 (file)
--- a/epan/epan_dissect.h
+++ b/epan/epan_dissect.h
@@ -31,6 +31,7 @@ extern "C" {
 #include "tvbuff.h"
 #include "proto.h"
 #include "packet_info.h"
+#include "emem.h"
 
 /* Dissection of a single byte array. Holds tvbuff info as
  * well as proto_tree info. As long as the epan_dissect_t for a byte
@@ -41,6 +42,7 @@ extern "C" {
 struct _epan_dissect_t {
        tvbuff_t        *tvb;
        proto_tree      *tree;
+       emem_header_t   *mem;
        packet_info     pi;
 };
 
@@ -49,3 +51,16 @@ struct _epan_dissect_t {
 #endif /* __cplusplus */
 
 #endif /* EPAN_DISSECT_H */
+
+/*
+ * Editor modelines  -  http://www.wireshark.org/tools/modelines.html
+ *
+ * Local variables:
+ * c-basic-offset: 8
+ * tab-width: 8
+ * indent-tabs-mode: t
+ * End:
+ *
+ * vi: set shiftwidth=8 tabstop=8 noexpandtab:
+ * :indentSize=8:tabSize=8:noTabs=false:
+ */