Fixup stat_tap table freeing.

As it turns out we *do* need to free table elements. We also need to
free the tables themselves and clear the table array. Do so.

Change-Id: Ic1c81388eac8f47f74caea0169c79685a83aaff9
Reviewed-on: https://code.wireshark.org/review/9901
Petri-Dish: Gerald Combs <gerald@wireshark.org>
Reviewed-by: Gerald Combs <gerald@wireshark.org>

diff --git a/asn1/camel/packet-camel-template.c b/asn1/camel/packet-camel-template.c
index 152aabad7c3ad137d25f4d92b131cd4bc52fcb2e..1d77cb779a0ca52c0c5b0f6f8e7062e598760342 100644 (file)
--- a/asn1/camel/packet-camel-template.c
+++ b/asn1/camel/packet-camel-template.c
@@ -1285,6 +1285,12 @@ camel_stat_reset(new_stat_tap_table* table)
   }
 }
 
+static void
+camel_stat_free_table_item(new_stat_tap_table* table _U_, guint row _U_, guint column, stat_tap_table_item_type* field_data)
+{
+  if (column != MESSAGE_TYPE_COLUMN) return;
+  g_free((char*)field_data->value.string_value);
+}
 
 /*--- proto_reg_handoff_camel ---------------------------------------*/
 static void range_delete_callback(guint32 ssn)
@@ -1537,7 +1543,7 @@ void proto_register_camel(void) {
     camel_stat_init,
     camel_stat_packet,
     camel_stat_reset,
-    NULL,
+    camel_stat_free_table_item,
     NULL,
     sizeof(camel_stat_fields)/sizeof(stat_tap_table_item), camel_stat_fields,
     sizeof(camel_stat_params)/sizeof(tap_param), camel_stat_params,

diff --git a/asn1/gsm_map/packet-gsm_map-template.c b/asn1/gsm_map/packet-gsm_map-template.c
index 54ba888c479957f8e2f4d7c62c90405b1cade43c..300d685dc0ecb224cdd113995d70ad922a737c86 100644 (file)
--- a/asn1/gsm_map/packet-gsm_map-template.c
+++ b/asn1/gsm_map/packet-gsm_map-template.c
@@ -2512,10 +2512,15 @@ void gsm_map_stat_init(new_stat_tap_ui* new_stat, new_stat_tap_gui_init_cb gui_c
   for (i = 0; i < GSM_MAP_MAX_NUM_OPR_CODES; i++)
   {
     const char *ocs = try_val_to_str(i, gsm_map_opr_code_strings);
-    if (!ocs) ocs = g_strdup_printf("Unknown op code %d", i);
+    char *col_str;
+    if (ocs) {
+      col_str = g_strdup(ocs);
+    } else {
+      col_str = g_strdup_printf("Unknown op code %d", i);
+    }
 
     items[ID_COLUMN].value.uint_value = i;
-    items[OP_CODE_COLUMN].value.string_value = ocs;
+    items[OP_CODE_COLUMN].value.string_value = col_str;
     new_stat_tap_init_table_row(table, i, num_fields, items);
   }
 }
@@ -2592,6 +2597,13 @@ gsm_map_stat_reset(new_stat_tap_table* table)
   }
 }
 
