it should now be possible to use "Update packets in real time" even if used with one of the "Multiple files" option.

If this is used together with an option where input files changes too fast (e.g. new file every second), capturing will be (hopefully) stopped.

I've replaced the former capture pipe message format into a somewhat more general format to remove a lot of confusion.


svn path=/trunk/; revision=14054

6 files changed, 176 insertions, 210 deletions

diff --git a/capture.c b/capture.c
index 637711e8f8..951b02114b 100644
--- a/capture.c
+++ b/capture.c
@@ -326,9 +326,10 @@ capture_input_closed(capture_options *capture_opts)
     }
 
     /* We're not doing a capture any more, so we don't have a save file. */
-    g_assert(capture_opts->save_file);
-    g_free(capture_opts->save_file);
-    capture_opts->save_file = NULL;
+    if(capture_opts->save_file) {
+        g_free(capture_opts->save_file);
+        capture_opts->save_file = NULL;
+    }
 }
 
 
diff --git a/capture_loop.c b/capture_loop.c
index b1b5a7e94d..2bf21491b1 100644
--- a/capture_loop.c
+++ b/capture_loop.c
@@ -1252,6 +1252,7 @@ capture_loop_start(capture_options *capture_opts, gboolean *stats_known, struct
 
           if(!result || avail > 0) {
             ld.go = FALSE;
+            /*g_warning("loop closing");*/
           }
       }
 #endif
@@ -1384,6 +1385,8 @@ capture_loop_start(capture_options *capture_opts, gboolean *stats_known, struct
   /* close the input file (pcap or capture pipe) */
   capture_loop_close_input(&ld);
 
+  /*g_warning("loop closed");*/
+
   /* ok, if the write and the close were successful. */
   return write_ok && close_ok;
 
@@ -1407,6 +1410,8 @@ error:
   /* close the input file (pcap or cap_pipe) */
   capture_loop_close_input(&ld);
 
+  /*g_warning("loop error");*/
+
   return FALSE;
 }
 
