gspawn: treat all negative fds as unset

Philip Withnall suggests that glib should treat all negative
file descriptors as unset/invalid, rather than explicitly requiring
them to be -1.

This can simplify the logic for some users of this code.

https://gitlab.gnome.org/GNOME/gnome-shell/merge_requests/132

2 files changed, 15 insertions, 9 deletions

diff --git a/glib/gspawn.c b/glib/gspawn.c
index d84433497..dfd97b1a8 100644
--- a/glib/gspawn.c
+++ b/glib/gspawn.c
@@ -843,12 +843,12 @@ g_spawn_async_with_fds (const gchar          *working_directory,
                         GError              **error)
 {
   g_return_val_if_fail (argv != NULL, FALSE);
-  g_return_val_if_fail (stdout_fd == -1 ||
+  g_return_val_if_fail (stdout_fd < 0 ||
                         !(flags & G_SPAWN_STDOUT_TO_DEV_NULL), FALSE);
-  g_return_val_if_fail (stderr_fd == -1 ||
+  g_return_val_if_fail (stderr_fd < 0 ||
                         !(flags & G_SPAWN_STDERR_TO_DEV_NULL), FALSE);
   /* can't inherit stdin if we have an input pipe. */
-  g_return_val_if_fail (stdin_fd == -1 ||
+  g_return_val_if_fail (stdin_fd < 0 ||
                         !(flags & G_SPAWN_CHILD_INHERITS_STDIN), FALSE);
 
   return fork_exec_with_fds (!(flags & G_SPAWN_DO_NOT_REAP_CHILD),
diff --git a/glib/tests/spawn-singlethread.c b/glib/tests/spawn-singlethread.c
index f7568ce98..909f702fc 100644
--- a/glib/tests/spawn-singlethread.c
+++ b/glib/tests/spawn-singlethread.c
@@ -183,13 +183,15 @@ test_spawn_async_with_fds (void)
 
   /* Each test has 3 variable parameters: stdin, stdout, stderr */
   enum fd_type {
-    NO_FD,        /* don't pass a fd */
+    NO_FD,        /* pass fd -1 (unset) */
+    FD_NEGATIVE,  /* pass fd of negative value (equivalent to unset) */
     PIPE,         /* pass fd of new/unique pipe */
     STDOUT_PIPE,  /* pass the same pipe as stdout */
   } tests[][3] = {
-    { NO_FD, NO_FD, NO_FD },      /* Test with no fds passed */
-    { PIPE, PIPE, PIPE },         /* Test with unique fds passed */
-    { NO_FD, PIPE, STDOUT_PIPE }, /* Test the same fd for stdout + stderr */
+    { NO_FD, NO_FD, NO_FD },       /* Test with no fds passed */
+    { NO_FD, FD_NEGATIVE, NO_FD }, /* Test another negative fd value */
+    { PIPE, PIPE, PIPE },          /* Test with unique fds passed */
+    { NO_FD, PIPE, STDOUT_PIPE },  /* Test the same fd for stdout + stderr */
   };
 
   arg = g_strdup_printf ("thread %d", tnum);
@@ -220,6 +222,10 @@ test_spawn_async_with_fds (void)
               test_pipe[j][0] = -1;
               test_pipe[j][1] = -1;
               break;
+            case FD_NEGATIVE:
+              test_pipe[j][0] = -5;
+              test_pipe[j][1] = -5;
+              break;
             case PIPE:
 #ifdef G_OS_UNIX
               g_unix_open_pipe (test_pipe[j], FD_CLOEXEC, &error);
@@ -261,7 +267,7 @@ test_spawn_async_with_fds (void)
       g_source_attach (source, context);
       g_source_unref (source);
 
-      if (test_pipe[1][0] != -1)
+      if (test_pipe[1][0] >= 0)
         {
           channel = g_io_channel_unix_new (test_pipe[1][0]);
           source = g_io_create_watch (channel, G_IO_IN | G_IO_HUP | G_IO_ERR);
@@ -280,7 +286,7 @@ test_spawn_async_with_fds (void)
 
       g_assert_true (data.child_exited);
 
-      if (test_pipe[1][0] != -1)
+      if (test_pipe[1][0] >= 0)
         {
           /* Check for echo on stdout */
           g_assert_true (data.stdout_done);