+static void
+gsm_map_stat_free_table_item(new_stat_tap_table* table _U_, guint row _U_, guint column, stat_tap_table_item_type* field_data)
+{
+  if (column != OP_CODE_COLUMN) return;
+  g_free((char*)field_data->value.string_value);
+}
+
 /*--- proto_reg_handoff_gsm_map ---------------------------------------*/
 static void range_delete_callback(guint32 ssn)
 {
@@ -3100,7 +3112,7 @@ void proto_register_gsm_map(void) {
     gsm_map_stat_init,
     gsm_map_stat_packet,
     gsm_map_stat_reset,
-    NULL,
+    gsm_map_stat_free_table_item,
     NULL,
     sizeof(gsm_map_stat_fields)/sizeof(stat_tap_table_item), gsm_map_stat_fields,
     sizeof(gsm_map_stat_params)/sizeof(tap_param), gsm_map_stat_params,

diff --git a/debian/libwireshark0.symbols b/debian/libwireshark0.symbols
index 65a0288ecaf8119cc410f75f391259402043be7b..0d95c9a4e4d37ae2a5af4e99f8f228d9aca16283 100644 (file)
--- a/debian/libwireshark0.symbols
+++ b/debian/libwireshark0.symbols
@@ -572,7 +572,7 @@ libwireshark.so.0 libwireshark0 #MINVER#
  free_rtd_table@Base 1.99.8
  free_srt_table@Base 1.99.8
  free_srt_table_data@Base 1.99.8
- free_stat_table@Base 1.99.8
+ free_stat_tables@Base 1.99.9
  ftype_can_contains@Base 1.9.1
  ftype_can_eq@Base 1.9.1
  ftype_can_ge@Base 1.9.1

diff --git a/epan/dissectors/packet-camel.c b/epan/dissectors/packet-camel.c
index 1f3000429e08a8791e75217531c22a0d1eb7de4e..356dacee1774ff25982007aade4ea353d5776a8c 100644 (file)
--- a/epan/dissectors/packet-camel.c
+++ b/epan/dissectors/packet-camel.c
@@ -8191,6 +8191,12 @@ camel_stat_reset(new_stat_tap_table* table)
   }
 }
 
+static void
+camel_stat_free_table_item(new_stat_tap_table* table _U_, guint row _U_, guint column, stat_tap_table_item_type* field_data)
+{
+  if (column != MESSAGE_TYPE_COLUMN) return;
+  g_free((char*)field_data->value.string_value);
+}
 
 /*--- proto_reg_handoff_camel ---------------------------------------*/
 static void range_delete_callback(guint32 ssn)
@@ -8238,7 +8244,7 @@ void proto_reg_handoff_camel(void) {
 
 
 /*--- End of included file: packet-camel-dis-tab.c ---*/
-#line 1328 "../../asn1/camel/packet-camel-template.c"
+#line 1334 "../../asn1/camel/packet-camel-template.c"
   } else {
     range_foreach(ssn_range, range_delete_callback);
     g_free(ssn_range);
@@ -10352,7 +10358,7 @@ void proto_register_camel(void) {
         "InvokeId_present", HFILL }},
 
 /*--- End of included file: packet-camel-hfarr.c ---*/
-#line 1501 "../../asn1/camel/packet-camel-template.c"
+#line 1507 "../../asn1/camel/packet-camel-template.c"
   };
 
   /* List of subtrees */
@@ -10568,7 +10574,7 @@ void proto_register_camel(void) {
     &ett_camel_InvokeId,
 
 /*--- End of included file: packet-camel-ettarr.c ---*/
-#line 1518 "../../asn1/camel/packet-camel-template.c"
+#line 1524 "../../asn1/camel/packet-camel-template.c"
   };
 
   static ei_register_info ei[] = {
@@ -10591,7 +10597,7 @@ void proto_register_camel(void) {
     camel_stat_init,
     camel_stat_packet,
     camel_stat_reset,
-    NULL,
+    camel_stat_free_table_item,
     NULL,
     sizeof(camel_stat_fields)/sizeof(stat_tap_table_item), camel_stat_fields,
     sizeof(camel_stat_params)/sizeof(tap_param), camel_stat_params,

diff --git a/epan/dissectors/packet-gsm_a_common.c b/epan/dissectors/packet-gsm_a_common.c
index c2b0c6c6b7d64a7ebf5a8ee4c3b239769d0e2687..72ceb0a276664722cf8c3d887b1598858ddbbc52 100644 (file)
--- a/epan/dissectors/packet-gsm_a_common.c
+++ b/epan/dissectors/packet-gsm_a_common.c
@@ -3598,11 +3598,16 @@ void gsm_a_stat_init(new_stat_tap_ui* new_stat, new_stat_tap_gui_init_cb gui_cal
     /* Add a row for each value type */
     for (i = 0; i < 256; i++)
     {
-       const char *ocs = try_val_to_str(i, msg_strings);
-       if (!ocs) ocs = g_strdup_printf("Unknown message %d", i);
+       const char *msg_str = try_val_to_str(i, msg_strings);
+       char *col_str;
+       if (msg_str) {
+           col_str = g_strdup(msg_str);
+       } else {
+           col_str = g_strdup_printf("Unknown message %d", i);
+       }
 
         items[IEI_COLUMN].value.uint_value = i;
-       items[MSG_NAME_COLUMN].value.string_value = ocs;
+       items[MSG_NAME_COLUMN].value.string_value = col_str;
        new_stat_tap_init_table_row(table, i, num_fields, items);
     }
 }
@@ -3762,6 +3767,13 @@ gsm_a_stat_reset(new_stat_tap_table* table)
     }
 }
 
