Add g_spawn_check_exit_status()

Many (if not "almost all") programs that spawn other programs via
g_spawn_sync() or the like simply want to check whether or not the
child exited successfully, but doing so requires use of
platform-specific functionality and there's actually a fair amount of
boilerplate involved.

This new API will help drain a *lot* of mostly duplicated code in
GNOME, from gnome-session to gdm.  And we can see that some bits even
inside GLib were doing it wrong; for example checking the exit status
on Unix, but ignoring it on Windows.

https://bugzilla.gnome.org/show_bug.cgi?id=679691

docs/reference/glib/glib-sections.txt

@@ -1093,6 +1093,7 @@ GSpawnChildSetupFunc
 g_spawn_async_with_pipes
 g_spawn_async
 g_spawn_sync
+g_spawn_check_exit_status
 g_spawn_command_line_async
 g_spawn_command_line_sync
 g_spawn_close_pid

gio/gdbusaddress.c

@@ -43,7 +43,6 @@
 
 #ifdef G_OS_UNIX
 #include <gio/gunixsocketaddress.h>
-#include <sys/wait.h>
 #endif
 
 #ifdef G_OS_WIN32
@@ -1062,40 +1061,16 @@ get_session_address_dbus_launch (GError **error)
                                   &launch_stderr,
                                   &exit_status,
                                   error))
+    {
+      goto out;
+    }
+
+  if (!g_spawn_check_exit_status (exit_status, error))
     {
       g_prefix_error (error, _("Error spawning command line `%s': "), command_line);
       goto out;
     }
 
-  if (!WIFEXITED (exit_status))
-    {
-      gchar *escaped_stderr;
-      escaped_stderr = g_strescape (launch_stderr, "");
-      g_set_error (error,
-                   G_IO_ERROR,
-                   G_IO_ERROR_FAILED,
-                   _("Abnormal program termination spawning command line `%s': %s"),
-                   command_line,
-                   escaped_stderr);
-      g_free (escaped_stderr);
-      goto out;
-    }
-
-  if (WEXITSTATUS (exit_status) != 0)
-    {
-      gchar *escaped_stderr;
-      escaped_stderr = g_strescape (launch_stderr, "");
-      g_set_error (error,
-                   G_IO_ERROR,
-                   G_IO_ERROR_FAILED,
-                   _("Command line `%s' exited with non-zero exit status %d: %s"),
-                   command_line,
-                   WEXITSTATUS (exit_status),
-                   escaped_stderr);
-      g_free (escaped_stderr);
-      goto out;
-    }
-
   /* From the dbus-launch(1) man page:
    *
    *   --binary-syntax Write to stdout a nul-terminated bus address,

gio/gdesktopappinfo.c

@@ -26,7 +26,6 @@
 #include <errno.h>
 #include <string.h>
 #include <unistd.h>
-#include <sys/wait.h>
 
 #ifdef HAVE_CRT_EXTERNS_H
 #include <crt_externs.h>
@@ -1850,8 +1849,7 @@ update_program_done (GPid     pid,
 		     gpointer data)
 {
   /* Did the application exit correctly */
-  if (WIFEXITED (status) &&
-      WEXITSTATUS (status) == 0)
+  if (g_spawn_check_exit_status (status, NULL))
     {
       /* Here we could clean out any caches in use */
     }

gio/glib-compile-resources.c

@@ -33,9 +33,6 @@
 #ifdef G_OS_WIN32
 #include <io.h>
 #endif
-#ifdef HAVE_SYS_WAIT_H
-#include <sys/wait.h>
-#endif
 
 #include <gio/gmemoryoutputstream.h>
 #include <gio/gzlibcompressor.h>
@@ -331,14 +328,14 @@ end_element (GMarkupParseContext  *context,
                   g_propagate_error (error, my_error);
                   goto cleanup;
                 }
-#ifdef HAVE_SYS_WAIT_H
-              if (!WIFEXITED (status) || WEXITSTATUS (status) != 0)
+	      
+	      /* Ugly...we shoud probably just let stderr be inherited */
+	      if (!g_spawn_check_exit_status (status, NULL))
                 {
                   g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED,
                                _("Error processing input file with xmllint:\n%s"), stderr_child);
                   goto cleanup;
                 }
