Skip to content

Various fixes for posix_spawn refactoring #213

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Jul 20, 2021
Merged
11 changes: 10 additions & 1 deletion cbits/posix/find_executable.c
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand All @@ -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)) {
Expand Down
10 changes: 7 additions & 3 deletions cbits/posix/fork_exec.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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)");
}
}

Expand Down Expand Up @@ -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;
Expand Down
13 changes: 11 additions & 2 deletions cbits/posix/posix_spawn.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}

Expand Down Expand Up @@ -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;
Expand Down
15 changes: 12 additions & 3 deletions configure.ac
Original file line number Diff line number Diff line change
Expand Up @@ -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])

Expand Down
5 changes: 4 additions & 1 deletion tests/all.T
Original file line number Diff line number Diff line change
Expand Up @@ -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, [''])