Round two of wmem cleanup callbacks. While the emem tree behaviour will require

recurring callbacks, I suspect most other potential uses will be once-only, so
make that possible, and improve the documentation on the remaining issues.

Also separate out the code into its own files and the testing into its own
test case.

svn path=/trunk/; revision=49209

11 files changed, 265 insertions, 102 deletions

diff --git a/doc/README.wmem b/doc/README.wmem
index 67f9967564..7d04c6fffe 100644
--- a/doc/README.wmem
+++ b/doc/README.wmem
@@ -105,33 +105,37 @@ wmem_stack.h
 2.3 Callbacks
 
 WARNING: You probably don't actually need these; use them only when you're
-         sure you understand the implications and the consequences.
+         sure you understand the dangers.
 
 Sometimes (though hopefully rarely) it may be necessary to store data in a wmem
 pool that requires additional cleanup before it is freed. For example, perhaps
 you have a pointer to a file-handle that needs to be closed. In this case, you
 can register a callback with the wmem_register_cleanup_callback function
-declared in wmem_core.h.
-
-This function takes the usual allocator, a function pointer (see wmem_user_cb_t
-also in wmem_core.h) and a void user_data pointer. Every time the memory in a
-pool is freed, all registered cleanup functions are called first, being passed
-a pointer to the allocator as well as whatever user_data was registered with
-that callback.
-
-WARNING: Callback calling order is not defined, you cannot rely on a certain
-         callback being called before or after another (in practice at the
-         moment it's first-in-last-out, but that may change).
-
-WARNING: Callbacks are not cleared when they are called - they are only cleared
-         when the pool is fully destroyed. (Do we need an unregister function?).
-
-WARNING: The user_data pointer is not freed when a callback is cleared, you
-         have to do that yourself (or just allocate it in the appropriate wmem
-         pool).
-
-WARNING: Calling wmem_free on allocated memory that a callback depends on will
-         not unregister that callback. Do not do this, it will crash!
+declared in wmem_user_cb.h. This function takes as parameters:
+ - the allocator
+ - boolean indicating whether or not the callback should be recurring (ie
+   happens once if FALSE, or every time if TRUE)
+ - the callback function (signature defined in wmem_user_cb.h)
+ - a void user_data pointer
+
+Every time the memory in a pool is freed, all registered cleanup functions are
+called first, being passed:
+ - a pointer to the allocator
+ - a boolean indicating if this is just a free_all (FALSE) or if this was
+   triggered by destruction of the entire pool (TRUE) - mostly useful for
+   recurring callbacks
+ - whatever user_data was registered with that callback.
+
+Note that the user_data pointer is not freed when a callback is finished, you
+have to do that yourself in the callback, or just allocate it in the
+appropriate wmem pool.
+
+Also note that callback calling order is not defined, you cannot rely on a
+certain callback being called before or after another.
+
+WARNING: Manually freeing memory with wmem_free will NOT trigger any callbacks.
+         It is an error to call wmem_free on memory if you have a callback
+         registered to deal with the contents of a that memory.
 
 3. Usage for Producers
 
diff --git a/epan/CMakeLists.txt b/epan/CMakeLists.txt
index 57f6677564..c1b1caf61d 100644
--- a/epan/CMakeLists.txt
+++ b/epan/CMakeLists.txt
@@ -1380,6 +1380,7 @@ set(WMEM_FILES
 	wmem/wmem_stack.c
 	wmem/wmem_strbuf.c
 	wmem/wmem_strutl.c
+	wmem/wmem_user_cb.c
 )
 
 ADD_CUSTOM_COMMAND(
diff --git a/epan/wmem/Makefile.common b/epan/wmem/Makefile.common
index 12dbfb2e2a..7ecde9f7af 100644
--- a/epan/wmem/Makefile.common
+++ b/epan/wmem/Makefile.common
@@ -32,7 +32,8 @@ LIBWMEM_SRC =				\
 	wmem_slist.c			\
 	wmem_stack.c			\
 	wmem_strbuf.c			\
-	wmem_strutl.c
+	wmem_strutl.c			\
+	wmem_user_cb.c
 
 LIBWMEM_INCLUDES =			\
 	wmem.h				\
@@ -46,7 +47,8 @@ LIBWMEM_INCLUDES =			\
 	wmem_stack.h			\
 	wmem_strbuf.h			\
 	wmem_strutl.h			\
-	wmem_user_cb.h
+	wmem_user_cb.h			\
+	wmem_user_cb_int.h
 
 
 #
diff --git a/epan/wmem/wmem.h b/epan/wmem/wmem.h
index 253eff8179..28eea6706c 100644
--- a/epan/wmem/wmem.h
+++ b/epan/wmem/wmem.h
@@ -32,6 +32,7 @@
 #include "wmem_stack.h"
 #include "wmem_strbuf.h"
 #include "wmem_strutl.h"
+#include "wmem_user_cb.h"
 
 #endif /* __WMEM_H__ */
 
diff --git a/epan/wmem/wmem_allocator.h b/epan/wmem/wmem_allocator.h
index 296e206de5..f62f71f183 100644
--- a/epan/wmem/wmem_allocator.h
+++ b/epan/wmem/wmem_allocator.h
@@ -34,6 +34,7 @@ extern "C" {
 #endif /* __cplusplus */
 
 enum _wmem_allocator_type_t;
+struct _wmem_user_cb_container_t;
 
 /* See section "4. Internal Design" of doc/README.wmem for details
  * on this structure */
@@ -49,7 +50,7 @@ struct _wmem_allocator_t {
     void  (*destroy)(struct _wmem_allocator_t *allocator);
 
     /* Callback List */
-    GSList                      *callbacks;
+    struct _wmem_user_cb_container_t *callbacks;
 
     /* Implementation details */
     void                        *private_data;
diff --git a/epan/wmem/wmem_core.c b/epan/wmem/wmem_core.c
index 8e0b0b1938..f8b24557e4 100644
--- a/epan/wmem/wmem_core.c
+++ b/epan/wmem/wmem_core.c
@@ -29,7 +29,7 @@
 
 #include "wmem_core.h"
 #include "wmem_scopes.h"
-#include "wmem_user_cb.h"
+#include "wmem_user_cb_int.h"
 #include "wmem_allocator.h"
 #include "wmem_allocator_simple.h"
 #include "wmem_allocator_block.h"
@@ -99,63 +99,30 @@ wmem_realloc(wmem_allocator_t *allocator, void *ptr, const size_t size)
     return allocator->realloc(allocator->private_data, ptr, size);
 }
 
-void
-wmem_free_all(wmem_allocator_t *allocator)
+static void
+wmem_free_all_real(wmem_allocator_t *allocator, gboolean final)
 {
-    GSList *tmp;
-    wmem_user_cb_container_t *cb;
-
-    /* Call all the user-registered callbacks */
-    tmp = allocator->callbacks;
-    while (tmp) {
-        cb = (wmem_user_cb_container_t*) tmp->data;
-        cb->cb(allocator, cb->user_data);
-        tmp = tmp->next;
-    }
-
-    /* Actually free-all */
+    wmem_call_cleanup_callbacks(allocator, final);
     allocator->free_all(allocator->private_data);
 }
 
 void
-wmem_gc(wmem_allocator_t *allocator)
+wmem_free_all(wmem_allocator_t *allocator)
 {
-    allocator->gc(allocator->private_data);
+    wmem_free_all_real(allocator, FALSE);
 }
 
 void
-wmem_register_cleanup_callback(wmem_allocator_t *allocator,
-        wmem_user_cb_t callback, void *user_data)
+wmem_gc(wmem_allocator_t *allocator)
 {
-    wmem_user_cb_container_t *container;
-
-    container = g_slice_new(wmem_user_cb_container_t);
-
-    container->cb        = callback;
-    container->user_data = user_data;
-
-    allocator->callbacks = g_slist_prepend(allocator->callbacks,
-                                           container);
+    allocator->gc(allocator->private_data);
 }
 
 void
 wmem_destroy_allocator(wmem_allocator_t *allocator)
 {
-    GSList *tmp;
-
-    /* Free-all first (this calls all the user-registered callbacks) */
-    wmem_free_all(allocator);
-
-    /* Destroy all user-registered callbacks
-     * (they were called in wmem_free_all) */
-    tmp = allocator->callbacks;
-    while (tmp) {
-        g_slice_free(wmem_user_cb_container_t, tmp->data);
-        tmp = tmp->next;
-    }
-    g_slist_free(allocator->callbacks);
 
-    /* Tell the allocator to destroy itself */
+    wmem_free_all_real(allocator, TRUE);
     allocator->destroy(allocator);
 }
 
diff --git a/epan/wmem/wmem_core.h b/epan/wmem/wmem_core.h
index 83735a7095..238a57c89d 100644
--- a/epan/wmem/wmem_core.h
+++ b/epan/wmem/wmem_core.h
@@ -27,6 +27,7 @@
 #define __WMEM_CORE_H__
 
 #include <string.h>
+#include <glib.h>
 #include <ws_symbol_export.h>
 
 #ifdef __cplusplus
@@ -44,9 +45,6 @@ typedef enum _wmem_allocator_type_t {
     WMEM_ALLOCATOR_STRICT
 } wmem_allocator_type_t;
 
-/* User callback type for registering cleanup routines */
-typedef void (*wmem_user_cb_t) (wmem_allocator_t *, void *);
-
 WS_DLL_PUBLIC
 void *
 wmem_alloc(wmem_allocator_t *allocator, const size_t size)
@@ -82,11 +80,6 @@ wmem_gc(wmem_allocator_t *allocator);
 
 WS_DLL_PUBLIC
 void
-wmem_register_cleanup_callback(wmem_allocator_t *allocator,
-        wmem_user_cb_t callback, void *user_data);
-
-WS_DLL_PUBLIC
-void
 wmem_destroy_allocator(wmem_allocator_t *allocator);
 
 WS_DLL_PUBLIC
diff --git a/epan/wmem/wmem_test.c b/epan/wmem/wmem_test.c
index f1b502f55c..228d83a884 100644
--- a/epan/wmem/wmem_test.c
+++ b/epan/wmem/wmem_test.c
@@ -83,23 +83,78 @@ wmem_allocator_force_new(const wmem_allocator_type_t type)
 }
 
 /* Some helpers for properly testing the user callback functionality */
-wmem_allocator_t *cur_global_allocator;
-void             *cur_global_user_data;
-gboolean          cb_called;
+wmem_allocator_t *expected_allocator;
+void             *expected_user_data;
+gboolean          expected_final;
+int               cb_called_count;
 
 static void
-wmem_test_callback(wmem_allocator_t *allocator, void *user_data)
+wmem_test_cb(wmem_allocator_t *allocator, gboolean final, void *user_data)
 {
-    g_assert(allocator == cur_global_allocator);
-    g_assert(user_data == cur_global_user_data);
-    g_assert(!cb_called);
+    g_assert(allocator == expected_allocator);
+    g_assert(final     == expected_final);
+    g_assert(user_data == expected_user_data);
 
-    cb_called = TRUE;
+    cb_called_count++;
 }
 
 /* ALLOCATOR TESTING FUNCTIONS (/wmem/allocator/) */
 
 static void
+wmem_test_allocator_callbacks(void)
+{
+    wmem_allocator_t *allocator;
+
+    allocator = wmem_allocator_force_new(WMEM_ALLOCATOR_STRICT);
+
+    expected_allocator = allocator;
+    expected_user_data = GINT_TO_POINTER(42);
+
+#define REG_TEST_CB(RECUR)  do { \
+    wmem_register_cleanup_callback(expected_allocator, (RECUR), \
+            &wmem_test_cb, GINT_TO_POINTER(42)); \
+    } while (0);
+
+    REG_TEST_CB(FALSE);
+    REG_TEST_CB(TRUE);
+    REG_TEST_CB(FALSE);
+    REG_TEST_CB(FALSE);
+    REG_TEST_CB(TRUE);
+
+    expected_final     = FALSE;
+
+    cb_called_count = 0;
+    wmem_free_all(allocator);
+    g_assert(cb_called_count == 5);
+
+    cb_called_count = 0;
+    wmem_free_all(allocator);
+    g_assert(cb_called_count == 2);
+
+    cb_called_count = 0;
+    wmem_free_all(allocator);
+    g_assert(cb_called_count == 2);
+
+    REG_TEST_CB(TRUE);
+    REG_TEST_CB(FALSE);
+
+    cb_called_count = 0;
+    wmem_free_all(allocator);
+    g_assert(cb_called_count == 4);
+
+    cb_called_count = 0;
+    wmem_free_all(allocator);
+    g_assert(cb_called_count == 3);
+
+    REG_TEST_CB(FALSE);
+
+    expected_final = TRUE;
+    cb_called_count = 0;
+    wmem_destroy_allocator(allocator);
+    g_assert(cb_called_count == 4);
+}
+
+static void
 wmem_test_allocator(wmem_allocator_type_t type, wmem_verify_func verify)
 {
     int i;
@@ -123,15 +178,8 @@ wmem_test_allocator(wmem_allocator_type_t type, wmem_verify_func verify)
         wmem_free(allocator, ptrs[i]);
     }
 
-    wmem_register_cleanup_callback(allocator, &wmem_test_callback,
-            GINT_TO_POINTER(42));
-    cur_global_allocator = allocator;
-    cur_global_user_data = GINT_TO_POINTER(42);
-
     if (verify) (*verify)(allocator);
-    cb_called = FALSE;
     wmem_free_all(allocator);
-    g_assert(cb_called);
     wmem_gc(allocator);
     if (verify) (*verify)(allocator);
 
@@ -143,9 +191,7 @@ wmem_test_allocator(wmem_allocator_type_t type, wmem_verify_func verify)
     }
 
     if (verify) (*verify)(allocator);
