Fix the leaking of packet comments, i.e. the rest of

https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=7530

The frame_data_cleanup function was ambiguous; it was being used for two
different purposes, and did neither of them quite properly. Split it instead
into frame_data_reset and frame_data_destroy, and call the correct one depending
on why we were originally calling frame_data_cleanup.

svn path=/trunk/; revision=48324

6 files changed, 34 insertions, 17 deletions

diff --git a/epan/frame_data.c b/epan/frame_data.c
index e89862448d..c7558d88aa 100644
--- a/epan/frame_data.c
+++ b/epan/frame_data.c
@@ -315,22 +315,39 @@ frame_data_set_after_dissect(frame_data *fdata,
 }
 
 void
-frame_data_cleanup(frame_data *fdata)
+frame_data_reset(frame_data *fdata)
+{
+  fdata->flags.visited = 0;
+
+  if (fdata->pfd) {
+    g_slist_free(fdata->pfd);
+    fdata->pfd = NULL;
+  }
+}
+
+void
+frame_data_destroy(frame_data *fdata)
 {
   if (fdata->pfd) {
     g_slist_free(fdata->pfd);
     fdata->pfd = NULL;
   }
 
-  /* XXX, frame_data_cleanup() is called when redissecting (rescan_packets()),
-   *      which might be triggered by lot of things, like: preferences change,
-   *      setting manual address resolve, etc.. (grep by redissect_packets)
-   *      fdata->opt_comment can be set by user, which we must not discard when redissecting.
-   */
-#if 0
   if (fdata->opt_comment) {
     g_free(fdata->opt_comment);
     fdata->opt_comment = NULL;
   }
-#endif
 }
+
+/*
+ * 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:
+ */
diff --git a/epan/frame_data.h b/epan/frame_data.h
index acda8d5d99..94bc7404dd 100644
--- a/epan/frame_data.h
+++ b/epan/frame_data.h
@@ -97,7 +97,9 @@ void p_remove_proto_data(frame_data *fd, int proto);
 /** compare two frame_datas */
 WS_DLL_PUBLIC gint frame_data_compare(const frame_data *fdata1, const frame_data *fdata2, int field);
 
-WS_DLL_PUBLIC void frame_data_cleanup(frame_data *fdata);
+WS_DLL_PUBLIC void frame_data_reset(frame_data *fdata);
+
+WS_DLL_PUBLIC void frame_data_destroy(frame_data *fdata);
 
 WS_DLL_PUBLIC void frame_data_init(frame_data *fdata, guint32 num,
                 const struct wtap_pkthdr *phdr, gint64 offset,
diff --git a/file.c b/file.c
index 0a57d792ce..73360ae341 100644
--- a/file.c
+++ b/file.c
@@ -1905,8 +1905,7 @@ rescan_packets(capture_file *cf, const char *action, const char *action_item, gb
        * as not visited, free the GSList referring to the state
        * data (the per-frame data itself was freed by
        * "init_dissection()"), and null out the GSList pointer. */
-      fdata->flags.visited = 0;
-      frame_data_cleanup(fdata);
+      frame_data_reset(fdata);
       frames_count = cf->count;
     }
 
@@ -1963,8 +1962,7 @@ rescan_packets(capture_file *cf, const char *action, const char *action_item, gb
        until it finishes.  Should we just stick them with that? */
     for (; framenum <= frames_count; framenum++) {
       fdata = frame_data_sequence_find(cf->frames, framenum);
-      fdata->flags.visited = 0;
-      frame_data_cleanup(fdata);
+      frame_data_reset(fdata);
     }
   }
 
diff --git a/frame_data_sequence.c b/frame_data_sequence.c
index 770d1cea34..b2ee96ddf9 100644
--- a/frame_data_sequence.c
+++ b/frame_data_sequence.c
@@ -279,7 +279,7 @@ free_frame_data_array(void *array, guint count, guint level, gboolean last)
     frame_data *real_array = (frame_data *) array;
 
     for (i=0; i < level_count; i++) {
-      frame_data_cleanup(&real_array[i]);
+      frame_data_destroy(&real_array[i]);
     }
   }
 
diff --git a/rawshark.c b/rawshark.c
index 79ba938736..b022e007da 100644
--- a/rawshark.c
+++ b/rawshark.c
@@ -1128,7 +1128,7 @@ process_packet(capture_file *cf, gint64 offset, struct wtap_pkthdr *whdr,
     }
 
     epan_dissect_cleanup(&edt);
-    frame_data_cleanup(&fdata);
+    frame_data_destroy(&fdata);
 
     return passed;
 }
diff --git a/tshark.c b/tshark.c
index d36a815c2d..ba34eb0404 100644
--- a/tshark.c
+++ b/tshark.c
@@ -2677,7 +2677,7 @@ process_packet_first_pass(capture_file *cf,
   } else {
     /* if we don't add it to the frame_data_sequence, clean it up right now
      * to avoid leaks */
-    frame_data_cleanup(&fdlocal);
+    frame_data_destroy(&fdlocal);
     /* TODO, bug #8160 */
     /*
     prev_cap_frame = fdlocal;
@@ -3252,7 +3252,7 @@ process_packet(capture_file *cf, gint64 offset, struct wtap_pkthdr *whdr,
 
   if (do_dissection) {
     epan_dissect_cleanup(&edt);
-    frame_data_cleanup(&fdata);
+    frame_data_destroy(&fdata);
   }
   return passed;
 }