Use a single wtap_rec and Buffer for an entire capture session.

That way we aren't allocating memory, reading packets from a batch, and
freeing the memory for each batch of packets delivered by dumpcap; we do
the allocation when the capture starts and the freeing when it finishes.

Change-Id: If012ab865f3a99d869535ad10827ad8680c1b10c
Reviewed-on: https://code.wireshark.org/review/32766
Petri-Dish: Guy Harris <guy@alum.mit.edu>
Tested-by: Petri Dish Buildbot
Reviewed-by: Anders Broman <a.broman58@gmail.com>

4 files changed, 53 insertions, 47 deletions

diff --git a/capchild/capture_session.h b/capchild/capture_session.h
index 65a7d48fa0..93ddabb147 100644
--- a/capchild/capture_session.h
+++ b/capchild/capture_session.h
@@ -52,6 +52,8 @@ typedef struct _capture_session {
     guint32   count;                      /**< Total number of frames captured */
     capture_options *capture_opts;        /**< options for this capture */
     capture_file *cf;                     /**< handle to cfile */
+    wtap_rec rec;                         /**< record we're reading packet metadata into */
+    Buffer buf;                           /**< Buffer we're reading packet data into */
     struct wtap *wtap;                    /**< current wtap file */
     struct _info_data *cap_data_info;     /**< stats for this capture */
 } capture_session;
diff --git a/file.c b/file.c
index 227b41f13a..e1db192416 100644
--- a/file.c
+++ b/file.c
@@ -752,7 +752,8 @@ cf_read(capture_file *cf, gboolean reloading)
 
 #ifdef HAVE_LIBPCAP
 cf_read_status_t