+static void
+gsm_a_stat_free_table_item(new_stat_tap_table* table _U_, guint row _U_, guint column, stat_tap_table_item_type* field_data)
+{
+    if (column != MSG_NAME_COLUMN) return;
+    g_free((char*)field_data->value.string_value);
+}
+
 /* Register the protocol with Wireshark */
 void
 proto_register_gsm_a_common(void)
@@ -4713,7 +4725,7 @@ proto_register_gsm_a_common(void)
        gsm_a_bssmap_stat_init,
        gsm_a_bssmap_stat_packet,
        gsm_a_stat_reset,
-       NULL,
+       gsm_a_stat_free_table_item,
        NULL,
        sizeof(gsm_a_stat_fields)/sizeof(stat_tap_table_item), gsm_a_stat_fields,
        sizeof(gsm_a_stat_params)/sizeof(tap_param), gsm_a_stat_params,

diff --git a/epan/dissectors/packet-gsm_map.c b/epan/dissectors/packet-gsm_map.c
index 366d95a01fca3fd9a646ae00b5a0a0a77dfd8e6d..1b28ec5029cdb52283030110a4b09e0f1bcdc2bc 100644 (file)
--- a/epan/dissectors/packet-gsm_map.c
+++ b/epan/dissectors/packet-gsm_map.c
@@ -22519,10 +22519,15 @@ void gsm_map_stat_init(new_stat_tap_ui* new_stat, new_stat_tap_gui_init_cb gui_c
   for (i = 0; i < GSM_MAP_MAX_NUM_OPR_CODES; i++)
   {
     const char *ocs = try_val_to_str(i, gsm_map_opr_code_strings);
-    if (!ocs) ocs = g_strdup_printf("Unknown op code %d", i);
+    char *col_str;
+    if (ocs) {
+      col_str = g_strdup(ocs);
+    } else {
+      col_str = g_strdup_printf("Unknown op code %d", i);
+    }
 
     items[ID_COLUMN].value.uint_value = i;
-    items[OP_CODE_COLUMN].value.string_value = ocs;
+    items[OP_CODE_COLUMN].value.string_value = col_str;
     new_stat_tap_init_table_row(table, i, num_fields, items);
   }
 }
@@ -22599,6 +22604,13 @@ gsm_map_stat_reset(new_stat_tap_table* table)
   }
 }
 
+static void
+gsm_map_stat_free_table_item(new_stat_tap_table* table _U_, guint row _U_, guint column, stat_tap_table_item_type* field_data)
+{
+  if (column != OP_CODE_COLUMN) return;
+  g_free((char*)field_data->value.string_value);
+}
+
 /*--- proto_reg_handoff_gsm_map ---------------------------------------*/
 static void range_delete_callback(guint32 ssn)
 {
@@ -29840,7 +29852,7 @@ void proto_register_gsm_map(void) {
         NULL, HFILL }},
 
 /*--- End of included file: packet-gsm_map-hfarr.c ---*/