-    cb_called = FALSE;
     wmem_free_all(allocator);
-    g_assert(cb_called);
     wmem_gc(allocator);
     if (verify) (*verify)(allocator);
 
@@ -162,9 +208,7 @@ wmem_test_allocator(wmem_allocator_type_t type, wmem_verify_func verify)
     }
 
     if (verify) (*verify)(allocator);
-    cb_called = FALSE;
     wmem_free_all(allocator);
-    g_assert(cb_called);
     wmem_gc(allocator);
     if (verify) (*verify)(allocator);
 
@@ -213,9 +257,7 @@ wmem_test_allocator(wmem_allocator_type_t type, wmem_verify_func verify)
         if (verify) (*verify)(allocator);
     }
 
-    cb_called = FALSE;
     wmem_destroy_allocator(allocator);
-    g_assert(cb_called);
 }
 
 static void
@@ -503,6 +545,7 @@ main(int argc, char **argv)
     g_test_add_func("/wmem/allocator/simple", wmem_test_allocator_simple);
     g_test_add_func("/wmem/allocator/strict", wmem_test_allocator_strict);
     g_test_add_func("/wmem/allocator/times",  wmem_time_allocators);
+    g_test_add_func("/wmem/allocator/cbs",    wmem_test_allocator_callbacks);
 
     g_test_add_func("/wmem/utils/strings", wmem_test_strutls);
 