-cf_continue_tail(capture_file *cf, volatile int to_read, int *err)
+cf_continue_tail(capture_file *cf, volatile int to_read, wtap_rec *rec,
+                 Buffer *buf, int *err)
 {
   gchar            *err_info;
   volatile int      newly_displayed_packets = 0;
@@ -799,19 +800,15 @@ cf_continue_tail(capture_file *cf, volatile int to_read, int *err)
   epan_dissect_init(&edt, cf->epan, create_proto_tree, FALSE);
 
   TRY {
-    wtap_rec rec;
-    Buffer buf;
     gint64 data_offset = 0;
     column_info *cinfo;
 
     /* If any tap listeners require the columns, construct them. */
     cinfo = (tap_flags & TL_REQUIRES_COLUMNS) ? &cf->cinfo : NULL;
 
-    wtap_rec_init(&rec);
-    ws_buffer_init(&buf, 1514);
     while (to_read != 0) {
       wtap_cleareof(cf->provider.wth);
-      if (!wtap_read(cf->provider.wth, &rec, &buf, err, &err_info,
+      if (!wtap_read(cf->provider.wth, rec, buf, err, &err_info,
                      &data_offset)) {
         break;
       }
@@ -821,13 +818,11 @@ cf_continue_tail(capture_file *cf, volatile int to_read, int *err)
            aren't any packets left to read) exit. */
         break;
       }
-      if (read_record(cf, &rec, &buf, dfcode, &edt, cinfo, data_offset)) {
+      if (read_record(cf, rec, buf, dfcode, &edt, cinfo, data_offset)) {
         newly_displayed_packets++;
       }
       to_read--;
     }
-    wtap_rec_cleanup(&rec);
-    ws_buffer_free(&buf);
   }
   CATCH(OutOfMemoryError) {
     simple_message_box(ESD_TYPE_ERROR, NULL,
@@ -898,11 +893,9 @@ cf_fake_continue_tail(capture_file *cf) {
 }
 
 cf_read_status_t
-cf_finish_tail(capture_file *cf, int *err)
+cf_finish_tail(capture_file *cf, wtap_rec *rec, Buffer *buf, int *err)
 {
   gchar     *err_info;
-  wtap_rec   rec;
-  Buffer     buf;
   gint64     data_offset;
   dfilter_t *dfcode;
   column_info *cinfo;
@@ -951,19 +944,15 @@ cf_finish_tail(capture_file *cf, int *err)
 
   epan_dissect_init(&edt, cf->epan, create_proto_tree, FALSE);
 
-  wtap_rec_init(&rec);
-  ws_buffer_init(&buf, 1514);
-  while ((wtap_read(cf->provider.wth, &rec, &buf, err, &err_info, &data_offset))) {
+  while ((wtap_read(cf->provider.wth, rec, buf, err, &err_info, &data_offset))) {
     if (cf->state == FILE_READ_ABORTED) {
       /* Well, the user decided to abort the read.  Break out of the
          loop, and let the code below (which is called even if there
          aren't any packets left to read) exit. */
       break;
     }
-    read_record(cf, &rec, &buf, dfcode, &edt, cinfo, data_offset);
+    read_record(cf, rec, buf, dfcode, &edt, cinfo, data_offset);
   }
-  wtap_rec_cleanup(&rec);
-  ws_buffer_free(&buf);
 
   /* Cleanup and release all dfilter resources */
   dfilter_free(dfcode);
diff --git a/file.h b/file.h
index 0d9bcb318e..c3ed5ad625 100644
--- a/file.h
+++ b/file.h
@@ -169,10 +169,13 @@ gboolean cf_read_record(capture_file *cf, frame_data *fdata);
  *
  * @param cf the capture file to be read from
  * @param to_read the number of packets to read
+ * @param rec pointer to wtap_rec to use when reading
+ * @param buf pointer to Buffer to use when reading
  * @param err the error code, if an error had occurred
  * @return one of cf_read_status_t
  */
-cf_read_status_t cf_continue_tail(capture_file *cf, volatile int to_read, int *err);
+cf_read_status_t cf_continue_tail(capture_file *cf, volatile int to_read,
+                                  wtap_rec *rec, Buffer *buf, int *err);
 
 /**
  * Fake reading packets from the "end" of a capture file.
@@ -185,10 +188,13 @@ void cf_fake_continue_tail(capture_file *cf);
  * Finish reading from "end" of a capture file.
  *
  * @param cf the capture file to be read from
+ * @param rec pointer to wtap_rec to use when reading
+ * @param buf pointer to Buffer to use when reading
  * @param err the error code, if an error had occurred
  * @return one of cf_read_status_t
  */
-cf_read_status_t cf_finish_tail(capture_file *cf, int *err);
+cf_read_status_t cf_finish_tail(capture_file *cf, wtap_rec *rec,
+                                Buffer *buf, int *err);
 
 /**
  * Determine whether this capture file (or a range of it) can be written
diff --git a/ui/capture.c b/ui/capture.c
index 9c60d29da6..239d8981b6 100644
--- a/ui/capture.c
+++ b/ui/capture.c
@@ -117,7 +117,6 @@ capture_callback_remove(capture_callback_t func, gpointer user_data)
 gboolean
 capture_start(capture_options *capture_opts, capture_session *cap_session, info_data_t* cap_data, void(*update_cb)(void))
 {
-    gboolean ret;
     GString *source;
 
     cap_session->state = CAPTURE_PREPARING;
@@ -127,8 +126,8 @@ capture_start(capture_options *capture_opts, capture_session *cap_session, info_
     cf_set_tempfile_source((capture_file *)cap_session->cf, source->str);
     g_string_free(source, TRUE);
     /* try to start the capture child process */
-    ret = sync_pipe_start(capture_opts, cap_session, cap_data, update_cb);
-    if(!ret) {
+    if (!sync_pipe_start(capture_opts, cap_session, cap_data, update_cb)) {
+        /* We failed to start the capture child. */
         if(capture_opts->save_file != NULL) {
             g_free(capture_opts->save_file);
             capture_opts->save_file = NULL;
@@ -136,34 +135,38 @@ capture_start(capture_options *capture_opts, capture_session *cap_session, info_
 
         g_log(LOG_DOMAIN_CAPTURE, G_LOG_LEVEL_MESSAGE, "Capture Start failed.");
         cap_session->state = CAPTURE_STOPPED;
-    } else {
-        /* the capture child might not respond shortly after bringing it up */
-        /* (for example: it will block if no input arrives from an input capture pipe (e.g. mkfifo)) */
-
-        /* to prevent problems, bring the main GUI into "capture mode" right after a successful */
-        /* spawn/exec of the capture child, without waiting for any response from it */
-        capture_callback_invoke(capture_cb_capture_prepared, cap_session);
+        return FALSE;
+    }
 
-        if (capture_opts->show_info) {
-            cap_session->wtap = NULL;
+    /* the capture child might not respond shortly after bringing it up */
+    /* (for example: it will block if no input arrives from an input capture pipe (e.g. mkfifo)) */
 
-            if (cap_data->counts.counts_hash != NULL)
-            {
-                /* Clean up any previous lists of packet counts */
-                g_hash_table_destroy(cap_data->counts.counts_hash);
-            }
+    /* to prevent problems, bring the main GUI into "capture mode" right after a successful */
+    /* spawn/exec of the capture child, without waiting for any response from it */
+    capture_callback_invoke(capture_cb_capture_prepared, cap_session);
 
-            cap_data->counts.counts_hash = g_hash_table_new_full(g_direct_hash, g_direct_equal, NULL, g_free);
-            cap_data->counts.other = 0;
-            cap_data->counts.total = 0;
+    wtap_rec_init(&cap_session->rec);
+    ws_buffer_init(&cap_session->buf, 1514);
 
-            cap_data->ui.counts = &cap_data->counts;
+    if (capture_opts->show_info) {
+        cap_session->wtap = NULL;
 
-            capture_info_ui_create(&cap_data->ui, cap_session);
+        if (cap_data->counts.counts_hash != NULL)
+        {
+            /* Clean up any previous lists of packet counts */
+            g_hash_table_destroy(cap_data->counts.counts_hash);
         }
+
+        cap_data->counts.counts_hash = g_hash_table_new_full(g_direct_hash, g_direct_equal, NULL, g_free);
+        cap_data->counts.other = 0;
+        cap_data->counts.total = 0;
+
+        cap_data->ui.counts = &cap_data->counts;
+
+        capture_info_ui_create(&cap_data->ui, cap_session);
     }
 
-    return ret;
+    return TRUE;
 }
 
 
@@ -412,7 +415,8 @@ capture_input_new_file(capture_session *cap_session, gchar *new_file)
         if( ((capture_file *) cap_session->cf)->state != FILE_CLOSED) {
             if(capture_opts->real_time_mode) {
                 capture_callback_invoke(capture_cb_capture_update_finished, cap_session);
-                cf_finish_tail((capture_file *)cap_session->cf, &err);
+                cf_finish_tail((capture_file *)cap_session->cf,
+                               &cap_session->rec, &cap_session->buf, &err);
                 cf_close((capture_file *)cap_session->cf);
             } else {
                 capture_callback_invoke(capture_cb_capture_fixed_finished, cap_session);
@@ -484,7 +488,8 @@ capture_input_new_packets(capture_session *cap_session, int to_read)
 
     if(capture_opts->real_time_mode) {
         /* Read from the capture file the number of records the child told us it added. */
-        switch (cf_continue_tail((capture_file *)cap_session->cf, to_read, &err)) {
+        switch (cf_continue_tail((capture_file *)cap_session->cf, to_read,
+                                 &cap_session->rec, &cap_session->buf, &err)) {
 
             case CF_READ_OK:
             case CF_READ_ERROR:
@@ -635,8 +640,11 @@ capture_input_closed(capture_session *cap_session, gchar *msg)
     if (msg != NULL)
         simple_dialog(ESD_TYPE_ERROR, ESD_BTN_OK, "%s", msg);
 
+    wtap_rec_cleanup(&cap_session->rec);
+    ws_buffer_free(&cap_session->buf);
     if(cap_session->state == CAPTURE_PREPARING) {
-        /* We didn't start a capture; note that the attempt to start it
+        /* We started the capture child, but we didn't manage to start
+           the capture process; note that the attempt to start it
            failed. */
         capture_callback_invoke(capture_cb_capture_failed, cap_session);
     } else {
@@ -647,7 +655,8 @@ capture_input_closed(capture_session *cap_session, gchar *msg)
             cf_read_status_t status;
 
             /* Read what remains of the capture file. */
-            status = cf_finish_tail((capture_file *)cap_session->cf, &err);
+            status = cf_finish_tail((capture_file *)cap_session->cf,
+                                    &cap_session->rec, &cap_session->buf, &err);
 
             /* Tell the GUI we are not doing a capture any more.
                Must be done after the cf_finish_tail(), so file lengths are