Fix #157255. Also some refactoring of this still very ugly source file.
authorTor Lillqvist <tml@iki.fi>
Sat, 11 Dec 2004 03:47:32 +0000 (03:47 +0000)
committerTor Lillqvist <tml@src.gnome.org>
Sat, 11 Dec 2004 03:47:32 +0000 (03:47 +0000)
2004-12-11  Tor Lillqvist  <tml@iki.fi>

* glib/gspawn-win32.c: Fix #157255. Also some refactoring of this
still very ugly source file.

ChangeLog
ChangeLog.pre-2-10
ChangeLog.pre-2-12
ChangeLog.pre-2-6
ChangeLog.pre-2-8
glib/gspawn-win32.c

index 090d0c6..323cbf4 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@
+2004-12-11  Tor Lillqvist  <tml@iki.fi>
+
+       * glib/gspawn-win32.c: Fix #157255. Also some refactoring of this
+       still very ugly source file.
+
 2004-12-09  Matthias Clasen  <mclasen@redhat.com>
 
        * glib/goption.c (print_help): Don't print help options
index 090d0c6..323cbf4 100644 (file)
@@ -1,3 +1,8 @@
+2004-12-11  Tor Lillqvist  <tml@iki.fi>
+
+       * glib/gspawn-win32.c: Fix #157255. Also some refactoring of this
+       still very ugly source file.
+
 2004-12-09  Matthias Clasen  <mclasen@redhat.com>
 
        * glib/goption.c (print_help): Don't print help options
index 090d0c6..323cbf4 100644 (file)
@@ -1,3 +1,8 @@
+2004-12-11  Tor Lillqvist  <tml@iki.fi>
+
+       * glib/gspawn-win32.c: Fix #157255. Also some refactoring of this
+       still very ugly source file.
+
 2004-12-09  Matthias Clasen  <mclasen@redhat.com>
 
        * glib/goption.c (print_help): Don't print help options
index 090d0c6..323cbf4 100644 (file)
@@ -1,3 +1,8 @@
+2004-12-11  Tor Lillqvist  <tml@iki.fi>
+
+       * glib/gspawn-win32.c: Fix #157255. Also some refactoring of this
+       still very ugly source file.
+
 2004-12-09  Matthias Clasen  <mclasen@redhat.com>
 
        * glib/goption.c (print_help): Don't print help options
index 090d0c6..323cbf4 100644 (file)
@@ -1,3 +1,8 @@
+2004-12-11  Tor Lillqvist  <tml@iki.fi>
+
+       * glib/gspawn-win32.c: Fix #157255. Also some refactoring of this
+       still very ugly source file.
+
 2004-12-09  Matthias Clasen  <mclasen@redhat.com>
 
        * glib/goption.c (print_help): Don't print help options
index 70eaaa8..083c32f 100644 (file)
@@ -64,7 +64,6 @@
 #ifdef G_SPAWN_WIN32_DEBUG
   static int debug = 1;
   #define SETUP_DEBUG() /* empty */
-
 #else
   static int debug = -1;
   #define SETUP_DEBUG()                                        \
@@ -101,11 +100,6 @@ enum {
   ARG_COUNT = ARG_PROGRAM
 };
 
-/* Return codes from do_spawn() */
-#define DO_SPAWN_ERROR_HELPER -1
-#define DO_SPAWN_OK_NO_HELPER -2
-#define DO_SPAWN_ERROR_NO_HELPER -3
-
 static gint
 protect_argv (gchar  **argv,
              gchar ***new_argv)