diff --git a/epan/wmem/wmem_user_cb.c b/epan/wmem/wmem_user_cb.c
new file mode 100644
index 0000000000..3eda0790a3
--- /dev/null
+++ b/epan/wmem/wmem_user_cb.c
@@ -0,0 +1,93 @@
+/* wmem_user_cb.c
+ * Wireshark Memory Manager User Callbacks
+ * Copyright 2012, Evan Huus <eapache@gmail.com>
+ *
+ * $Id$
+ *
+ * Wireshark - Network traffic analyzer
+ * By Gerald Combs <gerald@wireshark.org>
+ * Copyright 1998 Gerald Combs
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+#include "wmem_core.h"
+#include "wmem_allocator.h"
+
+#include "wmem_user_cb.h"
+#include "wmem_user_cb_int.h"
+
+typedef struct _wmem_user_cb_container_t {
+    wmem_user_cb_t                    cb;
+    void                             *user_data;
+    struct _wmem_user_cb_container_t *next;
+    gboolean                          recurring;
+} wmem_user_cb_container_t;
+
+void
+wmem_call_cleanup_callbacks(wmem_allocator_t *allocator, gboolean final)
+{
+    wmem_user_cb_container_t **prev, *cur;
+
+    prev = &(allocator->callbacks);
+    cur  = allocator->callbacks;
+
+    while (cur) {
+
+        /* call it */
+        cur->cb(allocator, final, cur->user_data);
+
+        /* if it was a one-time callback, or this is being triggered by
+         * the final destruction of the allocator, remove the callback */
+        if (! cur->recurring || final) {
+            *prev = cur->next;
+            g_slice_free(wmem_user_cb_container_t, cur);
+            cur = *prev;
+        }
+        else {
+            prev = &(cur->next);
+            cur  = cur->next;
+        }
+    }
+}
+
+void
+wmem_register_cleanup_callback(wmem_allocator_t *allocator, gboolean recurring,
+        wmem_user_cb_t callback, void *user_data)
+{
+    wmem_user_cb_container_t *container;
+
+    container = g_slice_new(wmem_user_cb_container_t);
+
+    container->cb        = callback;
+    container->user_data = user_data;
+    container->recurring = recurring;
+    container->next      = allocator->callbacks;
+
+    allocator->callbacks = container;
+}
+
+/*
+ * Editor modelines  -  http://www.wireshark.org/tools/modelines.html
+ *
+ * Local variables:
+ * c-basic-offset: 4
+ * tab-width: 8
+ * indent-tabs-mode: nil
+ * End:
+ *
+ * vi: set shiftwidth=4 tabstop=8 expandtab:
+ * :indentSize=4:tabSize=8:noTabs=true:
+ */
diff --git a/epan/wmem/wmem_user_cb.h b/epan/wmem/wmem_user_cb.h
index 8a5b093eb1..5a2ddbee71 100644
--- a/epan/wmem/wmem_user_cb.h
+++ b/epan/wmem/wmem_user_cb.h
@@ -26,16 +26,19 @@
 #ifndef __WMEM_USER_CB_H__
 #define __WMEM_USER_CB_H__
 
