Merge branch 'bug9781_v2'

changes/bug9781

@@ -0,0 +1,5 @@
+  o Minor bugfixes (tor-fw-helper):
+
+    - Give a correct log message when tor-fw-helper fails to launch.
+      (Previously, we would say something like "tor-fw-helper sent us a
+      string we could not parse".) Fixes bug 9781; bugfix on 0.2.4.2-alpha.

src/common/util.c

@@ -4540,6 +4540,30 @@ stream_status_to_string(enum stream_status stream_status)
   }
 }
 
+/* DOCDOC */
+static void
+log_portfw_spawn_error_message(const char *buf,
+                               const char *executable, int *child_status)
+{
+  /* Parse error message */
+  int retval, child_state, saved_errno;
+  retval = tor_sscanf(buf, SPAWN_ERROR_MESSAGE "%x/%x",
+                      &child_state, &saved_errno);
+  if (retval == 2) {
+    log_warn(LD_GENERAL,
+             "Failed to start child process \"%s\" in state %d: %s",
+             executable, child_state, strerror(saved_errno));
+    if (child_status)
+      *child_status = 1;
+  } else {
+    /* Failed to parse message from child process, log it as a
+       warning */
+    log_warn(LD_GENERAL,
+             "Unexpected message from port forwarding helper \"%s\": %s",
+             executable, buf);
+  }
+}
+
 #ifdef _WIN32
 
 /** Return a smartlist containing lines outputted from
@@ -4687,23 +4711,7 @@ log_from_pipe(FILE *stream, int severity, const char *executable,
 
     /* Check if buf starts with SPAWN_ERROR_MESSAGE */
     if (strcmpstart(buf, SPAWN_ERROR_MESSAGE) == 0) {
-      /* Parse error message */
+      log_portfw_spawn_error_message(buf, executable, child_status);
-      int retval, child_state, saved_errno;
-      retval = tor_sscanf(buf, SPAWN_ERROR_MESSAGE "%x/%x",
-                          &child_state, &saved_errno);
-      if (retval == 2) {
-        log_warn(LD_GENERAL,
-                 "Failed to start child process \"%s\" in state %d: %s",
-                 executable, child_state, strerror(saved_errno));
-        if (child_status)
-          *child_status = 1;
-      } else {
-        /* Failed to parse message from child process, log it as a
-           warning */
-        log_warn(LD_GENERAL,
-                 "Unexpected message from port forwarding helper \"%s\": %s",
-                 executable, buf);
-      }
     } else {
       log_fn(severity, LD_GENERAL, "Port forwarding helper says: %s", buf);
     }
@@ -4781,7 +4789,7 @@ get_string_from_pipe(FILE *stream, char *buf_out, size_t count)
 /** Parse a <b>line</b> from tor-fw-helper and issue an appropriate
  *  log message to our user. */
 static void
-handle_fw_helper_line(const char *line)
+handle_fw_helper_line(const char *executable, const char *line)
 {
   smartlist_t *tokens = smartlist_new();
   char *message = NULL;
@@ -4792,6 +4800,19 @@ handle_fw_helper_line(const char *line)
   int port = 0;
   int success = 0;
 
+  if (strcmpstart(line, SPAWN_ERROR_MESSAGE) == 0) {
+    /* We need to check for SPAWN_ERROR_MESSAGE again here, since it's
+     * possible that it got sent after we tried to read it in log_from_pipe.
+     *
+     * XXX Ideally, we should be using one of stdout/stderr for the real
+     * output, and one for the output of the startup code.  We used to do that
+     * before cd05f35d2c.
+     */
+    int child_status;
+    log_portfw_spawn_error_message(line, executable, &child_status);
+    goto done;
+  }
+
   smartlist_split_string(tokens, line, NULL,
                          SPLIT_SKIP_SPACE|SPLIT_IGNORE_BLANK, -1);
 
@@ -4871,7 +4892,7 @@ handle_fw_helper_line(const char *line)
 /** Read what tor-fw-helper has to say in its stdout and handle it
  *  appropriately */
 static int
-handle_fw_helper_output(process_handle_t *process_handle)
+handle_fw_helper_output(const char *executable, process_handle_t *process_handle)
 {
   smartlist_t *fw_helper_output = NULL;
   enum stream_status stream_status = 0;
@@ -4886,7 +4907,7 @@ handle_fw_helper_output(process_handle_t *process_handle)
 
   /* Handle the lines we got: */
   SMARTLIST_FOREACH_BEGIN(fw_helper_output, char *, line) {
-    handle_fw_helper_line(line);
+    handle_fw_helper_line(executable, line);
     tor_free(line);
   } SMARTLIST_FOREACH_END(line);
 
@@ -5001,7 +5022,7 @@ tor_check_port_forwarding(const char *filename,
     stderr_status = log_from_pipe(child_handle->stderr_handle,
                                   LOG_INFO, filename, &retval);
 #endif
-    if (handle_fw_helper_output(child_handle) < 0) {
+    if (handle_fw_helper_output(filename, child_handle) < 0) {
       log_warn(LD_GENERAL, "Failed to handle fw helper output.");
       stdout_status = -1;
       retval = -1;