fix bug #803: sync pipe on Win32 wasn't set to binary mode, so error message transpor...
authorulfl <ulfl@f5534014-38df-0310-8fa8-9805f1628bb7>
Mon, 13 Mar 2006 00:30:51 +0000 (00:30 +0000)
committerulfl <ulfl@f5534014-38df-0310-8fa8-9805f1628bb7>
Mon, 13 Mar 2006 00:30:51 +0000 (00:30 +0000)
I've also changed the way the secondary error message is transported from former "header message 0 secondary 0" to "header header message 0 header secondary 0" as that might be a bit more clearer, and I'll need it for further development anyway.

I was using this while debugging and not recognizing the real problem - for about four hours :-(. I'll need this feature when doing the interface (and link layer type) browsing later (transferring this data from dumpcap to Ethereal) to get a full blown privilege seperation.

git-svn-id: http://anonsvn.wireshark.org/wireshark/trunk@17608 f5534014-38df-0310-8fa8-9805f1628bb7

capture.c
capture.h
capture_loop.c
capture_sync.c
dumpcap.c

index 4ddf910acda88dd071649be04fb15f614be2d6ee..2285299be588c3df5dec334049f5ba72c26b45d7 100644 (file)
--- a/capture.c
+++ b/capture.c
@@ -374,9 +374,8 @@ capture_input_drops(capture_options *capture_opts, int dropped)
    The secondary message might be a null string.
  */
 void
-capture_input_error_message(capture_options *capture_opts, char *error_msg)
+capture_input_error_message(capture_options *capture_opts, char *error_msg, char *secondary_error_msg)
 {
-  char *secondary_error_msg = strchr(error_msg, '\0') + 1;
   gchar *safe_error_msg;
   gchar *safe_secondary_error_msg;
 
index f36dd50348eb11bfb8c5e31532d0fd1a6b5b4821..0bc6ed75e629721cf796e43e8f04e7ea4b2bf62a 100644 (file)
--- a/capture.h
+++ b/capture.h
@@ -68,7 +68,7 @@ extern void capture_input_drops(capture_options *capture_opts, int dropped);
 /**
  * Capture child told us that an error has occurred while starting the capture.
  */
-extern void capture_input_error_message(capture_options *capture_opts, char *error_message);
+extern void capture_input_error_message(capture_options *capture_opts, char *error_message, char *secondary_error_msg);
 
 /**
  * Capture child told us that an error has occurred while parsing a
index a908c5b1a790631c4c92a60c0b492dbefabe53f7..f3c8564167dfc2dce0f4bfe52b0ee665a378014d 100644 (file)
@@ -556,7 +556,8 @@ capture_loop_open_input(capture_options *capture_opts, loop_data *ld,
        Do, however, warn about the lack of 64-bit support, and warn that
        WAN devices aren't supported. */
     g_snprintf(errmsg, errmsg_len,
-"The capture session could not be initiated (%s).",
+"The capture session could not be initiated.\n"
+"\"%s\"",
                open_err_str);
     g_snprintf(secondary_errmsg, secondary_errmsg_len,
 "\n"
index 39bf34f18f55926e98893b620309a7bb7f37c1c6..631f870df63132723d8b8642897f5b380fc686b0 100644 (file)
@@ -215,34 +215,24 @@ signal_pipe_capquit_to_child(capture_options *capture_opts)
 }
 #endif
 
-
-
-/* read a message from the sending pipe in the standard format
-   (1-byte message indicator, 3-byte message length (excluding length
-   and indicator field), and the rest is the message) */
 static int
-pipe_read_block(int pipe, char *indicator, int len, char *msg) {
-    int required;
+pipe_read_bytes(int pipe, char *bytes, int required) {
     int newly;
-    guchar header[4];
-    int offset;
+    int offset = 0;
 
 
-    /* read header (indicator and 3-byte length) */
-    required = 4;
-    offset = 0;
     while(required) {
-        newly = read(pipe, &header[offset], required);
+        newly = read(pipe, &bytes[offset], required);
         if (newly == 0) {
             /* EOF */
             g_log(LOG_DOMAIN_CAPTURE, G_LOG_LEVEL_DEBUG,
-                  "read %d header empty (capture closed)", pipe);
+                  "read from pipe %d: EOF (capture closed?)", pipe);
             return newly;
         }
         if (newly < 0) {
             /* error */
             g_log(LOG_DOMAIN_CAPTURE, G_LOG_LEVEL_DEBUG,
-                  "read %d header error: %s", pipe, strerror(errno));
+                  "read from pipe %d: error(%u): %s", pipe, errno, strerror(errno));
             return newly;
         }
 
@@ -250,9 +240,40 @@ pipe_read_block(int pipe, char *indicator, int len, char *msg) {
         offset += newly;
     }
 
+    return newly;
+}
+
+/* convert header values (indicator and 4-byte length) */
+static void
+pipe_convert_header(const guchar *header, int header_len, char *indicator, int *block_len) {    
+
+    g_assert(header_len == 4);
+
     /* convert header values */
     *indicator = header[0];
-    required = header[1]<<16 | header[2]<<8 | header[3];
+    *block_len = header[1]<<16 | header[2]<<8 | header[3];
+}
+
+/* read a message from the sending pipe in the standard format
+   (1-byte message indicator, 3-byte message length (excluding length
+   and indicator field), and the rest is the message) */
+static int
+pipe_read_block(int pipe, char *indicator, int len, char *msg) {
+    int required;
+    int newly;
+    guchar header[4];
+
+
+    /* read header (indicator and 3-byte length) */
+    newly = pipe_read_bytes(pipe, header, 4);
+    if(newly != 4) {
+        g_log(LOG_DOMAIN_CAPTURE, G_LOG_LEVEL_DEBUG,
+              "read %d failed to read header: %u", pipe, newly);
+        return -1;
+    }
+
+    /* convert header values */
+    pipe_convert_header(header, 4, indicator, &required);
 
     /* only indicator with no value? */
     if(required == 0) {
@@ -261,6 +282,7 @@ pipe_read_block(int pipe, char *indicator, int len, char *msg) {
         return 4;
     }
 
+    /* does the data fit into the given buffer? */
     if(required > len) {
         g_log(LOG_DOMAIN_CAPTURE, G_LOG_LEVEL_DEBUG,
               "read %d length error, required %d > len %d, indicator: %u",
@@ -274,26 +296,17 @@ pipe_read_block(int pipe, char *indicator, int len, char *msg) {
     }
     len = required;
 
-    /* read value */
-    offset = 0;
-    while(required) {
-        newly = read(pipe, &msg[offset], required);
-        if (newly == -1) {
-            /* error */
-            g_log(LOG_DOMAIN_CAPTURE, G_LOG_LEVEL_DEBUG,
-                  "read %d value error: %s, indicator: %u", pipe,
-                  strerror(errno), *indicator);
-            return newly;
-        }
-
-        required -= newly;
-        offset += newly;
+    /* read the actual block data */
+    newly = pipe_read_bytes(pipe, msg, required);
+    if(newly != required) {
+        g_warning("Unknown message from dumpcap, try to show it as a string: %s", msg);
+        return -1;
     }
 
     g_log(LOG_DOMAIN_CAPTURE, G_LOG_LEVEL_DEBUG,
           "read %d ok indicator: %c len: %u msg: %s", pipe, *indicator,
           len, msg);
-    return len + 4;
+    return newly + 4;
 }
 
 
@@ -633,7 +646,7 @@ sync_pipe_start(capture_options *capture_opts) {
       execv(exename, argv);
       g_snprintf(errmsg, sizeof errmsg, "Couldn't run %s in child process: %s",
                exename, strerror(errno));
-      sync_pipe_errmsg_to_parent(errmsg, NULL);
+      sync_pipe_errmsg_to_parent(errmsg, "");
 
       /* Exit with "_exit()", so that we don't close the connection
          to the X server (and cause stuff buffered up by our parent but
@@ -697,6 +710,10 @@ sync_pipe_input_cb(gint source, gpointer user_data)
   char buffer[SP_MAX_MSG_LEN+1];
   int  nread;
   char indicator;
+  int  primary_len;
+  char * primary_msg;
+  int  secondary_len;
+  char * secondary_msg;
 
 
   nread = pipe_read_block(source, &indicator, SP_MAX_MSG_LEN, buffer);
@@ -746,8 +763,16 @@ sync_pipe_input_cb(gint source, gpointer user_data)
     capture_input_new_packets(capture_opts, nread);
     break;
   case SP_ERROR_MSG:
-    capture_input_error_message(capture_opts, buffer);
+    /* convert primary message */
+    pipe_convert_header(buffer, 4, &indicator, &primary_len);
+    primary_msg = buffer+4;
+    /* convert secondary message */
+    pipe_convert_header(primary_msg + primary_len, 4, &indicator, &secondary_len);
+    secondary_msg = primary_msg + primary_len + 4;
+    /* message output */
+    capture_input_error_message(capture_opts, primary_msg, secondary_msg);
     /* the capture child will close the sync_pipe, nothing to do for now */
+    /* (an error message doesn't mean we have to stop capturing) */
     break;
   case SP_BAD_FILTER:
     capture_input_cfilter_error_message(capture_opts, buffer);
index 268f32615e52dc8ab4228c0680ebd54ea6691f73..a9724861410debf92061b06c1895a277efcc15d5 100644 (file)
--- a/dumpcap.c
+++ b/dumpcap.c
@@ -337,6 +337,10 @@ main(int argc, char *argv[])
       /*** hidden option: Ethereal child mode (using binary output messages) ***/
       case 'Z':
           capture_child = TRUE;
+#ifdef _WIN32
+          /* set output pipe to binary mode, to avoid ugly text conversions */
+                 _setmode(1, O_BINARY);
+#endif
           break;
 
       /*** all non capture option specific ***/
@@ -505,51 +509,51 @@ console_log_handler(const char *log_domain, GLogLevelFlags log_level,
 /*
  * Maximum length of sync pipe message data.  Must be < 2^24, as the
  * message length is 3 bytes.
- * XXX - this must be large enough to handle a Really Big Filter
+ * XXX - this should be large enough to handle a Really Big Filter
  * Expression, as the error message for an incorrect filter expression
  * is a bit larger than the filter expression.
  */
 #define SP_MAX_MSG_LEN 4096
 
+/* write a single message header to the recipient pipe */
+static int
+pipe_write_header(int pipe, char indicator, int length)
+{
+    guchar header[1+3]; /* indicator + 3-byte len */
+
+
+    g_assert(length <= SP_MAX_MSG_LEN);
+
+    /* write header (indicator + 3-byte len) */
+    header[0] = indicator;
+    header[1] = (length >> 16) & 0xFF;
+    header[2] = (length >> 8) & 0xFF;
+    header[3] = (length >> 0) & 0xFF;
+
+    /* write header */
+    return write(pipe, header, sizeof header);
+}
 
 /* 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).
-   If msg is NULL, the message has only a length and indicator.
-   Otherwise, if secondary_msg isn't NULL, send both msg and
-   secondary_msg as null-terminated strings, otherwise just send
-   msg as a null-terminated string and follow it with an empty string. */
+   If msg is NULL, the message has only a length and indicator. */
 static void
-pipe_write_block(int pipe, char indicator, const char *msg,
-                 const char *secondary_msg)
+pipe_write_block(int pipe, char indicator, const char *msg)
 {
-    guchar header[3+1]; /* indicator + 3-byte len */
     int ret;
-    size_t len, secondary_len, total_len;
+    size_t len;
 
     /*g_warning("write %d enter", pipe);*/
 
-    len = 0;
-    secondary_len = 0;
-    total_len = 0;
     if(msg != NULL) {
-      len = strlen(msg) + 1;   /* include the terminating '\0' */
-      total_len = len;
-      if(secondary_msg == NULL)
-        secondary_msg = "";    /* default to an empty string */
-      secondary_len = strlen(secondary_msg) + 1;
-      total_len += secondary_len;
+        len = strlen(msg) + 1;    /* including the terminating '\0'! */
+    } else {
+        len = 0;
     }
-    g_assert(indicator < '0' || indicator > '9');
-    g_assert(total_len <= SP_MAX_MSG_LEN);
 
     /* write header (indicator + 3-byte len) */
-    header[0] = indicator;
-    header[1] = (total_len >> 16) & 0xFF;
-    header[2] = (total_len >> 8) & 0xFF;
-    header[3] = (total_len >> 0) & 0xFF;
-
-    ret = write(pipe, header, sizeof header);
+    ret = pipe_write_header(pipe, indicator, len);
     if(ret == -1) {
         return;
     }
@@ -561,10 +565,6 @@ pipe_write_block(int pipe, char indicator, const char *msg,
         if(ret == -1) {
             return;
         }
-        ret = write(pipe, secondary_msg, secondary_len);
-        if(ret == -1) {
-            return;
-        }
     } else {
         /*g_warning("write %d indicator: %c no value", pipe, indicator);*/
     }
@@ -583,7 +583,7 @@ sync_pipe_packet_count_to_parent(int packet_count)
     if(capture_child) {
         g_snprintf(tmp, sizeof(tmp), "%d", packet_count);
         g_log(LOG_DOMAIN_CAPTURE_CHILD, G_LOG_LEVEL_DEBUG, "Packets: %s", tmp);
-        pipe_write_block(1, SP_PACKET_COUNT, tmp, NULL);
+        pipe_write_block(1, SP_PACKET_COUNT, tmp);
     } else {
         count += packet_count;
         fprintf(stderr, "\rPackets: %u ", count);
@@ -600,7 +600,7 @@ sync_pipe_filename_to_parent(const char *filename)
     g_log(LOG_DOMAIN_CAPTURE_CHILD, G_LOG_LEVEL_MESSAGE, "File: %s", filename);
 
     if(capture_child) {
-        pipe_write_block(1, SP_FILE, filename, NULL);
+        pipe_write_block(1, SP_FILE, filename);
     }
 }
 
@@ -612,7 +612,7 @@ sync_pipe_cfilter_error_to_parent(const char *cfilter _U_, const char *errmsg)
     g_log(LOG_DOMAIN_CAPTURE_CHILD, G_LOG_LEVEL_DEBUG, "Capture filter error: %s", errmsg);
 
     if (capture_child) {
-        pipe_write_block(1, SP_BAD_FILTER, errmsg, NULL);
+        pipe_write_block(1, SP_BAD_FILTER, errmsg);
     }
 }
 
@@ -620,13 +620,16 @@ void
 sync_pipe_errmsg_to_parent(const char *error_msg, const char *secondary_error_msg)
 {
 
-    g_log(LOG_DOMAIN_CAPTURE_CHILD, G_LOG_LEVEL_MESSAGE, "Error: %s", error_msg);
-    if (secondary_error_msg != NULL)
-        g_log(LOG_DOMAIN_CAPTURE_CHILD, G_LOG_LEVEL_MESSAGE, "Secondary error: %s",
-              secondary_error_msg);
+    g_log(LOG_DOMAIN_CAPTURE_CHILD, G_LOG_LEVEL_MESSAGE, 
+        "Primary Error: %s", error_msg);
+    g_log(LOG_DOMAIN_CAPTURE_CHILD, G_LOG_LEVEL_MESSAGE, 
+        "Secondary Error: %s", secondary_error_msg);
 
     if(capture_child) {
-        pipe_write_block(1, SP_ERROR_MSG, error_msg, secondary_error_msg);
+        /* first write a "master header" with the length of the two messages plus their "slave headers" */
+        pipe_write_header(1, SP_ERROR_MSG, strlen(error_msg) + 1 + 4 + strlen(secondary_error_msg) + 1 + 4);
+        pipe_write_block(1, SP_ERROR_MSG, error_msg);
+        pipe_write_block(1, SP_ERROR_MSG, secondary_error_msg);
     }
 }
 
@@ -640,7 +643,7 @@ sync_pipe_drops_to_parent(int drops)
     g_log(LOG_DOMAIN_CAPTURE_CHILD, G_LOG_LEVEL_MESSAGE, "Packets dropped: %s", tmp);
 
     if(capture_child) {
-        pipe_write_block(1, SP_DROPS, tmp, NULL);
+        pipe_write_block(1, SP_DROPS, tmp);
     }
 }