-#line 3039 "../../asn1/gsm_map/packet-gsm_map-template.c"
+#line 3051 "../../asn1/gsm_map/packet-gsm_map-template.c"
   };
 
   /* List of subtrees */
@@ -30555,7 +30567,7 @@ void proto_register_gsm_map(void) {
     &ett_NokiaMAP_Extensions_AllowedServiceData,
 
 /*--- End of included file: packet-gsm_map-ettarr.c ---*/
-#line 3073 "../../asn1/gsm_map/packet-gsm_map-template.c"
+#line 3085 "../../asn1/gsm_map/packet-gsm_map-template.c"
   };
 
   static ei_register_info ei[] = {
@@ -30586,7 +30598,7 @@ void proto_register_gsm_map(void) {
     gsm_map_stat_init,
     gsm_map_stat_packet,
     gsm_map_stat_reset,
-    NULL,
+    gsm_map_stat_free_table_item,
     NULL,
     sizeof(gsm_map_stat_fields)/sizeof(stat_tap_table_item), gsm_map_stat_fields,
     sizeof(gsm_map_stat_params)/sizeof(tap_param), gsm_map_stat_params,
@@ -30677,7 +30689,7 @@ void proto_register_gsm_map(void) {
 
 
 /*--- End of included file: packet-gsm_map-dis-tab.c ---*/
-#line 3129 "../../asn1/gsm_map/packet-gsm_map-template.c"
+#line 3141 "../../asn1/gsm_map/packet-gsm_map-template.c"
   oid_add_from_string("ericsson-gsm-Map-Ext","1.2.826.0.1249.58.1.0" );
   oid_add_from_string("accessTypeNotAllowed-id","1.3.12.2.1107.3.66.1.2");
   /*oid_add_from_string("map-ac networkLocUp(1) version3(3)","0.4.0.0.1.0.1.3" );

diff --git a/epan/stat_tap_ui.c b/epan/stat_tap_ui.c
index a6f8761e7a667e7a95bb29b9bf923336cd6a32cc..6242adca3fe170618aeba5fca321aebe4e0aa3ea 100644 (file)
--- a/epan/stat_tap_ui.c
+++ b/epan/stat_tap_ui.c
@@ -251,7 +251,7 @@ void reset_stat_table(new_stat_tap_ui* new_stat, new_stat_tap_gui_reset_cb gui_c
     }
 }
 
-void free_stat_table(new_stat_tap_ui* new_stat, new_stat_tap_gui_free_cb gui_callback, void *callback_data)
+void free_stat_tables(new_stat_tap_ui* new_stat, new_stat_tap_gui_free_cb gui_callback, void *callback_data)
 {
     guint i = 0, element, field_index;
     new_stat_tap_table *stat_table;
@@ -271,11 +271,16 @@ void free_stat_table(new_stat_tap_ui* new_stat, new_stat_tap_gui_free_cb gui_cal
             {
                 field_data = new_stat_tap_get_field_data(stat_table, element, field_index);
                 /* Give dissector a crack at it */
+                /* XXX Should this be per-row instead? */
                 if (new_stat->stat_tap_free_table_item_cb)
                     new_stat->stat_tap_free_table_item_cb(stat_table, element, field_index, field_data);
             }
+            g_free(stat_table->elements[element]);
         }
+        g_free(stat_table->elements);
+        g_free(stat_table);
     }
+    g_array_set_size(new_stat->tables, 0);
 }
 
 

