Merge pull request #213 from bgamari/wip/refactor

Various fixes for `posix_spawn` refactoring

Author: snoyberg

cbits/posix/find_executable.c

@@ -11,7 +11,7 @@
 
 #include "common.h"
 
-// the below is only necessary when we don't have execvpe.
+// the below is only necessary when we need to emulate execvpe.
 #if !defined(HAVE_execvpe)
 
 /* Return true if the given file exists and is an executable. */
@@ -28,7 +28,16 @@ static char *find_in_search_path(char *search_path, const char *filename) {
     char *tokbuf;
     char *path = strtok_r(search_path, ":", &tokbuf);
     while (path != NULL) {
+	// N.B. gcc 6.3.0, used by Debian 9, inexplicably warns that `path`
+	// may not be initialised with -Wall.  Silence this warning. See #210.
+#if defined(__GNUC__) && __GNUC__ == 6 && __GNUC_MINOR__ == 3
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wmaybe-uninitialized"
+#endif
         const int tmp_len = filename_len + 1 + strlen(path) + 1;
+#if defined(__GNUC__) && __GNUC__ == 6 && __GNUC_MINOR__ == 3
+#pragma GCC diagnostic pop
+#endif
         char *tmp = malloc(tmp_len);
         snprintf(tmp, tmp_len, "%s/%s", path, filename);
         if (is_executable(tmp)) {

cbits/posix/fork_exec.c

@@ -62,7 +62,6 @@ setup_std_handle_fork(int fd,
     case STD_HANDLE_CLOSE:
         if (close(fd) == -1) {
             child_failed(pipe, "close");
-            return -1;
         }
         return 0;
 
@@ -84,7 +83,12 @@ setup_std_handle_fork(int fd,
         if (close(b->use_pipe.parent_end) == -1) {
             child_failed(pipe, "close(parent_end)");
         }
-        break;
+	return 0;
+
+    default:
+	// N.B. this should be unreachable but some compilers apparently can't
+	// see this.
+        child_failed(pipe, "setup_std_handle_fork(invalid behavior)");
     }
 }
 
@@ -219,7 +223,7 @@ do_spawn_fork (char *const args[],
 
         /* Reset the SIGINT/SIGQUIT signal handlers in the child, if requested
          */
-        if (flags & RESET_INT_QUIT_HANDLERS != 0) {
+        if ((flags & RESET_INT_QUIT_HANDLERS) != 0) {
             struct sigaction dfl;
             (void)sigemptyset(&dfl.sa_mask);
             dfl.sa_flags = 0;

cbits/posix/posix_spawn.c

@@ -64,6 +64,13 @@ setup_std_handle_spawn (int fd,
             return -1;
         }
         return 0;
+
+    default:
+	// N.B. this should be unreachable
+	// but some compilers apparently can't
+	// see this.
+        *failed_doing = "posix_spawn_file_actions_addclose(invalid behavior)";
+        return -1;
     }
 }
 
@@ -143,11 +150,13 @@ do_spawn_posix (char *const args[],
 #endif
     }
 
-#if defined(HAVE_POSIX_SPAWN_SETPGROUP)
     if ((flags & RUN_PROCESS_IN_NEW_GROUP) != 0) {
+#if defined(HAVE_POSIX_SPAWN_SETPGROUP)
         spawn_flags |= POSIX_SPAWN_SETPGROUP;
-    }
+#else
+	goto not_supported;
 #endif
+    }
 
     if (setup_std_handle_spawn(STDIN_FILENO,  stdInHdl,  &fa, failed_doing) != 0) {
         goto fail;

configure.ac

@@ -18,9 +18,18 @@ AC_CHECK_FUNCS([execvpe])
 
 # posix_spawn checks
 AC_CHECK_HEADERS([spawn.h])
-AC_CHECK_FUNCS([posix_spawnp posix_spawn_file_actions_addchdir])
-AC_CHECK_DECLS([POSIX_SPAWN_SETSID, POSIX_SPAWN_SETSID_NP])
-AC_CHECK_DECLS([POSIX_SPAWN_SETPGROUP])
+AC_CHECK_FUNCS([posix_spawnp posix_spawn_file_actions_addchdir],[],[],[
+  #define _GNU_SOURCE
+  #include <spawn.h>
+])
+AC_CHECK_DECLS([POSIX_SPAWN_SETSID, POSIX_SPAWN_SETSID_NP],[],[],[
+  #define _GNU_SOURCE
+  #include <spawn.h>
+])
+AC_CHECK_DECLS([POSIX_SPAWN_SETPGROUP],[],[],[
+  #define _GNU_SOURCE
+  #include <spawn.h>
+])
 
 FP_CHECK_CONSTS([SIG_DFL SIG_IGN])
 

tests/all.T

@@ -38,7 +38,10 @@ test('T3994', [only_ways(['threaded1','threaded2']),
 test('T4889', normal, compile_and_run, [''])
 
 test('process009', when(opsys('mingw32'), skip), compile_and_run, [''])
-test('process010', normalise_exec, compile_and_run, [''])
+test('process010', [
+    normalise_fun(lambda s: s.replace('illegal operation (Inappropriate ioctl for device)', 'does not exist (No such file or directory)')),
+    normalise_exec
+], compile_and_run, [''])
 test('process011', when(opsys('mingw32'), skip), compile_and_run, [''])
 
 test('T8343', normal, compile_and_run, [''])