@@ -186,27 +180,7 @@ protect_argv (gchar  **argv,
 
 #ifndef GSPAWN_HELPER
 
-static gboolean make_pipe            (gint                  p[2],
-                                      GError              **error);
-static gboolean do_spawn_with_pipes  (gboolean              dont_wait,
-                                     gboolean              dont_return_handle,
-                                     const gchar          *working_directory,
-                                      gchar               **argv,
-                                      gchar               **envp,
-                                      gboolean              close_descriptors,
-                                      gboolean              search_path,
-                                      gboolean              stdout_to_null,
-                                      gboolean              stderr_to_null,
-                                      gboolean              child_inherits_stdin,
-                                     gboolean              file_and_argv_zero,
-                                      GSpawnChildSetupFunc  child_setup,
-                                      gpointer              user_data,
-                                      GPid                 *child_handle,
-                                      gint                 *standard_input,
-                                      gint                 *standard_output,
-                                      gint                 *standard_error,
-                                     gint                 *exit_status,
-                                      GError              **error);
+#define HELPER_PROCESS "gspawn-win32-helper"
 
 GQuark
 g_spawn_error_quark (void)
@@ -243,20 +217,14 @@ g_spawn_async (const gchar          *working_directory,
  * on a file descriptor twice, and another thread has
  * re-opened it since the first close)
  */
-static gint
+static void
 close_and_invalidate (gint *fd)
 {
-  gint ret;
-
   if (*fd < 0)
-    return -1;
-  else
-    {
-      ret = close (*fd);
-      *fd = -1;
-    }
+    return;
 
-  return ret;
+  close (*fd);
+  *fd = -1;
 }
 
 typedef enum
@@ -301,413 +269,221 @@ read_data (GString     *str,
     return READ_OK;
 }
 
-gboolean
-g_spawn_sync (const gchar          *working_directory,
-              gchar               **argv,
-              gchar               **envp,
-              GSpawnFlags           flags,
-              GSpawnChildSetupFunc  child_setup,
-              gpointer              user_data,
-              gchar               **standard_output,
-              gchar               **standard_error,
-              gint                 *exit_status,
-              GError              **error)     
+static gboolean
+make_pipe (gint     p[2],
+           GError **error)
 {
-  gint outpipe = -1;
-  gint errpipe = -1;
-  GPid pid;
-  GIOChannel *outchannel = NULL;
-  GIOChannel *errchannel = NULL;
-  GPollFD outfd, errfd;
-  GPollFD fds[2];
-  gint nfds;
-  gint outindex = -1;
-  gint errindex = -1;
-  gint ret;
-  GString *outstr = NULL;
-  GString *errstr = NULL;
-  gboolean failed;
-  gint status;
-  
-  g_return_val_if_fail (argv != NULL, FALSE);
-  g_return_val_if_fail (!(flags & G_SPAWN_DO_NOT_REAP_CHILD), FALSE);
-  g_return_val_if_fail (standard_output == NULL ||
-                        !(flags & G_SPAWN_STDOUT_TO_DEV_NULL), FALSE);
-  g_return_val_if_fail (standard_error == NULL ||
-                        !(flags & G_SPAWN_STDERR_TO_DEV_NULL), FALSE);
-  
-  /* Just to ensure segfaults if callers try to use
-   * these when an error is reported.
-   */
-  if (standard_output)
-    *standard_output = NULL;
-
-  if (standard_error)
-    *standard_error = NULL;
-  
-  if (!do_spawn_with_pipes (FALSE,
-                           TRUE,
-                           working_directory,
-                           argv,
-                           envp,
-                           !(flags & G_SPAWN_LEAVE_DESCRIPTORS_OPEN),
-                           (flags & G_SPAWN_SEARCH_PATH) != 0,
-                           (flags & G_SPAWN_STDOUT_TO_DEV_NULL) != 0,
-                           (flags & G_SPAWN_STDERR_TO_DEV_NULL) != 0,
-                           (flags & G_SPAWN_CHILD_INHERITS_STDIN) != 0,
-                           (flags & G_SPAWN_FILE_AND_ARGV_ZERO) != 0,
-                           child_setup,
-                           user_data,
-                           &pid,
-                           NULL,
-                           standard_output ? &outpipe : NULL,
-                           standard_error ? &errpipe : NULL,
-                           &status,
-                           error))
-    return FALSE;
-
-  /* Read data from child. */
-  
-  failed = FALSE;
-
-  if (outpipe >= 0)
-    {
-      outstr = g_string_new (NULL);
-      outchannel = g_io_channel_win32_new_fd (outpipe);
-      g_io_channel_set_encoding (outchannel, NULL, NULL);
-      g_io_channel_win32_make_pollfd (outchannel,
-                                     G_IO_IN | G_IO_ERR | G_IO_HUP,
-                                     &outfd);
-    }
-      
-  if (errpipe >= 0)
+  if (pipe (p) < 0)
     {
-      errstr = g_string_new (NULL);
-      errchannel = g_io_channel_win32_new_fd (errpipe);
-      g_io_channel_set_encoding (errchannel, NULL, NULL);
-      g_io_channel_win32_make_pollfd (errchannel,
-                                     G_IO_IN | G_IO_ERR | G_IO_HUP,
-                                     &errfd);
+      g_set_error (error,
+                   G_SPAWN_ERROR,
+                   G_SPAWN_ERROR_FAILED,
+                   _("Failed to create pipe for communicating with child process (%s)"),
+                   g_strerror (errno));
+      return FALSE;
     }
+  else
+    return TRUE;
+}
 
-  /* Read data until we get EOF on both pipes. */
-  while (!failed &&
-         (outpipe >= 0 ||
-          errpipe >= 0))
+/* The helper process writes a status report back to us, through a
+ * pipe, consisting of two ints.
+ */
+static gboolean
+read_helper_report (int      fd,
+                   gint     report[2],
+                   GError **error)
+{
+  gint bytes = 0;
+  
+  while (bytes < sizeof(gint)*2)
     {
-      nfds = 0;
-      if (outpipe >= 0)
-       {
-         fds[nfds] = outfd;
-         outindex = nfds;
-         nfds++;
-       }
-      if (errpipe >= 0)
-       {
-         fds[nfds] = errfd;
-         errindex = nfds;
-         nfds++;
-       }
+      gint chunk;
 
       if (debug)
-       g_print ("%s:g_spawn_sync: calling g_io_channel_win32_poll, nfds=%d\n",
-                __FILE__, nfds);
+       g_print ("%s:read_helper_report: read %d...\n",
+                __FILE__,
+                sizeof(gint)*2 - bytes);
 
-      ret = g_io_channel_win32_poll (fds, nfds, -1);
+      chunk = read (fd, ((gchar*)report) + bytes,
+                   sizeof(gint)*2 - bytes);
 
-      if (ret < 0)
+      if (debug)
+       g_print ("...got %d bytes\n", chunk);
+          
+      if (chunk < 0)
         {
-          failed = TRUE;
-
+          /* Some weird shit happened, bail out */
+              
           g_set_error (error,
                        G_SPAWN_ERROR,
-                       G_SPAWN_ERROR_READ,
-                       _("Unexpected error in g_io_channel_win32_poll() reading data from a child process"));
-              
-          break;
-        }
-
-      if (outpipe >= 0 && (fds[outindex].revents & G_IO_IN))
-        {
-          switch (read_data (outstr, outchannel, error))
-            {
-            case READ_FAILED:
-             if (debug)
-               g_print ("g_spawn_sync: outchannel: READ_FAILED\n");
-              failed = TRUE;
-              break;
-            case READ_EOF:
-             if (debug)
-               g_print ("g_spawn_sync: outchannel: READ_EOF\n");
-              g_io_channel_unref (outchannel);
-             outchannel = NULL;
-              close_and_invalidate (&outpipe);
-              break;
-            default:
-             if (debug)
-               g_print ("g_spawn_sync: outchannel: OK\n");
-              break;
-            }
+                       G_SPAWN_ERROR_FAILED,
+                       _("Failed to read from child pipe (%s)"),
+                       g_strerror (errno));
 
-          if (failed)
-            break;
+          return FALSE;
         }
+      else if (chunk == 0)
+        break; /* EOF */
+      else
+       bytes += chunk;
+    }
 
-      if (errpipe >= 0 && (fds[errindex].revents & G_IO_IN))
-        {
-          switch (read_data (errstr, errchannel, error))
-            {
-            case READ_FAILED:
-             if (debug)
-               g_print ("g_spawn_sync: errchannel: READ_FAILED\n");
-              failed = TRUE;
-              break;
-            case READ_EOF:
-             if (debug)
-               g_print ("g_spawn_sync: errchannel: READ_EOF\n");
-             g_io_channel_unref (errchannel);
-             errchannel = NULL;
-              close_and_invalidate (&errpipe);
-              break;
-            default:
-             if (debug)
-               g_print ("g_spawn_sync: errchannel: OK\n");
-              break;
-            }
+  if (bytes < sizeof(gint)*2)
+    return FALSE;
 
-          if (failed)
-            break;
-        }
+  return TRUE;
+}
+
+static void
+set_child_error (gint         report[2],
+                const gchar *working_directory,
+                GError     **error)
+{
+  switch (report[0])
+    {
+    case CHILD_CHDIR_FAILED:
+      g_set_error (error,
+                  G_SPAWN_ERROR,
+                  G_SPAWN_ERROR_CHDIR,
+                  _("Failed to change to directory '%s' (%s)"),
+                  working_directory,
+                  g_strerror (report[1]));
+      break;
+    case CHILD_SPAWN_FAILED:
+      g_set_error (error,
+                  G_SPAWN_ERROR,
+                  G_SPAWN_ERROR_FAILED,
+                  _("Failed to execute child process (%s)"),
+                  g_strerror (report[1]));
+      break;
+    default:
+      g_assert_not_reached ();
     }
+}
 