-#endif
 
               g_free (stderr_child);
               g_free (real_file);
@@ -387,14 +384,13 @@ end_element (GMarkupParseContext  *context,
                   g_propagate_error (error, my_error);
                   goto cleanup;
                 }
-#ifdef HAVE_SYS_WAIT_H
-              if (!WIFEXITED (status) || WEXITSTATUS (status) != 0)
+	      
+	      if (!g_spawn_check_exit_status (status, NULL))
                 {
                   g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED,
 			       _("Error processing input file with to-pixdata:\n%s"), stderr_child);
                   goto cleanup;
                 }
-#endif
 
               g_free (stderr_child);
               g_free (real_file);

gio/tests/gdbus-connection-slow.c

@@ -25,9 +25,6 @@
 #include <string.h>
 
 #include <sys/types.h>
-#ifdef HAVE_SYS_WAIT_H
-#include <sys/wait.h>
-#endif
 
 #include "gdbus-tests.h"
 
@@ -97,10 +94,8 @@ test_connection_flush (void)
                                        &exit_status,
                                        &error);
       g_assert_no_error (error);
-#ifdef HAVE_SYS_WAIT_H
-      g_assert (WIFEXITED (exit_status));
-      g_assert_cmpint (WEXITSTATUS (exit_status), ==, 0);
-#endif
+      g_spawn_check_exit_status (exit_status, &error);
+      g_assert_no_error (error);
       g_assert (ret);
 
       timeout_mainloop_id = g_timeout_add (1000, test_connection_flush_on_timeout, GUINT_TO_POINTER (n));

gio/tests/gdbus-connection.c

@@ -141,7 +141,7 @@ test_connection_life_cycle (void)
    *
    */
   c = g_bus_get_sync (G_BUS_TYPE_SESSION, NULL, &error);
-  _g_assert_error_domain (error, G_IO_ERROR);
+  g_assert (error != NULL);
   g_assert (!g_dbus_error_is_remote_error (error));
   g_assert (c == NULL);
   g_error_free (error);

glib/glib.symbols

@@ -980,6 +980,7 @@ g_spawn_command_line_async PRIVATE
 g_spawn_command_line_sync PRIVATE
 #endif
 g_spawn_error_quark
+g_spawn_exit_error_quark
 #ifndef _WIN64
 g_spawn_sync PRIVATE
 #endif
@@ -990,6 +991,7 @@ g_spawn_command_line_async_utf8
 g_spawn_command_line_sync_utf8
 g_spawn_sync_utf8
 #endif
+g_spawn_check_exit_status
 #if !defined(G_OS_UNIX) || defined(G_STDIO_NO_WRAP_ON_UNIX)
 /* gstdio wrappers */
 g_chmod

glib/gmain.c

@@ -4664,11 +4664,15 @@ g_child_watch_source_new (GPid pid)
  * If you obtain @pid from g_spawn_async() or g_spawn_async_with_pipes() 
  * you will need to pass #G_SPAWN_DO_NOT_REAP_CHILD as flag to 
  * the spawn function for the child watching to work.
+ *
+ * In many programs, you will want to call g_spawn_check_exit_status()
+ * in the callback to determine whether or not the child exited
+ * successfully.
  * 
- * Note that on platforms where #GPid must be explicitly closed
- * (see g_spawn_close_pid()) @pid must not be closed while the
- * source is still active. Typically, you will want to call
- * g_spawn_close_pid() in the callback function for the source.
+ * Also, note that on platforms where #GPid must be explicitly closed
+ * (see g_spawn_close_pid()) @pid must not be closed while the source
+ * is still active.  Typically, you should invoke g_spawn_close_pid()
+ * in the callback function for the source.
  * 
  * GLib supports only a single callback per process id.
  *

glib/gmain.h

