various capture code cleanup and fixes:

display filename in statusbar while capturing
print_usage banner fixed
cf_cb_live_capture_prepare no longer needed
rename sync_pipe_do_capture -> sync_pipe_start
bugfix: sync_pipe_input_wait_for_start replaced by former implementation
fix cleanup of old file in capture_input_new_file
fix a tempfile detection bug (named file showed up as tempfile after capture)

svn path=/trunk/; revision=14053

5 files changed, 103 insertions, 105 deletions

diff --git a/capture.c b/capture.c
index 271849be40..637711e8f8 100644
--- a/capture.c
+++ b/capture.c
@@ -83,12 +83,8 @@ capture_start(capture_options *capture_opts)
   cf_close(capture_opts->cf);
 
   /* try to start the capture child process */
-  ret = sync_pipe_do_capture(capture_opts, capture_opts->save_file == NULL);
-
-  if(ret) {
-      /* tell callbacks (menu, ...) that capture is running now */
-      cf_callback_invoke(cf_cb_live_capture_prepare, capture_opts);
-  } else {
+  ret = sync_pipe_start(capture_opts, capture_opts->save_file == NULL);
+  if(!ret) {
       if(capture_opts->save_file != NULL) {
           g_free(capture_opts->save_file);
           capture_opts->save_file = NULL;
@@ -99,6 +95,22 @@ capture_start(capture_options *capture_opts)
 }
 
 
+void
+capture_stop(capture_options *capture_opts)
+{
+  /* stop the capture child gracefully */
+  sync_pipe_stop(capture_opts);
+}
+
+void
+capture_kill_child(capture_options *capture_opts)
+{
+  /* kill the capture child */
+  sync_pipe_kill(capture_opts);
+}
+
+
+
 /* We've succeeded a (non real-time) capture, try to read it into a new capture file */
 static gboolean
 capture_input_read_all(capture_options *capture_opts, gboolean is_tempfile, gboolean drops_known,
@@ -192,12 +204,15 @@ capture_input_new_file(capture_options *capture_opts, gchar *new_file)
 
   /* free the old filename */
   if(capture_opts->save_file != NULL) {
-    /* we start a new capture file, simply close the old one */
-    /* XXX - is it enough to call cf_close here? */
-    /* XXX - is it safe to call cf_close even if the file is close before? */
-    cf_close(capture_opts->cf);
+    /* we start a new capture file, close the old one (if we had one before) */
+    if( ((capture_file *) capture_opts->cf)->state != FILE_CLOSED) {
+        cf_callback_invoke(cf_cb_live_capture_update_finished, capture_opts->cf);
+        cf_finish_tail(capture_opts->cf, &err);
+        cf_close(capture_opts->cf);
+    }
     g_free(capture_opts->save_file);
     is_tempfile = FALSE;
+    cf_set_tempfile(capture_opts->cf, FALSE);
   } else {
     /* we didn't had a save_file before, must be a tempfile */
     is_tempfile = TRUE;
@@ -317,19 +332,4 @@ capture_input_closed(capture_options *capture_opts)
 }
 
 
-void
-capture_stop(capture_options *capture_opts)
-{
-  /* stop the capture child gracefully, if we have one */
-  sync_pipe_stop(capture_opts);
-}
-
-void
-capture_kill_child(capture_options *capture_opts)
-{
-  /* kill the capture child, if we have one */
-  sync_pipe_kill(capture_opts);
-}
-
-
 #endif /* HAVE_LIBPCAP */
diff --git a/capture_sync.c b/capture_sync.c
index 4ce506621f..ab77f3352b 100644
--- a/capture_sync.c
+++ b/capture_sync.c
@@ -230,7 +230,7 @@ sync_pipe_quote_encapsulate(const char *string)
 #define ARGV_NUMBER_LEN 24
 
 gboolean
-sync_pipe_do_capture(capture_options *capture_opts, gboolean is_tempfile) {
+sync_pipe_start(capture_options *capture_opts, gboolean is_tempfile) {
     char ssnap[ARGV_NUMBER_LEN];
     char scount[ARGV_NUMBER_LEN];
     char sfilesize[ARGV_NUMBER_LEN];
@@ -254,7 +254,7 @@ sync_pipe_do_capture(capture_options *capture_opts, gboolean is_tempfile) {
     int sync_pipe[2];                       /* pipe used to send messages from child to parent */
 
 
-    /*g_warning("sync_pipe_do_capture");
+    /*g_warning("sync_pipe_start");
     capture_opts_info(capture_opts);*/
 
     capture_opts->fork_child = -1;
@@ -502,76 +502,63 @@ sync_pipe_input_wait_for_start(capture_options *capture_opts, int sync_pipe_read
     char    *msg;
 
 
-    /* Read a byte count from "sync_pipe_read", terminated with a
+    /* Read a byte count from "sync_pipe[PIPE_READ]", terminated with a
        colon; if the count is 0, the child process created the
        capture file and we should start reading from it, otherwise
        the capture couldn't start and the count is a count of bytes
        of error message, and we should display the message. */
     byte_count = 0;
     for (;;) {
-        i = read(sync_pipe_read, &c, 1);
-        if (i == 0) {
-            /* EOF - the child process died, report the failure. */
-            sync_pipe_wait_for_child(capture_opts, TRUE);
-            return FALSE;
-        }
-
-        /* the first message should be the capture start or an error message */
-        if (c == SP_CAPSTART || c == SP_ERROR_MSG) {
-            return TRUE;
-        }
+      i = read(sync_pipe_read, &c, 1);
+      if (i == 0) {
+	/* EOF - the child process died, report the failure. */
+	sync_pipe_wait_for_child(capture_opts, TRUE);
+	return FALSE;
+      }
 
-        if (!isdigit(c)) {
+      /* the first message should be the capture start or an error message */
+      if (c == SP_CAPSTART || c == SP_ERROR_MSG)
+	break;
+      if (!isdigit(c)) {
             /* Child process handed us crap, report the failure. */
-            simple_dialog(ESD_TYPE_ERROR, ESD_BTN_OK,
-            "Capture child process sent us a bad message");
-            return FALSE;
-        }
-
-        byte_count = byte_count*10 + c - '0';
-
-        if (c != SP_CAPSTART) {
-        /* Failure - the child process sent us a message indicating
-        what the problem was. */
-        if (byte_count == 0) {
-            /* Zero-length message? */
-            simple_dialog(ESD_TYPE_ERROR, ESD_BTN_OK,
-                "Capture child process failed, but its error message was empty.");
-            return FALSE;
-        }
-
-        msg = g_malloc(byte_count + 1);
-        if (msg == NULL) {
-            simple_dialog(ESD_TYPE_ERROR, ESD_BTN_OK,
-                "Capture child process failed, but its error message was too big.");
-            return FALSE;
-        }
-
-        i = read(sync_pipe_read, msg, byte_count);
-        msg[byte_count] = '\0';
-        if (i < 0) {
-            simple_dialog(ESD_TYPE_ERROR, ESD_BTN_OK,
-                "Capture child process failed: Error %s reading its error message.",
-            strerror(errno));
-            g_free(msg);
-            return FALSE;
-        }
-        
-        if (i == 0) {
-            simple_dialog(ESD_TYPE_ERROR, ESD_BTN_OK,
-            "Capture child process failed: EOF reading its error message.");
-            sync_pipe_wait_for_child(capture_opts, FALSE);
-            g_free(msg);
-            return FALSE;
-        }
+	simple_dialog(ESD_TYPE_ERROR, ESD_BTN_OK,
+			"Capture child process sent us a bad message");
+	return FALSE;
+      }
+      byte_count = byte_count*10 + c - '0';
+    }
+    if (c != SP_CAPSTART) {
+      /* Failure - the child process sent us a message indicating
+	 what the problem was. */
+      if (byte_count == 0) {
+	/* Zero-length message? */
+	simple_dialog(ESD_TYPE_ERROR, ESD_BTN_OK,
+		"Capture child process failed, but its error message was empty.");
+      } else {
+	msg = g_malloc(byte_count + 1);
+	if (msg == NULL) {
+	  simple_dialog(ESD_TYPE_ERROR, ESD_BTN_OK,
+		"Capture child process failed, but its error message was too big.");
+	} else {
+	  i = read(sync_pipe_read, msg, byte_count);
+	  msg[byte_count] = '\0';
+	  if (i < 0) {
+	    simple_dialog(ESD_TYPE_ERROR, ESD_BTN_OK,
+		  "Capture child process failed: Error %s reading its error message.",
+		  strerror(errno));
+	  } else if (i == 0) {
+	    simple_dialog(ESD_TYPE_ERROR, ESD_BTN_OK,
+		  "Capture child process failed: EOF reading its error message.");
+	    sync_pipe_wait_for_child(capture_opts, FALSE);
+	  } else
+	    simple_dialog(ESD_TYPE_ERROR, ESD_BTN_OK, msg);
+	  g_free(msg);
+	}
 
-        simple_dialog(ESD_TYPE_ERROR, ESD_BTN_OK, msg);
-        g_free(msg);
-        return FALSE;
-        }
+      }
+      return FALSE;
     }
 
-    g_assert_not_reached();
     return TRUE;
 }
 
@@ -756,6 +743,7 @@ sync_pipe_wait_for_child(capture_options *capture_opts, gboolean always_report)
 
 
 #ifndef _WIN32
+/* convert signal to corresponding name */
 static char *
 sync_pipe_signame(int sig)
 {
@@ -850,20 +838,25 @@ sync_pipe_signame(int sig)
 #endif
 
 
+/* user wants to stop the capture run */
 void
 sync_pipe_stop(capture_options *capture_opts)
 {
   /* XXX - in which cases this will be 0? */
   if (capture_opts->fork_child != -1 && capture_opts->fork_child != 0) {
 #ifndef _WIN32
+    /* send the SIGUSR1 signal to close the capture child gracefully. */
     kill(capture_opts->fork_child, SIGUSR1);
 #else
+    /* Win32 doesn't have the kill() system call, use the special signal pipe 
+       instead to close the capture child gracefully. */
     signal_pipe_capend_to_child(capture_opts);
 #endif
   }
 }
 
 
+/* Ethereal has to exit, force the capture child to close */
 void
 sync_pipe_kill(capture_options *capture_opts)
 {
diff --git a/capture_sync.h b/capture_sync.h
index 5a52552663..b429cc4184 100644
--- a/capture_sync.h
+++ b/capture_sync.h
@@ -27,7 +27,8 @@
  *  
  *  Sync mode capture (internal interface).
  *
- *  Will start a new Ethereal child instance which will do the actual capture work.
+ *  Will start a new Ethereal child instance which will do the actual capture 
+ *  work.
  */
 
 #ifndef __CAPTURE_SYNC_H__
@@ -36,6 +37,8 @@
 /** 
  * Start a new capture session.
  *  Create a capture child which is doing the real capture work.
+ *  The various capture_input_... functions will be called, if something had 
+ *  happened.
  *
  *  Most of the parameters are passed through the global capture_opts.
  *
@@ -44,7 +47,7 @@
  *  @return             TRUE if a capture could be started, FALSE if not
  */
 extern gboolean 
-sync_pipe_do_capture(capture_options *capture_opts, gboolean is_tempfile);
+sync_pipe_start(capture_options *capture_opts, gboolean is_tempfile);
 
 /** User wants to stop capturing, gracefully close the capture child */
 extern void
diff --git a/file.h b/file.h
index 8b6e8d9cd7..da9277f327 100644
--- a/file.h
+++ b/file.h
@@ -60,7 +60,6 @@ typedef enum {
     cf_cb_file_read_start,
     cf_cb_file_read_finished,
 #ifdef HAVE_LIBPCAP
-    cf_cb_live_capture_prepare,
     cf_cb_live_capture_update_started,
     cf_cb_live_capture_fixed_started,
     cf_cb_live_capture_update_finished,
diff --git a/gtk/main.c b/gtk/main.c
index 452ffc061f..647d9cf7cf 100644
--- a/gtk/main.c
+++ b/gtk/main.c
@@ -1082,7 +1082,7 @@ print_usage(gboolean print_ver) {
 #ifdef HAVE_LIBPCAP
   fprintf(output, "\n%s [ -vh ] [ -klLnpQS ] [ -a <capture autostop condition> ] ...\n",
 	  PACKAGE);
-  fprintf(output, "\t[ -b <number of ringbuffer files>[:<duration>] ]\n");
+  fprintf(output, "\t[ -b <ringbuffer files options> ] ...]\n");
   fprintf(output, "\t[ -B <byte view height> ] [ -c <count> ] [ -f <capture filter> ]\n");
   fprintf(output, "\t[ -i <interface> ] [ -m <medium font> ] [ -N <resolving> ]\n");
   fprintf(output, "\t[ -o <preference setting> ] ... [ -P <packet list height> ]\n");
@@ -1282,8 +1282,9 @@ main_cf_cb_file_read_finished(capture_file *cf)
 
 #ifdef HAVE_LIBPCAP
 static void
-main_cf_cb_live_capture_prepare(capture_options *capture_opts)
+main_cf_cb_live_capture_update_started(capture_options *capture_opts)
 {
+    gchar *capture_msg;
     gchar *title;
 
 
@@ -1291,12 +1292,6 @@ main_cf_cb_live_capture_prepare(capture_options *capture_opts)
                             get_interface_descriptive_name(capture_opts->iface));
     set_main_window_name(title);
     g_free(title);
-}
-
-static void
-main_cf_cb_live_capture_update_started(capture_options *capture_opts)
-{
-    gchar *capture_msg;
 
     /* Disable menu items that make no sense if you're currently running
        a capture. */
@@ -1306,7 +1301,8 @@ main_cf_cb_live_capture_update_started(capture_options *capture_opts)
        packets (yes, I know, we don't have any *yet*). */
     set_menus_for_captured_packets(TRUE);
 
-    capture_msg = g_strdup_printf(" %s: <live capture in progress>", get_interface_descriptive_name(capture_opts->iface));
+    capture_msg = g_strdup_printf(" %s: <live capture in progress> to file: %s", 
+        get_interface_descriptive_name(capture_opts->iface), capture_opts->save_file);
 
     statusbar_push_file_msg(capture_msg);
 
@@ -1320,6 +1316,13 @@ static void
 main_cf_cb_live_capture_fixed_started(capture_options *capture_opts)
 {
     gchar *capture_msg;
+    gchar *title;
+
+
+    title = g_strdup_printf("%s: Capturing - Ethereal",
+                            get_interface_descriptive_name(capture_opts->iface));
+    set_main_window_name(title);
+    g_free(title);
 
     /* Disable menu items that make no sense if you're currently running
        a capture. */
@@ -1329,15 +1332,15 @@ main_cf_cb_live_capture_fixed_started(capture_options *capture_opts)
        packets (yes, I know, we don't have any *yet*). */
     /*set_menus_for_captured_packets(TRUE);*/
 
-    capture_msg = g_strdup_printf(" %s: <live capture in progress>", get_interface_descriptive_name(capture_opts->iface));
+    capture_msg = g_strdup_printf(" %s: <live capture in progress> to file: %s", 
+        get_interface_descriptive_name(capture_opts->iface), capture_opts->save_file);
 
     statusbar_push_file_msg(capture_msg);
+    gtk_statusbar_push(GTK_STATUSBAR(packets_bar), packets_ctx, " <capturing>");
 
     g_free(capture_msg);
 
-    /* Set up main window for a capture file. */
-/*    main_set_for_capture_file(TRUE);*/
-    /* XXX: shouldn't be already set */
+    /* Don't set up main window for a capture file. */
     main_set_for_capture_file(FALSE);
 }
 
@@ -1368,6 +1371,9 @@ main_cf_cb_live_capture_fixed_finished(capture_file *cf)
     /* Pop the "<live capture in progress>" message off the status bar. */
     statusbar_pop_file_msg();
 
+    /* Pop the "<capturing>" message off the status bar */
+	gtk_statusbar_pop(GTK_STATUSBAR(packets_bar), packets_ctx);
+
     /*set_display_filename(cf);*/
 
     /* Enable menu items that make sense if you're not currently running
@@ -1463,9 +1469,6 @@ void main_cf_callback(gint event, gpointer data, gpointer user_data _U_)
         main_cf_cb_file_read_finished(data);
         break;
 #ifdef HAVE_LIBPCAP
-    case(cf_cb_live_capture_prepare):
-        main_cf_cb_live_capture_prepare(data);
-        break;
     case(cf_cb_live_capture_update_started):
         main_cf_cb_live_capture_update_started(data);
         break;