-  /* These should only be open still if we had an error.  */
-  
-  if (outchannel != NULL)
-    g_io_channel_unref (outchannel);
-  if (errchannel != NULL)
-    g_io_channel_unref (errchannel);
-  if (outpipe >= 0)
-    close_and_invalidate (&outpipe);
-  if (errpipe >= 0)
-    close_and_invalidate (&errpipe);
+static gboolean
+do_spawn_with_pipes (gboolean              dont_wait,
+                    gboolean              dont_return_handle,
+                    const gchar          *working_directory,
+                    gchar               **argv,
+                    char                **envp,
+                    GSpawnFlags           flags,
+                    GSpawnChildSetupFunc  child_setup,
+                    gpointer              user_data,
+                    GPid                 *child_handle,
+                    gint                 *standard_input,
+                    gint                 *standard_output,
+                    gint                 *standard_error,
+                    gint                 *exit_status,
+                    gint                 *err_report,
+                    GError              **error)     
+{
+  char **protected_argv;
+  char args[ARG_COUNT][10];
+  char **new_argv;
+  int i;
+  int rc = -1;
+  int saved_errno;
+  int argc;
+  int stdin_pipe[2] = { -1, -1 };
+  int stdout_pipe[2] = { -1, -1 };
+  int stderr_pipe[2] = { -1, -1 };
+  int child_err_report_pipe[2] = { -1, -1 };
+  int helper_report[2];
   
-  g_spawn_close_pid(pid);
+  SETUP_DEBUG();
 
-  if (failed)
-    {
-      if (outstr)
-        g_string_free (outstr, TRUE);
-      if (errstr)
-        g_string_free (errstr, TRUE);
+  argc = protect_argv (argv, &protected_argv);
 
-      return FALSE;
-    }
-  else
+  if (!standard_input && !standard_output && !standard_error &&
+      (flags & G_SPAWN_CHILD_INHERITS_STDIN) &&
+      !(flags & G_SPAWN_STDOUT_TO_DEV_NULL) &&
+      !(flags & G_SPAWN_STDERR_TO_DEV_NULL) &&
+      (working_directory == NULL || !*working_directory) &&
+      (flags & G_SPAWN_LEAVE_DESCRIPTORS_OPEN))
     {
-      if (exit_status)
-        *exit_status = status;
-      
-      if (standard_output)        
-        *standard_output = g_string_free (outstr, FALSE);
-
-      if (standard_error)
-        *standard_error = g_string_free (errstr, FALSE);
-
-      return TRUE;
-    }
-}
-
-gboolean
-g_spawn_async_with_pipes (const gchar          *working_directory,
-                          gchar               **argv,
-                          gchar               **envp,
-                          GSpawnFlags           flags,
-                          GSpawnChildSetupFunc  child_setup,
-                          gpointer              user_data,
-                          GPid                 *child_handle,
-                          gint                 *standard_input,
-                          gint                 *standard_output,
-                          gint                 *standard_error,
-                          GError              **error)
-{
-  g_return_val_if_fail (argv != NULL, FALSE);
-  g_return_val_if_fail (standard_output == NULL ||
-                        !(flags & G_SPAWN_STDOUT_TO_DEV_NULL), FALSE);
-  g_return_val_if_fail (standard_error == NULL ||
-                        !(flags & G_SPAWN_STDERR_TO_DEV_NULL), FALSE);
-  /* can't inherit stdin if we have an input pipe. */
-  g_return_val_if_fail (standard_input == NULL ||
-                        !(flags & G_SPAWN_CHILD_INHERITS_STDIN), FALSE);
-  
-  return do_spawn_with_pipes (TRUE,
-                             !(flags & G_SPAWN_DO_NOT_REAP_CHILD),
-                             working_directory,
-                             argv,
-                             envp,
-                             !(flags & G_SPAWN_LEAVE_DESCRIPTORS_OPEN),
-                             (flags & G_SPAWN_SEARCH_PATH) != 0,
-                             (flags & G_SPAWN_STDOUT_TO_DEV_NULL) != 0,
-                             (flags & G_SPAWN_STDERR_TO_DEV_NULL) != 0,
-                             (flags & G_SPAWN_CHILD_INHERITS_STDIN) != 0,
-                             (flags & G_SPAWN_FILE_AND_ARGV_ZERO) != 0,
-                             child_setup,
-                             user_data,
-                             child_handle,
-                             standard_input,
-                             standard_output,
-                             standard_error,
-                             NULL,
-                             error);
-}
-
-gboolean
-g_spawn_command_line_sync (const gchar  *command_line,
-                           gchar       **standard_output,
-                           gchar       **standard_error,
-                           gint         *exit_status,
-                           GError      **error)
-{
-  gboolean retval;
-  gchar **argv = 0;
-
-  g_return_val_if_fail (command_line != NULL, FALSE);
-  
-  if (!g_shell_parse_argv (command_line,
-                           NULL, &argv,
-                           error))
-    return FALSE;
-  
-  retval = g_spawn_sync (NULL,
-                         argv,
-                         NULL,
-                         G_SPAWN_SEARCH_PATH,
-                         NULL,
-                         NULL,
-                         standard_output,
-                         standard_error,
-                         exit_status,
-                         error);
-  g_strfreev (argv);
-
-  return retval;
-}
-
-gboolean
-g_spawn_command_line_async (const gchar *command_line,
-                            GError     **error)
-{
-  gboolean retval;
-  gchar **argv = 0;
-
-  g_return_val_if_fail (command_line != NULL, FALSE);
-
-  if (!g_shell_parse_argv (command_line,
-                           NULL, &argv,
-                           error))
-    return FALSE;
-  
-  retval = g_spawn_async (NULL,
-                          argv,
-                          NULL,
-                          G_SPAWN_SEARCH_PATH,
-                          NULL,
-                          NULL,
-                          NULL,
-                          error);
-  g_strfreev (argv);
-
-  return retval;
-}
-
-/* This stinks, code reorg needed. Presumably do_spawn() should be
- * inserted into its only caller, do_spawn_with_pipes(), and then code
- * snippets from that function should be split out to separate
- * functions if necessary.
- */
-
-static gint shortcut_spawn_retval;
-
-static gint
-do_spawn (gboolean              dont_wait,
-         gint                  child_err_report_fd,
-         gint                  stdin_fd,
-         gint                  stdout_fd,
-         gint                  stderr_fd,
-         const gchar          *working_directory,
-         gchar               **argv,
-         gchar               **envp,
-         gboolean              close_descriptors,
-         gboolean              search_path,
-         gboolean              stdout_to_null,
-         gboolean              stderr_to_null,
-         gboolean              child_inherits_stdin,
-         gboolean              file_and_argv_zero,
-         GSpawnChildSetupFunc  child_setup,
-         gpointer              user_data)
-{
-  gchar **protected_argv;
-  gchar **new_argv;
-  gchar args[ARG_COUNT][10];
-  gint i;
-  int rc;
-  int argc;
-
-  SETUP_DEBUG();
-
-  argc = protect_argv (argv, &protected_argv);
-
-  if (stdin_fd == -1 && stdout_fd == -1 && stderr_fd == -1 &&
-      (working_directory == NULL || !*working_directory) &&
-      !close_descriptors &&
-      !stdout_to_null && !stderr_to_null &&
-      child_inherits_stdin)
-    {
-      /* We can do without the helper process */
-      int mode = dont_wait ? P_NOWAIT : P_WAIT;
+      /* We can do without the helper process */
+      int mode = dont_wait ? P_NOWAIT : P_WAIT;
 
       if (debug)
-       g_print ("doing without gspawn-win32-helper\n");
+       g_print ("doing without " HELPER_PROCESS "\n");
 
-      if (search_path)
-       rc = spawnvp (mode, argv[0], file_and_argv_zero ? protected_argv + 1 : protected_argv);
+      /* Call user function just before we spawn the program. Dunno
+       * what's the usefulness of this. A child setup function used on
+       * Unix probably isn't of much use as such on Win32, anyhow.
+       */
+      if (child_setup)
+       (* child_setup) (user_data);
+
+      new_argv = (flags & G_SPAWN_FILE_AND_ARGV_ZERO) ? protected_argv + 1 : protected_argv;
+      if (flags & G_SPAWN_SEARCH_PATH)
+       if (envp != NULL)
+         rc = spawnvpe (mode, argv[0], (const char **) new_argv, (const char **) envp);
+       else
+         rc = spawnvp (mode, argv[0], (const char **) new_argv);
       else
-       rc = spawnv (mode, argv[0], file_and_argv_zero ? protected_argv + 1 : protected_argv);
+       if (envp != NULL)
+         rc = spawnve (mode, argv[0], (const char **) new_argv, (const char **) envp);
+       else
+         rc = spawnv (mode, argv[0], (const char **) new_argv);
+
+      saved_errno = errno;
 
       for (i = 0; i < argc; i++)
        g_free (protected_argv[i]);
       g_free (protected_argv);
-      
-      if (rc == -1)
-       {
-         return DO_SPAWN_ERROR_NO_HELPER;
-       }
-      else
+
+      if (rc == -1 && saved_errno != 0)
        {
-         shortcut_spawn_retval = rc;
-         return DO_SPAWN_OK_NO_HELPER;
+         g_set_error (error,
+                      G_SPAWN_ERROR,
+                      G_SPAWN_ERROR_FAILED,
+                      _("Failed to execute child process (%s)"),
+                      g_strerror (errno));
+         goto cleanup_and_fail;
        }
+
+      if (child_handle && dont_wait && !dont_return_handle)
+       *child_handle = (GPid) rc;
+      else if (!dont_wait && exit_status)
+       *exit_status = rc;
+      
+      return TRUE;
     }
 
-  new_argv = g_new (gchar *, argc + 1 + ARG_COUNT);
+  new_argv = g_new (char *, argc + 1 + ARG_COUNT);
 
-  new_argv[0] = "gspawn-win32-helper";
-  _g_sprintf (args[ARG_CHILD_ERR_REPORT], "%d", child_err_report_fd);
+  if (standard_input && !make_pipe (stdin_pipe, error))
+    goto cleanup_and_fail;
+  
+  if (standard_output && !make_pipe (stdout_pipe, error))
+    goto cleanup_and_fail;
+  
+  if (standard_error && !make_pipe (stderr_pipe, error))
+    goto cleanup_and_fail;
+  
+  if (!make_pipe (child_err_report_pipe, error))
+    goto cleanup_and_fail;
+  
+  new_argv[0] = HELPER_PROCESS;
+  _g_sprintf (args[ARG_CHILD_ERR_REPORT], "%d", child_err_report_pipe[1]);
   new_argv[ARG_CHILD_ERR_REPORT] = args[ARG_CHILD_ERR_REPORT];
-
-  if (file_and_argv_zero)
+  
+  if (flags & G_SPAWN_FILE_AND_ARGV_ZERO)
     {
       /* Overload ARG_CHILD_ERR_REPORT to also encode the
        * G_SPAWN_FILE_AND_ARGV_ZERO functionality.
        */
       strcat (args[ARG_CHILD_ERR_REPORT], "#");
     }
-
-  if (stdin_fd >= 0)
+  
+  if (standard_input)
     {
-      _g_sprintf (args[ARG_STDIN], "%d", stdin_fd);
+      _g_sprintf (args[ARG_STDIN], "%d", stdin_pipe[0]);
       new_argv[ARG_STDIN] = args[ARG_STDIN];
     }
-  else if (child_inherits_stdin)
+  else if (flags & G_SPAWN_CHILD_INHERITS_STDIN)
     {
       /* Let stdin be alone */
       new_argv[ARG_STDIN] = "-";
@@ -717,13 +493,13 @@ do_spawn (gboolean              dont_wait,
       /* Keep process from blocking on a read of stdin */
       new_argv[ARG_STDIN] = "z";
     }
-
-  if (stdout_fd >= 0)
+  
+  if (standard_output)
     {
-      _g_sprintf (args[ARG_STDOUT], "%d", stdout_fd);
+      _g_sprintf (args[ARG_STDOUT], "%d", stdout_pipe[1]);
       new_argv[ARG_STDOUT] = args[ARG_STDOUT];
     }
-  else if (stdout_to_null)
+  else if (flags & G_SPAWN_STDOUT_TO_DEV_NULL)
     {
       new_argv[ARG_STDOUT] = "z";
     }
@@ -731,13 +507,13 @@ do_spawn (gboolean              dont_wait,
     {
       new_argv[ARG_STDOUT] = "-";
     }
-
-  if (stderr_fd >= 0)
+  
+  if (standard_error)
     {
-      _g_sprintf (args[ARG_STDERR], "%d", stderr_fd);
+      _g_sprintf (args[ARG_STDERR], "%d", stderr_pipe[1]);
       new_argv[ARG_STDERR] = args[ARG_STDERR];
     }
-  else if (stderr_to_null)
+  else if (flags & G_SPAWN_STDERR_TO_DEV_NULL)
     {
       new_argv[ARG_STDERR] = "z";
     }
@@ -745,19 +521,19 @@ do_spawn (gboolean              dont_wait,
     {
       new_argv[ARG_STDERR] = "-";
     }
-
+  
   if (working_directory && *working_directory)
     /* The g_strdup() to lose the constness */
     new_argv[ARG_WORKING_DIRECTORY] = g_strdup (working_directory);
   else
     new_argv[ARG_WORKING_DIRECTORY] = g_strdup ("-");
-
-  if (close_descriptors)
+  
+  if (!(flags & G_SPAWN_LEAVE_DESCRIPTORS_OPEN))
     new_argv[ARG_CLOSE_DESCRIPTORS] = "y";
   else
     new_argv[ARG_CLOSE_DESCRIPTORS] = "-";
 
-  if (search_path)
+  if (flags & G_SPAWN_SEARCH_PATH)
     new_argv[ARG_USE_PATH] = "y";
   else
     new_argv[ARG_USE_PATH] = "-";
@@ -770,42 +546,33 @@ do_spawn (gboolean              dont_wait,
   for (i = 0; i <= argc; i++)
     new_argv[ARG_PROGRAM + i] = protected_argv[i];
 
-  /* Call user function just before we execute the helper program,
-   * which executes the program. Dunno what's the usefulness of this.
-   * A child setup function used on Unix probably isn't of much use
-   * as such on Win32, anyhow
-   */
-  if (child_setup)
-    {
-      (* child_setup) (user_data);
-    }
-
   if (debug)
     {
-      g_print ("calling gspawn-win32-helper with argv:\n");
+      g_print ("calling " HELPER_PROCESS " with argv:\n");
       for (i = 0; i < argc + 1 + ARG_COUNT; i++)
        g_print ("argv[%d]: %s\n", i, (new_argv[i] ? new_argv[i] : "NULL"));
     }
-  
+
+  if (child_setup)
+    (* child_setup) (user_data);
+
   if (envp != NULL)
     /* Let's hope envp hasn't mucked with PATH so that
      * gspawn-win32-helper.exe isn't found.
      */
-    rc = spawnvpe (P_NOWAIT, "gspawn-win32-helper", new_argv, envp);
+    rc = spawnvpe (P_NOWAIT, HELPER_PROCESS, (const char **) new_argv, (const char **) envp);
   else
-    rc = spawnvp (P_NOWAIT, "gspawn-win32-helper", new_argv);
+    rc = spawnvp (P_NOWAIT, HELPER_PROCESS, (const char **) new_argv);
+
+  saved_errno = errno;
 
-  /* Close the child_err_report_fd and the other process's ends of the
-   * pipes in this process, otherwise the reader will never get
-   * EOF.
+  /* Close thed the other process's ends of the pipes in this
+   * process, otherwise the reader will never get EOF.
    */
-  close (child_err_report_fd);
-  if (stdin_fd >= 0)
-    close (stdin_fd);
-  if (stdout_fd >= 0)
-    close (stdout_fd);
-  if (stderr_fd >= 0)
-    close (stderr_fd);
+  close_and_invalidate (&child_err_report_pipe[1]);
+  close_and_invalidate (&stdin_pipe[0]);
+  close_and_invalidate (&stdout_pipe[1]);
+  close_and_invalidate (&stderr_pipe[1]);
 
   for (i = 0; i < argc; i++)
     g_free (protected_argv[i]);
@@ -814,116 +581,8 @@ do_spawn (gboolean              dont_wait,
   g_free (new_argv[ARG_WORKING_DIRECTORY]);
   g_free (new_argv);
 
-  return rc;
-}
-
-static gboolean
-read_ints (int      fd,
-           gint*    buf,
-           gint     n_ints_in_buf,
-           gint    *n_ints_read,
-           GError **error)
-{
-  gint bytes = 0;
-  
-  while (bytes < sizeof(gint)*n_ints_in_buf)
-    {
-      gint chunk;
-
-      if (debug)
-       g_print ("%s:read_ints: trying to read %d bytes from pipe...\n",
-                __FILE__,
-                sizeof(gint)*n_ints_in_buf - bytes);
-
-      chunk = read (fd, ((gchar*)buf) + bytes,
-                   sizeof(gint)*n_ints_in_buf - bytes);
-
-      if (debug)
-       g_print ("... got %d bytes\n", chunk);
-          
-      if (chunk < 0)
-        {
-          /* Some weird shit happened, bail out */
-              
-          g_set_error (error,
-                       G_SPAWN_ERROR,
-                       G_SPAWN_ERROR_FAILED,
-                       _("Failed to read from child pipe (%s)"),
-                       g_strerror (errno));
-
-          return FALSE;
-        }
-      else if (chunk == 0)
-        break; /* EOF */
-      else
-       bytes += chunk;
-    }
-
-  *n_ints_read = bytes/sizeof(gint);
-
-  return TRUE;
-}
-
-static gboolean
-do_spawn_with_pipes (gboolean              dont_wait,
-                    gboolean              dont_return_handle,
-                    const gchar          *working_directory,
-                    gchar               **argv,
-                    gchar               **envp,
-                    gboolean              close_descriptors,
-                    gboolean              search_path,
-                    gboolean              stdout_to_null,
-                    gboolean              stderr_to_null,
-                    gboolean              child_inherits_stdin,
-                    gboolean              file_and_argv_zero,
-                    GSpawnChildSetupFunc  child_setup,
-                    gpointer              user_data,
-                    GPid                 *child_handle,
-                    gint                 *standard_input,
-                    gint                 *standard_output,
-                    gint                 *standard_error,
-                    gint                 *exit_status,
-                    GError              **error)     
-{
-  gint stdin_pipe[2] = { -1, -1 };
-  gint stdout_pipe[2] = { -1, -1 };
-  gint stderr_pipe[2] = { -1, -1 };
-  gint child_err_report_pipe[2] = { -1, -1 };
-  gint helper = -1;
-  gint buf[2];
-  gint n_ints = 0;
-  
-  if (!make_pipe (child_err_report_pipe, error))
-    return FALSE;
-
-  if (standard_input && !make_pipe (stdin_pipe, error))
-    goto cleanup_and_fail;
-  
-  if (standard_output && !make_pipe (stdout_pipe, error))
-    goto cleanup_and_fail;
-
-  if (standard_error && !make_pipe (stderr_pipe, error))
-    goto cleanup_and_fail;
-
-  helper = do_spawn (dont_wait,
-                    child_err_report_pipe[1],
-                    stdin_pipe[0],
-                    stdout_pipe[1],
-                    stderr_pipe[1],
-                    working_directory,
-                    argv,
-                    envp,
-                    close_descriptors,
-                    search_path,
-                    stdout_to_null,
-                    stderr_to_null,
-                    child_inherits_stdin,
-                    file_and_argv_zero,
-                    child_setup,
-                    user_data);
-      
   /* Check if gspawn-win32-helper couldn't be run */
-  if (helper == DO_SPAWN_ERROR_HELPER)
+  if (rc == -1 && saved_errno != 0)
     {
       g_set_error (error,
                   G_SPAWN_ERROR,
@@ -932,70 +591,47 @@ do_spawn_with_pipes (gboolean              dont_wait,
       goto cleanup_and_fail;
     }
 
-  else if (helper == DO_SPAWN_OK_NO_HELPER)
+  if (!dont_wait)
     {
-      if (child_handle && dont_wait && !dont_return_handle)
-       *child_handle = (GPid) shortcut_spawn_retval;
-      else if (!dont_wait && exit_status)
-       *exit_status = shortcut_spawn_retval;
-
-      close_and_invalidate (&child_err_report_pipe[0]);
-      close_and_invalidate (&child_err_report_pipe[1]);
-
-      return TRUE;
+      /* Synchronous case. Pass helper's report pipe back to caller,
+       * which takes care of reading it after the grandchild has
+       * finished.
+       */
+      g_assert (err_report != NULL);
+      *err_report = child_err_report_pipe[0];
     }
-  else if (helper == DO_SPAWN_ERROR_NO_HELPER)
+  else
     {
-      g_set_error (error,
-                  G_SPAWN_ERROR,
-                  G_SPAWN_ERROR_FAILED,
-                  _("Failed to execute child process (%s)"),
-                  g_strerror (errno));
-      helper = -1;
-      goto cleanup_and_fail;
-    }
-
-  if (!read_ints (child_err_report_pipe[0],
-                 buf, 2, &n_ints,
-                 error) ||
-          n_ints != 2)
-    goto cleanup_and_fail;
+      /* Asynchronous case. We read the helper's report right away. */
+      if (!read_helper_report (child_err_report_pipe[0], helper_report, error))
+       goto cleanup_and_fail;
         
-  /* Error code from gspawn-win32-helper. */
-  switch (buf[0])
-    {
-    case CHILD_NO_ERROR:
-      if (child_handle && dont_wait && !dont_return_handle)
+      close (child_err_report_pipe[0]);
+
+      switch (helper_report[0])
        {
-         /* helper is our HANDLE for gspawn-win32-helper. It has
-          * told us the HANDLE of its child. Duplicate that into
-          * a HANDLE valid in this process.
-          */
-         if (!DuplicateHandle ((HANDLE) helper, (HANDLE) buf[1],
-                               GetCurrentProcess (), (LPHANDLE) child_handle,
-                               0, TRUE, DUPLICATE_SAME_ACCESS))
+       case CHILD_NO_ERROR:
+         if (child_handle && dont_wait && !dont_return_handle)
+           {
+             /* rc is our HANDLE for gspawn-win32-helper. It has
+              * told us the HANDLE of its child. Duplicate that into
+              * a HANDLE valid in this process.
+              */
+             if (!DuplicateHandle ((HANDLE) rc, (HANDLE) helper_report[1],
+                                   GetCurrentProcess (), (LPHANDLE) child_handle,
+                                   0, TRUE, DUPLICATE_SAME_ACCESS))
+               *child_handle = 0;
+           }
+         else if (child_handle)
            *child_handle = 0;
+         if (exit_status)
+           *exit_status = helper_report[1];
+         break;
+         
+       default:
+         set_child_error (helper_report, working_directory, error);
+         goto cleanup_and_fail;
        }
-      else if (child_handle)
-       *child_handle = 0;
-      break;
-      
-    case CHILD_CHDIR_FAILED:
-      g_set_error (error,
-                  G_SPAWN_ERROR,
-                  G_SPAWN_ERROR_CHDIR,
-                  _("Failed to change to directory '%s' (%s)"),
-                  working_directory,
-                  g_strerror (buf[1]));
-      goto cleanup_and_fail;
-      
-    case CHILD_SPAWN_FAILED:
-      g_set_error (error,
-                  G_SPAWN_ERROR,
-                  G_SPAWN_ERROR_FAILED,
-                  _("Failed to execute child process (%s)"),
-                  g_strerror (buf[1]));
-      goto cleanup_and_fail;
     }
 
   /* Success against all odds! return the information */
@@ -1006,45 +642,375 @@ do_spawn_with_pipes (gboolean              dont_wait,
     *standard_output = stdout_pipe[0];
   if (standard_error)
     *standard_error = stderr_pipe[0];
-  if (exit_status)
-    *exit_status = buf[1];
-  CloseHandle ((HANDLE) helper);
+  if (rc != -1)
+    CloseHandle ((HANDLE) rc);
   
-  close_and_invalidate (&child_err_report_pipe[0]);
-
   return TRUE;
 
  cleanup_and_fail:
-  if (helper != -1)
-    CloseHandle ((HANDLE) helper);
-  close_and_invalidate (&child_err_report_pipe[0]);
-  close_and_invalidate (&child_err_report_pipe[1]);
-  close_and_invalidate (&stdin_pipe[0]);
-  close_and_invalidate (&stdin_pipe[1]);
-  close_and_invalidate (&stdout_pipe[0]);
-  close_and_invalidate (&stdout_pipe[1]);
-  close_and_invalidate (&stderr_pipe[0]);
-  close_and_invalidate (&stderr_pipe[1]);
+  if (rc != -1)
+    CloseHandle ((HANDLE) rc);
+  if (child_err_report_pipe[0] != -1)
+    close (child_err_report_pipe[0]);
+  if (child_err_report_pipe[1] != -1)
+    close (child_err_report_pipe[1]);
+  if (stdin_pipe[0] != -1)
+    close (stdin_pipe[0]);
+  if (stdin_pipe[1] != -1)
+    close (stdin_pipe[1]);
+  if (stdout_pipe[0] != -1)
+    close (stdout_pipe[0]);
+  if (stdout_pipe[1] != -1)
+    close (stdout_pipe[1]);
+  if (stderr_pipe[0] != -1)
+    close (stderr_pipe[0]);
+  if (stderr_pipe[1] != -1)
+    close (stderr_pipe[1]);
 
   return FALSE;
 }
 
-static gboolean
-make_pipe (gint     p[2],
-           GError **error)
+gboolean
+g_spawn_sync (const gchar          *working_directory,
+              gchar               **argv,
+              gchar               **envp,
+              GSpawnFlags           flags,
+              GSpawnChildSetupFunc  child_setup,
+              gpointer              user_data,
+              gchar               **standard_output,
+              gchar               **standard_error,
+              gint                 *exit_status,
+              GError              **error)     
 {
-  if (pipe (p) < 0)
+  gint outpipe = -1;
+  gint errpipe = -1;
+  gint reportpipe = -1;
+  GPid pid;
+  GIOChannel *outchannel = NULL;
+  GIOChannel *errchannel = NULL;
+  GPollFD outfd, errfd;
+  GPollFD fds[2];
+  gint nfds;
+  gint outindex = -1;
+  gint errindex = -1;
+  gint ret;
+  GString *outstr = NULL;
+  GString *errstr = NULL;
+  gboolean failed;
+  gint status;
+  
+  g_return_val_if_fail (argv != NULL, FALSE);
+  g_return_val_if_fail (!(flags & G_SPAWN_DO_NOT_REAP_CHILD), FALSE);
+  g_return_val_if_fail (standard_output == NULL ||
+                        !(flags & G_SPAWN_STDOUT_TO_DEV_NULL), FALSE);
+  g_return_val_if_fail (standard_error == NULL ||
+                        !(flags & G_SPAWN_STDERR_TO_DEV_NULL), FALSE);
+  
+  /* Just to ensure segfaults if callers try to use
+   * these when an error is reported.
+   */
+  if (standard_output)
+    *standard_output = NULL;
+
+  if (standard_error)
+    *standard_error = NULL;
+  
+  if (!do_spawn_with_pipes (FALSE,
+                           TRUE,
+                           working_directory,
+                           argv,
+                           envp,
+                           flags,
+                           child_setup,
+                           user_data,
+                           &pid,
+                           NULL,
+                           standard_output ? &outpipe : NULL,
+                           standard_error ? &errpipe : NULL,
+                           &status,
+                           &reportpipe,
+                           error))
+    return FALSE;
+
+  /* Read data from child. */
+  
+  failed = FALSE;
+
+  if (outpipe >= 0)
     {
-      g_set_error (error,
-                   G_SPAWN_ERROR,
-                   G_SPAWN_ERROR_FAILED,
-                   _("Failed to create pipe for communicating with child process (%s)"),
-                   g_strerror (errno));
+      outstr = g_string_new (NULL);
+      outchannel = g_io_channel_win32_new_fd (outpipe);
+      g_io_channel_set_encoding (outchannel, NULL, NULL);
+      g_io_channel_win32_make_pollfd (outchannel,
+                                     G_IO_IN | G_IO_ERR | G_IO_HUP,
+                                     &outfd);
+    }
+      
+  if (errpipe >= 0)
+    {
+      errstr = g_string_new (NULL);
+      errchannel = g_io_channel_win32_new_fd (errpipe);
+      g_io_channel_set_encoding (errchannel, NULL, NULL);
+      g_io_channel_win32_make_pollfd (errchannel,
+                                     G_IO_IN | G_IO_ERR | G_IO_HUP,
+                                     &errfd);
+    }
+
+  /* Read data until we get EOF on all pipes. */
+  while (!failed && (outpipe >= 0 || errpipe >= 0))
+    {
+      nfds = 0;
+      if (outpipe >= 0)
+       {
+         fds[nfds] = outfd;
+         outindex = nfds;
+         nfds++;
+       }
+      if (errpipe >= 0)
+       {
+         fds[nfds] = errfd;
+         errindex = nfds;
+         nfds++;
+       }
+
+      if (debug)
+       g_print ("g_spawn_sync: calling g_io_channel_win32_poll, nfds=%d\n",
+                nfds);
+
+      ret = g_io_channel_win32_poll (fds, nfds, -1);
+
+      if (ret < 0)
+        {
+          failed = TRUE;
+
+          g_set_error (error,
+                       G_SPAWN_ERROR,
+                       G_SPAWN_ERROR_READ,
+                       _("Unexpected error in g_io_channel_win32_poll() reading data from a child process"));
+         
+          break;
+        }
+
+      if (outpipe >= 0 && (fds[outindex].revents & G_IO_IN))
+        {
+          switch (read_data (outstr, outchannel, error))
+            {
+            case READ_FAILED:
+             if (debug)
+               g_print ("g_spawn_sync: outchannel: READ_FAILED\n");
+              failed = TRUE;
+              break;
+            case READ_EOF:
+             if (debug)
+               g_print ("g_spawn_sync: outchannel: READ_EOF\n");
+              g_io_channel_unref (outchannel);
+             outchannel = NULL;
+              close_and_invalidate (&outpipe);
+              break;
+            default:
+             if (debug)
+               g_print ("g_spawn_sync: outchannel: OK\n");
+              break;
+            }
+
+          if (failed)
+            break;
+        }
+
+      if (errpipe >= 0 && (fds[errindex].revents & G_IO_IN))
+        {
+          switch (read_data (errstr, errchannel, error))
+            {
+            case READ_FAILED:
+             if (debug)
+               g_print ("g_spawn_sync: errchannel: READ_FAILED\n");
+              failed = TRUE;
+              break;
+            case READ_EOF:
+             if (debug)
+               g_print ("g_spawn_sync: errchannel: READ_EOF\n");
+             g_io_channel_unref (errchannel);
+             errchannel = NULL;
+              close_and_invalidate (&errpipe);
+              break;
+            default:
+             if (debug)
+               g_print ("g_spawn_sync: errchannel: OK\n");
+              break;
+            }
+
+          if (failed)
+            break;
+        }
+    }
+
+  if (reportpipe == -1)
+    {
+      /* No helper process, exit status of actual spawned process
+       * already available.
+       */
+      if (exit_status)
+        *exit_status = status;
+    }
+  else
+    {
+      /* Helper process was involved. Read its report now after the
+       * grandchild has finished.
+       */
+      gint helper_report[2];
+
+      if (!read_helper_report (reportpipe, helper_report, error))
+       failed = TRUE;
+      else
+       {
+         switch (helper_report[0])
+           {
+           case CHILD_NO_ERROR:
+             if (exit_status)
+               *exit_status = helper_report[1];
+             break;
+           default:
+             set_child_error (helper_report, working_directory, error);
+             failed = TRUE;
+             break;
+           }
+       }
+      close_and_invalidate (&reportpipe);
+    }
+
+
+  /* These should only be open still if we had an error.  */
+  
+  if (outchannel != NULL)
+    g_io_channel_unref (outchannel);
+  if (errchannel != NULL)
+    g_io_channel_unref (errchannel);
+  if (outpipe >= 0)
+    close_and_invalidate (&outpipe);
+  if (errpipe >= 0)
+    close_and_invalidate (&errpipe);
+  
+  g_spawn_close_pid (pid);
+
+  if (failed)
+    {
+      if (outstr)
+        g_string_free (outstr, TRUE);
+      if (errstr)
+        g_string_free (errstr, TRUE);
+
       return FALSE;
     }
   else
-    return TRUE;
+    {
+      if (standard_output)        
+        *standard_output = g_string_free (outstr, FALSE);
+
+      if (standard_error)
+        *standard_error = g_string_free (errstr, FALSE);
+
+      return TRUE;
+    }
+}
+
+gboolean
+g_spawn_async_with_pipes (const gchar          *working_directory,
+                          gchar               **argv,
+                          gchar               **envp,
+                          GSpawnFlags           flags,
+                          GSpawnChildSetupFunc  child_setup,
+                          gpointer              user_data,
+                          GPid                 *child_handle,
+                          gint                 *standard_input,
+                          gint                 *standard_output,
+                          gint                 *standard_error,
+                          GError              **error)
+{
+  g_return_val_if_fail (argv != NULL, FALSE);
+  g_return_val_if_fail (standard_output == NULL ||
+                        !(flags & G_SPAWN_STDOUT_TO_DEV_NULL), FALSE);
+  g_return_val_if_fail (standard_error == NULL ||
+                        !(flags & G_SPAWN_STDERR_TO_DEV_NULL), FALSE);
+  /* can't inherit stdin if we have an input pipe. */
+  g_return_val_if_fail (standard_input == NULL ||
+                        !(flags & G_SPAWN_CHILD_INHERITS_STDIN), FALSE);
+  
+  return do_spawn_with_pipes (TRUE,
+                             !(flags & G_SPAWN_DO_NOT_REAP_CHILD),
+                             working_directory,
+                             argv,
+                             envp,
+                             flags,
+                             child_setup,
+                             user_data,
+                             child_handle,
+                             standard_input,
+                             standard_output,
+                             standard_error,
+                             NULL,
+                             NULL,
+                             error);
 }
+
+gboolean
+g_spawn_command_line_sync (const gchar  *command_line,
+                           gchar       **standard_output,
+                           gchar       **standard_error,
+                           gint         *exit_status,
+                           GError      **error)
+{
+  gboolean retval;
+  gchar **argv = 0;
+
+  g_return_val_if_fail (command_line != NULL, FALSE);
+  
+  if (!g_shell_parse_argv (command_line,
+                           NULL, &argv,
+                           error))
+    return FALSE;
+  
+  retval = g_spawn_sync (NULL,
+                         argv,
+                         NULL,
+                         G_SPAWN_SEARCH_PATH,
+                         NULL,
+                         NULL,
+                         standard_output,
+                         standard_error,
+                         exit_status,
+                         error);
+  g_strfreev (argv);
+
+  return retval;
+}
+
+gboolean
+g_spawn_command_line_async (const gchar *command_line,
+                            GError     **error)
+{
+  gboolean retval;
+  gchar **argv = 0;
+
+  g_return_val_if_fail (command_line != NULL, FALSE);
+
+  if (!g_shell_parse_argv (command_line,
+                           NULL, &argv,
+                           error))
+    return FALSE;
+  
+  retval = g_spawn_async (NULL,
+                          argv,
+                          NULL,
+                          G_SPAWN_SEARCH_PATH,
+                          NULL,
+                          NULL,
+                          NULL,
+                          error);
+  g_strfreev (argv);
+
+  return retval;
+}
+
 #endif /* !GSPAWN_HELPER */
 
 void