diff --git a/epan/stat_tap_ui.h b/epan/stat_tap_ui.h
index cba5d46dc32c8e2ef24cb9b6d62af5ad2e9ae034..4689cd62934a843d2ff4c4452a2782c0356bd012 100644 (file)
--- a/epan/stat_tap_ui.h
+++ b/epan/stat_tap_ui.h
@@ -167,7 +167,19 @@ WS_DLL_PUBLIC void new_stat_tap_init_table_row(new_stat_tap_table *stat_table, g
 WS_DLL_PUBLIC stat_tap_table_item_type* new_stat_tap_get_field_data(const new_stat_tap_table *stat_table, guint table_index, guint field_index);
 WS_DLL_PUBLIC void new_stat_tap_set_field_data(new_stat_tap_table *stat_table, guint table_index, guint field_index, stat_tap_table_item_type* field_data);
 WS_DLL_PUBLIC void reset_stat_table(new_stat_tap_ui* new_stat, new_stat_tap_gui_reset_cb gui_callback, void *callback_data);
-WS_DLL_PUBLIC void free_stat_table(new_stat_tap_ui* new_stat, new_stat_tap_gui_free_cb gui_callback, void *callback_data);
+
+/** Free all of the tables associated with a new_stat_tap_ui.
+ *
+ * Frees data created by stat_tap_ui.stat_tap_init_cb.
+ * new_stat_tap_ui.stat_tap_free_table_item_cb is called for each index in each
+ * row.
+ *
+ * @param new_stat Parent new_stat_tap_ui struct, provided by the dissector.
+ * @param gui_callback Per-table callback, run before rows are removed.
+ * Provided by the UI.
+ * @param callback_data Data for the per-table callback.
+ */
+WS_DLL_PUBLIC void free_stat_tables(new_stat_tap_ui* new_stat, new_stat_tap_gui_free_cb gui_callback, void *callback_data);
 
 
 WS_DLL_PUBLIC gboolean process_stat_cmd_arg(char *optstr);

diff --git a/ui/gtk/simple_stattable.c b/ui/gtk/simple_stattable.c
index cde2d520c5f1aac52cfd0083e01f482da7c1c827..e366b75776634f15998bf7e227e8f8564eb6f656 100644 (file)
--- a/ui/gtk/simple_stattable.c
+++ b/ui/gtk/simple_stattable.c
@@ -62,7 +62,7 @@ win_destroy_cb(GtkWindow *win _U_, gpointer data)
 
        remove_tap_listener(&ss->data);
 
-       free_stat_table(ss->new_stat_tap, NULL, NULL);
+    free_stat_tables(ss->new_stat_tap, NULL, NULL);
 
        g_free(ss);
 }
@@ -223,7 +223,7 @@ init_simple_stat_tables(new_stat_tap_ui *new_stat_tap, const char *filter)
        if(error_string){
                simple_dialog(ESD_TYPE_ERROR, ESD_BTN_OK, "%s", error_string->str);
                g_string_free(error_string, TRUE);
-               free_stat_table(ss->new_stat_tap, NULL, NULL);
+               free_stat_tables(ss->new_stat_tap, NULL, NULL);
                g_free(ss);
                return;
        }

diff --git a/ui/qt/simple_statistics_dialog.cpp b/ui/qt/simple_statistics_dialog.cpp
index 7a224948bc64e54f1c63c667d878794d5b629607..ed77b3655e00a4f1953eb44a1776bd8ee02851c4 100644 (file)
--- a/ui/qt/simple_statistics_dialog.cpp
+++ b/ui/qt/simple_statistics_dialog.cpp
@@ -257,7 +257,7 @@ void SimpleStatisticsDialog::fillTree()
         QMessageBox::critical(this, tr("Failed to attach to tap \"%1\"").arg(stu_->tap_name),
                              error_string->str);
         g_string_free(error_string, TRUE);
-        free_stat_table(stu_, NULL, NULL);
+        free_stat_tables(stu_, NULL, NULL);
         reject();
     }
 
@@ -266,7 +266,7 @@ void SimpleStatisticsDialog::fillTree()
     tapDraw(&stat_data);
 
     remove_tap_listener(&stat_data);
-    free_stat_table(stu_, NULL, NULL);
+    free_stat_tables(stu_, NULL, NULL);
 }
 
 /*