diff --git a/capture_sync.c b/capture_sync.c
index ab77f3352b..169014dde5 100644
--- a/capture_sync.c
+++ b/capture_sync.c
@@ -118,70 +118,162 @@ static void sync_pipe_wait_for_child(capture_options *capture_opts, gboolean alw
 /*
  * Indications sent out on the sync pipe.
  */
-#define SP_CAPSTART     ';'	/* capture start message */
-#define SP_PACKET_COUNT '*'     /* followed by count of packets captured since last message */
-#define SP_ERROR_MSG    '!'     /* followed by length of error message that follows */
-#define SP_DROPS        '#'	/* followed by count of packets dropped in capture */
-#define SP_FILE	        ':'	/* followed by length of the name of the last opened file that follows */
+#define SP_CAPSTART     'S'	    /* capture start message */
+#define SP_CAPQUIT      'Q'     /* capture quit message */
+#define SP_PACKET_COUNT 'P'     /* count of packets captured since last message */
+#define SP_ERROR_MSG    'E'     /* error message */
+#define SP_DROPS        'D'	    /* count of packets dropped in capture */
+#define SP_FILE	        'F'	    /* the name of the recently opened file */
 
 
+/* write a message to the recipient pipe in the standard format 
+   (3 digit message length (excluding length and indicator field), 
+   1 byte message indicator and the rest is the message) */
+static void
+pipe_write_block(int pipe, char indicator, int len, const char *msg)
+{
+    char lenbuf[SP_DECISIZE+1+1];
+    int ret;
+
+    /*g_warning("write %d enter", pipe);*/
+
+    g_assert(len < 1000);
+    g_assert(indicator < '0' || indicator > '9');
+
+    /* write header (3 digit len + indicator + zero terminator) */
+    g_snprintf(lenbuf, 5, "%03u%c", len, indicator);
+
+    ret = write(pipe, lenbuf, strlen(lenbuf));
+    g_assert(ret != -1);
+
+    /* write value (if we have one) */
+    if(len) {
+        /*g_warning("write %d indicator: %c value len: %u msg: %s", pipe, indicator, len, msg);*/
+        ret = write(pipe, msg, len);
+        g_assert(ret != -1);
+    } else {
+        /*g_warning("write %d indicator: %c no value", pipe, indicator);*/
+    }
+
+    /*g_warning("write %d leave", pipe);*/
+}
+
+
+/* read a message from the sending pipe in the standard format 
+   (3 digit message length (excluding length and indicator field), 
+   1 byte message indicator and the rest is the message) */
+int
+pipe_read_block(int pipe, char *indicator, int len, char *msg) {
+    int required;
+    int newly;
+    char header[4];
+    int offset;
+
+
+    /* read header (3 digit len and indicator) */
+    required = 4;
+    offset = 0;
+    while(required) {
+        newly = read(pipe, &header[offset], required);
+        if (newly == 0) {
+            /* EOF */
+            /*g_warning("read %d header empty (capture closed)", pipe);*/
+            return newly;
+        }
+        if (newly < 0) {
+            /* error */
+            /*g_warning("read %d header error: %s", pipe, strerror(errno));*/
+            return newly;
+        }
+
+        required -= newly;
+        offset += newly;
+    }
+
+    /* convert header values */
+    *indicator = header[3];
+    required = atoi(header);
+
+    /* only indicator with no value? */
+    if(required == 0) {
+        /*g_warning("read %d indicator: %c empty value", pipe, *indicator);*/
+        return 4;
+    }
+
+    g_assert(required <= len);
+    len = required;
+
+    /* read value */
+    offset = 0;
+    while(required) {
+        newly = read(pipe, &msg[offset], required);
+        if (newly == -1) {
+            /* error */
+            /*g_warning("read %d value error, indicator: %u", pipe, *indicator);*/
+            return newly;
+        }
+
+        required -= newly;
+        offset += newly;
+    }
+
+    /*g_warning("read %d ok indicator: %c len: %u msg: %s", pipe, *indicator, len, msg);*/
+    return len + 4;
+}
 
 void
 sync_pipe_capstart_to_parent(void)
 {
-    static const char capstart_msg = SP_CAPSTART;
+/*    static const char capstart_msg = SP_CAPSTART;
 
-    write(1, &capstart_msg, 1);
+    write(1, &capstart_msg, 1);*/
+
+    pipe_write_block(1, SP_CAPSTART, 0, NULL);
 }
 
 void
 sync_pipe_packet_count_to_parent(int packet_count)
 {
     char tmp[SP_DECISIZE+1+1];
-    sprintf(tmp, "%d%c", packet_count, SP_PACKET_COUNT);
-    write(1, tmp, strlen(tmp));
+
+
+    g_snprintf(tmp, SP_DECISIZE, "%d", packet_count);
+
+    pipe_write_block(1, SP_PACKET_COUNT, strlen(tmp)+1, tmp);
 }
 
 void
 sync_pipe_filename_to_parent(const char *filename)
 {
-    int msglen = strlen(filename);
-    char lenbuf[SP_DECISIZE+1+1];
-
-    sprintf(lenbuf, "%u%c", msglen, SP_FILE);
-    write(1, lenbuf, strlen(lenbuf));
-    write(1, filename, msglen);
+    pipe_write_block(1, SP_FILE, strlen(filename)+1, filename);
 }
 
 void
 sync_pipe_errmsg_to_parent(const char *errmsg)
 {
-    int msglen = strlen(errmsg);
-    char lenbuf[SP_DECISIZE+1+1];
-
-    sprintf(lenbuf, "%u%c", msglen, SP_ERROR_MSG);
-    write(1, lenbuf, strlen(lenbuf));
-    write(1, errmsg, msglen);
+    pipe_write_block(1, SP_ERROR_MSG, strlen(errmsg)+1, errmsg);
 }
 
 void
 sync_pipe_drops_to_parent(int drops)
 {
 	char tmp[SP_DECISIZE+1+1];
-	sprintf(tmp, "%d%c", drops, SP_DROPS);
-	write(1, tmp, strlen(tmp));
+
+
+    g_snprintf(tmp, SP_DROPS, "%d", drops);
+
+    pipe_write_block(1, SP_DROPS, strlen(tmp)+1, tmp);
 }
 
 
 #ifdef _WIN32
-#define SP_CAPEND 'q'
 
 static void
 signal_pipe_capend_to_child(capture_options *capture_opts)
 {
-    static const char capend_msg = SP_CAPEND;
 
-    write(capture_opts->signal_pipe_fd, &capend_msg, 1);
+
+    pipe_write_block(capture_opts->signal_pipe_fd, SP_CAPQUIT, 0, NULL);
 }
 #endif
 
@@ -346,7 +438,8 @@ sync_pipe_start(capture_options *capture_opts, gboolean is_tempfile) {
 
 #ifdef _WIN32
     /* Create a pipe for the child process */
-    if(_pipe(sync_pipe, 512, O_BINARY) < 0) {
+    /* (inrease this value if you have trouble while fast capture file switches) */
+    if(_pipe(sync_pipe, 5120, O_BINARY) < 0) {
       /* Couldn't create the pipe between parent and child. */
       simple_dialog(ESD_TYPE_ERROR, ESD_BTN_OK, "Couldn't create sync pipe: %s",
                         strerror(errno));
@@ -470,16 +563,6 @@ sync_pipe_start(capture_options *capture_opts, gboolean is_tempfile) {
     /* we might wait for a moment till child is ready, so update screen now */
     main_window_update();
 
-    /* the child have to send us a capture start or error message now */
-    if(!sync_pipe_input_wait_for_start(capture_opts, sync_pipe[PIPE_READ])) {
-	    /* Close the sync pipe. */
-	    close(sync_pipe[PIPE_READ]);
-#ifdef _WIN32
-        close(signal_pipe[PIPE_WRITE]);
-#endif
-        return FALSE;
-    }
-
     /* We were able to set up to read the capture file;
        arrange that our callback be called whenever it's possible
        to read from the sync pipe, so that it's called when
@@ -493,76 +576,6 @@ sync_pipe_start(capture_options *capture_opts, gboolean is_tempfile) {
 }
 
 
-/* capture prepared, waiting for the child to signal us capture has indeed started */
-static gboolean
-sync_pipe_input_wait_for_start(capture_options *capture_opts, int sync_pipe_read) {
-    guint   byte_count;
-    int     i;
-    guchar  c;
-    char    *msg;
-
-
-    /* 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)
-	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.");
-      } 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);
-	}
-
-      }
-      return FALSE;
-    }
-
-    return TRUE;
-}
-
-
 /* There's stuff to read from the sync pipe, meaning the child has sent
    us a message, or the sync pipe has closed, meaning the child has
    closed it (perhaps because it exited). */
@@ -571,12 +584,13 @@ sync_pipe_input_cb(gint source, gpointer user_data)
 {
   capture_options *capture_opts = (capture_options *)user_data;
 #define BUFSIZE	4096
-  char buffer[BUFSIZE+1], *p = buffer, *q = buffer, *msg, *r;
-  int  nread, msglen, chars_to_copy;
-  int  to_read = 0;
+  char buffer[BUFSIZE+1];
+  int  nread;
+  char indicator;
 
 
-  if ((nread = read(source, buffer, BUFSIZE)) <= 0) {
+  nread = pipe_read_block(source, &indicator, BUFSIZE, buffer);
+  if(nread <= 0) {
     /* The child has closed the sync pipe, meaning it's not going to be
        capturing any more packets.  Pick up its exit status, and
        complain if it did anything other than exit with status 0. */
@@ -588,108 +602,47 @@ sync_pipe_input_cb(gint source, gpointer user_data)
     return FALSE;
   }
 
-  buffer[nread] = '\0';
-
-  while (nread != 0) {
-    /* look for (possibly multiple) indications */
-    switch (*q) {
-    case SP_PACKET_COUNT :
-      to_read += atoi(p);
-      p = q + 1;
-      q++;
-      nread--;
-      break;
-    case SP_DROPS :
-      cf_set_drops_known(capture_opts->cf, TRUE);
-      cf_set_drops(capture_opts->cf, atoi(p));
-      p = q + 1;
-      q++;
-      nread--;
-      break;
-    case SP_ERROR_MSG :
-      msglen = atoi(p);
-      p = q + 1;
-      q++;
-      nread--;
-
-      /* Read the entire message.
-         XXX - if the child hasn't sent it all yet, this could cause us
-         to hang until they do. */
-      msg = g_malloc(msglen + 1);
-      r = msg;
-      while (msglen != 0) {
-      	if (nread == 0) {
-      	  /* Read more. */
-          if ((nread = read(source, buffer, BUFSIZE)) <= 0)
-            break;
-          p = buffer;
-          q = buffer;
-        }
-      	chars_to_copy = MIN(msglen, nread);
-        memcpy(r, q, chars_to_copy);
-        r += chars_to_copy;
-        q += chars_to_copy;
-        nread -= chars_to_copy;
-        msglen -= chars_to_copy;
-      }
-      *r = '\0';
-      simple_dialog(ESD_TYPE_ERROR, ESD_BTN_OK, msg);
-      g_free(msg);
-      break;
-    case SP_FILE :
-      msglen = atoi(p);
-      p = q + 1;
-      q++;
-      nread--;
-
-      /* Read the entire file name.
-         XXX - if the child hasn't sent it all yet, this could cause us
-         to hang until they do. */
-      msg = g_malloc(msglen + 1);
-      r = msg;
-      while (msglen != 0) {
-      	if (nread == 0) {
-      	  /* Read more. */
-          if ((nread = read(source, buffer, BUFSIZE)) <= 0)
-            break;
-          p = buffer;
-          q = buffer;
-        }
-      	chars_to_copy = MIN(msglen, nread);
-        memcpy(r, q, chars_to_copy);
-        r += chars_to_copy;
-        q += chars_to_copy;
-        nread -= chars_to_copy;
-        msglen -= chars_to_copy;
-      }
-      *r = '\0';
-
-      if(!capture_input_new_file(capture_opts, msg)) {
+  switch(indicator) {
+  case(SP_CAPSTART):
+    break;
+  case SP_PACKET_COUNT:
+    nread = atoi(buffer);
+    capture_input_new_packets(capture_opts, nread);
+    break;
+  case SP_DROPS:
+    cf_set_drops_known(capture_opts->cf, TRUE);
+    cf_set_drops(capture_opts->cf, atoi(buffer));
+    break;
+  case SP_ERROR_MSG:
+    simple_dialog(ESD_TYPE_ERROR, ESD_BTN_OK, buffer);
+    break;
+  case SP_FILE:
+      if(!capture_input_new_file(capture_opts, buffer)) {
          /* We weren't able to open the new capture file; user has been
             alerted. Close the sync pipe. */
-/*            close(sync_pipe[PIPE_READ]);*/
 
             /* XXX - how to kill things here ? */
             /* XXX - is it safe to close the pipe inside this callback? */
             close(source);
-      }
 
-      g_free(msg);
+            /* the child has send us a filename which we couldn't open.
+               this probably means, the child is creating files faster than we can handle it.
+               this should only be the case for very fast file switches
+               we can't do much more than telling the child to stop
+               (this is the emergency brake if user e.g. wants to switch files every second) */
+            sync_pipe_stop(capture_opts);
+      }
 
       break;
-    default :
-      q++;
-      nread--;
-      break;
-    }
+  default:
+      g_assert_not_reached();
   }
 
-  capture_input_new_packets(capture_opts, to_read);
-
   return TRUE;
 }
 
 
+
 /* the child process is going down, wait until it's completely terminated */
 static void
 sync_pipe_wait_for_child(capture_options *capture_opts, gboolean always_report)
diff --git a/file.c b/file.c
index e90ae240a3..4beb1f2065 100644
--- a/file.c
+++ b/file.c
@@ -557,6 +557,12 @@ cf_finish_tail(capture_file *cf, int *err)
   int         fd;
   struct stat cf_stat;
 
+
+  if(cf->wth == NULL) {
+    cf_close(cf);
+    return CF_READ_ERROR;
+  }
+
   packet_list_freeze();
 
   while ((wtap_read(cf->wth, err, &err_info, &data_offset))) {
diff --git a/gtk/capture_dlg.c b/gtk/capture_dlg.c
index 246ba922a4..470a22cf37 100644
--- a/gtk/capture_dlg.c
+++ b/gtk/capture_dlg.c
@@ -1417,8 +1417,8 @@ capture_prep_ok_cb(GtkWidget *ok_bt _U_, gpointer parent_w) {
     capture_opts->autostop_files =
       gtk_spin_button_get_value_as_int(GTK_SPIN_BUTTON(stop_files_sb));
 
-  if(capture_opts->real_time_mode)
-    capture_opts->multi_files_on = FALSE;
+/*  if(capture_opts->real_time_mode)
+    capture_opts->multi_files_on = FALSE;*/
 
   if (capture_opts->multi_files_on) {
     capture_opts->has_autostop_filesize =
@@ -1563,25 +1563,25 @@ capture_prep_adjust_sensitivity(GtkWidget *tb _U_, gpointer parent_w)
        mode off if it's on, and make its toggle button, and the spin
        button for the number of ring buffer files (and the spin button's
        label), insensitive. */
-    gtk_toggle_button_set_active(GTK_TOGGLE_BUTTON(multi_files_on_cb), FALSE);
-    gtk_widget_set_sensitive(GTK_WIDGET(multi_files_on_cb), FALSE);
+/*    gtk_toggle_button_set_active(GTK_TOGGLE_BUTTON(multi_files_on_cb), FALSE);
+    gtk_widget_set_sensitive(GTK_WIDGET(multi_files_on_cb), FALSE);*/
 
     /* Auto-scroll mode is meaningful only in "Update list of packets
        in real time" captures, so make its toggle button sensitive. */
     gtk_widget_set_sensitive(GTK_WIDGET(auto_scroll_cb), TRUE);
 
-    gtk_widget_set_sensitive(GTK_WIDGET(hide_info_cb), TRUE);
+    /*gtk_widget_set_sensitive(GTK_WIDGET(hide_info_cb), TRUE);*/
   } else {
     /* "Update list of packets in real time" captures disabled; that
        means ring buffer mode is OK, so make its toggle button
        sensitive. */
-    gtk_widget_set_sensitive(GTK_WIDGET(multi_files_on_cb), TRUE);
+/*    gtk_widget_set_sensitive(GTK_WIDGET(multi_files_on_cb), TRUE);*/
 
     /* Auto-scroll mode is meaningful only in "Update list of packets
        in real time" captures, so make its toggle button insensitive. */
     gtk_widget_set_sensitive(GTK_WIDGET(auto_scroll_cb), FALSE);
 
-    gtk_widget_set_sensitive(GTK_WIDGET(hide_info_cb), FALSE);
+    /*gtk_widget_set_sensitive(GTK_WIDGET(hide_info_cb), FALSE);*/
   }
 
   if (gtk_toggle_button_get_active(GTK_TOGGLE_BUTTON(multi_files_on_cb))) {
diff --git a/gtk/main.c b/gtk/main.c
index 647d9cf7cf..200017952b 100644
--- a/gtk/main.c
+++ b/gtk/main.c
@@ -1190,7 +1190,7 @@ set_display_filename(capture_file *cf)
 
   name_ptr = cf_get_display_name(cf);
 	
-  if (!cf->is_tempfile) {
+  if (!cf->is_tempfile && cf->filename) {
     /* Add this filename to the list of recent files in the "Recent Files" submenu */
     add_menu_recent_capture_file(cf->filename);
   }
@@ -2107,7 +2107,8 @@ main(int argc, char *argv[])
       }*/
       if (!capture_opts->has_autostop_filesize && !capture_opts->has_file_duration) {
 	fprintf(stderr, "ethereal: Ring buffer requested, but no maximum capture file size or duration were specified.\n");
-	capture_opts->multi_files_on = FALSE;
+/* XXX - this must be redesigned as the conditions changed */
+/*	capture_opts->multi_files_on = FALSE;*/
       }
     }
   }