-#include "wmem_core.h"
-
 #ifdef __cplusplus
 extern "C" {
 #endif /* __cplusplus */
 
-typedef struct _wmem_user_cb_container_t {
-    wmem_user_cb_t  cb;
-    void           *user_data;
-} wmem_user_cb_container_t;
+#include <glib.h>
+
+/* User callback type for registering cleanup routines */
+typedef void (*wmem_user_cb_t) (wmem_allocator_t *, gboolean, void *);
+
+WS_DLL_PUBLIC
+void
+wmem_register_cleanup_callback(wmem_allocator_t *allocator, gboolean recurring,
+        wmem_user_cb_t callback, void *user_data);
 
 #ifdef __cplusplus
 }
diff --git a/epan/wmem/wmem_user_cb_int.h b/epan/wmem/wmem_user_cb_int.h
new file mode 100644
index 0000000000..a8bae60666
--- /dev/null
+++ b/epan/wmem/wmem_user_cb_int.h
@@ -0,0 +1,55 @@
+/* wmem_user_cb_int.h
+ * Definitions for the Wireshark Memory Manager User Callback Internals
+ * Copyright 2012, Evan Huus <eapache@gmail.com>
+ *
+ * $Id$
+ *
+ * Wireshark - Network traffic analyzer
+ * By Gerald Combs <gerald@wireshark.org>
+ * Copyright 1998 Gerald Combs
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+#ifndef __WMEM_USER_CB_INT_H__
+#define __WMEM_USER_CB_INT_H__
+
+#ifdef __cplusplus
+extern "C" {
+#endif /* __cplusplus */
+
+#include <glib.h>
+
+void
+wmem_call_cleanup_callbacks(wmem_allocator_t *allocator, gboolean final);
+
+#ifdef __cplusplus
+}
+#endif /* __cplusplus */
+
+#endif /* __WMEM_USER_CB_INT_H__ */
+
+/*
+ * Editor modelines  -  http://www.wireshark.org/tools/modelines.html
+ *
+ * Local variables:
+ * c-basic-offset: 4
+ * tab-width: 8
+ * indent-tabs-mode: nil
+ * End:
+ *
+ * vi: set shiftwidth=4 tabstop=8 expandtab:
+ * :indentSize=4:tabSize=8:noTabs=true:
+ */