@@ -139,11 +139,13 @@ typedef gboolean (*GSourceFunc)       (gpointer user_data);
 /**
  * GChildWatchFunc:
  * @pid: the process id of the child process
- * @status: Status information about the child process,
- *     see waitpid(2) for more information about this field
+ * @status: Status information about the child process, encoded
+ *     in a platform-specific manner
  * @user_data: user data passed to g_child_watch_add()
  *
- * The type of functions to be called when a child exists.
+ * Prototype of a #GChildWatchSource callback, called when a child
+ * process has exited.  To interpret @status, see the documentation
+ * for g_spawn_check_exit_status().
  */
 typedef void     (*GChildWatchFunc)   (GPid     pid,
                                        gint     status,

glib/gspawn.c

@@ -94,6 +94,12 @@ g_spawn_error_quark (void)
   return g_quark_from_static_string ("g-exec-error-quark");
 }
 
+GQuark
+g_spawn_exit_error_quark (void)
+{
+  return g_quark_from_static_string ("g-spawn-exit-error-quark");
+}
+
 /**
  * g_spawn_async:
  * @working_directory: (allow-none): child's current working directory, or %NULL to inherit parent's
@@ -232,13 +238,15 @@ read_data (GString *str,
  * if those parameters are non-%NULL. Note that you must set the  
  * %G_SPAWN_STDOUT_TO_DEV_NULL and %G_SPAWN_STDERR_TO_DEV_NULL flags when
  * passing %NULL for @standard_output and @standard_error.
- * If @exit_status is non-%NULL, the exit status of the child is stored
- * there as it would be returned by waitpid(); standard UNIX macros such 
- * as WIFEXITED() and WEXITSTATUS() must be used to evaluate the exit status.
- * Note that this function call waitpid() even if @exit_status is %NULL, and
- * does not accept the %G_SPAWN_DO_NOT_REAP_CHILD flag.
- * If an error occurs, no data is returned in @standard_output, 
- * @standard_error, or @exit_status. 
+ *
+ * If @exit_status is non-%NULL, the platform-specific exit status of
+ * the child is stored there; see the doucumentation of
+ * g_spawn_check_exit_status() for how to use and interpret this.
+ * Note that it is invalid to pass %G_SPAWN_DO_NOT_REAP_CHILD in
+ * @flags.
+ *
+ * If an error occurs, no data is returned in @standard_output,
+ * @standard_error, or @exit_status.
  *
  * This function calls g_spawn_async_with_pipes() internally; see that
  * function for full details on the other parameters and details on
@@ -701,9 +709,9 @@ g_spawn_async_with_pipes (const gchar          *working_directory,
  * appropriate. Possible errors are those from g_spawn_sync() and those
  * from g_shell_parse_argv().
  *
- * If @exit_status is non-%NULL, the exit status of the child is stored there as
- * it would be returned by waitpid(); standard UNIX macros such as WIFEXITED()
- * and WEXITSTATUS() must be used to evaluate the exit status.
+ * If @exit_status is non-%NULL, the platform-specific exit status of
+ * the child is stored there; see the documentation of
+ * g_spawn_check_exit_status() for how to use and interpret this.
  * 
  * On Windows, please note the implications of g_shell_parse_argv()
  * parsing @command_line. Parsing is done according to Unix shell rules, not 
@@ -793,6 +801,106 @@ g_spawn_command_line_async (const gchar *command_line,
   return retval;
 }
 
+/**
+ * g_spawn_check_exit_status:
+ * @exit_status: An exit code as returned from g_spawn_sync()
+ * @error: a #GError
+ *
+ * Set @error if @exit_status indicates the child exited abnormally
+ * (e.g. with a nonzero exit code, or via a fatal signal).
+ *
+ * The g_spawn_sync() and g_child_watch_add() family of APIs return an
+ * exit status for subprocesses encoded in a platform-specific way.
+ * On Unix, this is guaranteed to be in the same format
+ * <literal>waitpid(2)</literal> returns, and on Windows it is
+ * guaranteed to be the result of
+ * <literal>GetExitCodeProcess()</literal>.  Prior to the introduction
+ * of this function in GLib 2.34, interpreting @exit_status required
+ * use of platform-specific APIs, which is problematic for software
+ * using GLib as a cross-platform layer.
+ *
+ * Additionally, many programs simply want to determine whether or not
+ * the child exited successfully, and either propagate a #GError or
+ * print a message to standard error.  In that common case, this
+ * function can be used.  Note that the error message in @error will
+ * contain human-readable information about the exit status.
+ *
+ * The <literal>domain</literal> and <literal>code</literal> of @error
+ * have special semantics in the case where the process has an "exit
+ * code", as opposed to being killed by a signal.  On Unix, this
+ * happens if <literal>WIFEXITED</literal> would be true of
+ * @exit_status.  On Windows, it is always the case.
+ *
+ * The special semantics are that the actual exit code will be the
+ * code set in @error, and the domain will be %G_SPAWN_EXIT_ERROR.
+ * This allows you to differentiate between different exit codes.
+ *
+ * If the process was terminated by some means other than an exit
+ * status, the domain will be %G_SPAWN_ERROR, and the code will be
+ * %G_SPAWN_ERROR_FAILED.
+ *
+ * This function just offers convenience; you can of course also check
+ * the available platform via a macro such as %G_OS_UNIX, and use
+ * <literal>WIFEXITED()</literal> and <literal>WEXITSTATUS()</literal>
+ * on @exit_status directly.  Do not attempt to scan or parse the
+ * error message string; it may be translated and/or change in future
+ * versions of GLib.
+ *
+ * Returns: %TRUE if child exited successfully, %FALSE otherwise (and @error will be set)
+ * Since: 2.34
+ */
+gboolean
+g_spawn_check_exit_status (gint      exit_status,
+			   GError  **error)
+{
+  gboolean ret = FALSE;
+
+#ifdef G_OS_UNIX
+  if (WIFEXITED (exit_status))
+    {
+      if (WEXITSTATUS (exit_status) != 0)
+	{
+	  g_set_error (error, G_SPAWN_EXIT_ERROR, WEXITSTATUS (exit_status),
+		       _("Child process exited with code %ld"),
+		       (long) WEXITSTATUS (exit_status));
+	  goto out;
+	}
+    }
+  else if (WIFSIGNALED (exit_status))
+    {
+      g_set_error (error, G_SPAWN_ERROR, G_SPAWN_ERROR_FAILED,
+		   _("Child process killed by signal %ld"),
+		   (long) WTERMSIG (exit_status));
+      goto out;
+    }
+  else if (WIFSTOPPED (exit_status))
+    {
+      g_set_error (error, G_SPAWN_ERROR, G_SPAWN_ERROR_FAILED,
+		   _("Child process stopped by signal %ld"),
+		   (long) WSTOPSIG (exit_status));
+      goto out;
+    }
+  else
+    {
+      g_set_error (error, G_SPAWN_ERROR, G_SPAWN_ERROR_FAILED,
+		   _("Child process exited abnormally"));
+      goto out;
+    }
+#else
+  if (exit_status != 0)
+    {
+      g_set_error (error, G_SPAWN_EXIT_ERROR, exit_status,
+		   _("Child process exited with code %ld"),
+		   (long) exit_status);
+      goto out;
+    }
+#endif
+
+  ret = TRUE;
+ out:
+  return ret;
+}
+
 static gint
 exec_err_to_g_error (gint en)
 {

glib/gspawn.h

@@ -96,6 +96,14 @@ typedef enum
                               */
 } GSpawnError;
 
+/**
+ * G_SPAWN_EXIT_ERROR:
+ *
+ * Error domain used by g_spawn_check_exit_status().  The code
+ * will be the the program exit code.
+ */
+#define G_SPAWN_EXIT_ERROR g_spawn_exit_error_quark ()
+
 /**
  * GSpawnChildSetupFunc:
  * @user_data: user data to pass to the function.
@@ -176,6 +184,7 @@ typedef enum
 } GSpawnFlags;
 
 GQuark g_spawn_error_quark (void);
+GQuark g_spawn_exit_error_quark (void);
 
 #ifndef __GTK_DOC_IGNORE__
 #ifdef G_OS_WIN32
@@ -236,6 +245,10 @@ gboolean g_spawn_command_line_sync  (const gchar          *command_line,
 gboolean g_spawn_command_line_async (const gchar          *command_line,
                                      GError              **error);
 
+GLIB_AVAILABLE_IN_2_34
+gboolean g_spawn_check_exit_status (gint      exit_status,
+				    GError  **error);
+
 void g_spawn_close_pid (GPid pid);
 
 G_END_DECLS