* libguile/posix.c (renumber_file_descriptor): Refactor it as dup_handle_error. (dup_handle_error, dup2_handle_error): New functions that wrap around dup and dup2 by retrying on EINTR or EBUSY, as well as erroring out on other errors. (start_child): Close standard file descriptors only after all of them have been dup2'd. --- libguile/posix.c | 82 +++++++++++++++++++++++++++++++----------------- 1 file changed, 53 insertions(+), 29 deletions(-)
diff --git a/libguile/posix.c b/libguile/posix.c index 3ab12b99e..dc3080b3c 100644 --- a/libguile/posix.c +++ b/libguile/posix.c @@ -1280,14 +1280,14 @@ SCM_DEFINE (scm_fork, "primitive-fork", 0, 0, 0, #endif /* HAVE_FORK */ #ifdef HAVE_FORK -/* 'renumber_file_descriptor' is a helper function for 'start_child' - below, and is specialized for that particular environment where it - doesn't make sense to report errors via exceptions. It uses dup(2) - to duplicate the file descriptor FD, closes the original FD, and - returns the new descriptor. If dup(2) fails, print an error message - to ERR and abort. */ +/* 'dup_handle_error' is a helper function for 'start_child' below, and + is specialized for that particular environment where it doesn't make + sense to report errors via exceptions. It uses dup(2) to duplicate + the file descriptor FD, does *not* close the original FD, and returns + the new descriptor. If dup(2) fails, print an error message to ERR + and abort. */ static int -renumber_file_descriptor (int fd, int err) +dup_handle_error (int fd, int err) { int new_fd; @@ -1304,7 +1304,33 @@ renumber_file_descriptor (int fd, int err) _exit (127); /* Use exit status 127, as with other exec errors. */ } - close (fd); + return new_fd; +} + +/* 'dup2_handle_error' is a helper function for 'start_child' below, and + is specialized for that particular environment where it doesn't make + sense to report errors via exceptions. It uses dup2(2) to duplicate + the file descriptor FD, does *not* close the original FD, and returns + the new descriptor. If dup2(2) fails, print an error message to ERR + and abort. */ +static int +dup2_handle_error (int fd, int to, int err) +{ + int new_fd; + + do + new_fd = dup2 (fd, to); + while (new_fd == -1 && (errno == EINTR || errno == EBUSY)); + + if (new_fd == -1) + { + /* At this point we are in the child process before exec. We + cannot safely raise an exception in this environment. */ + const char *msg = strerror (errno); + fprintf (fdopen (err, "a"), "start_child: dup failed: %s\n", msg); + _exit (127); /* Use exit status 127, as with other exec errors. */ + } + return new_fd; } #endif /* HAVE_FORK */ @@ -1357,27 +1383,25 @@ start_child (const char *exec_file, char **exec_argv, if (err == -1) err = open ("/dev/null", O_WRONLY); - if (in > 0) - { - if (out == 0) - out = renumber_file_descriptor (out, err); - if (err == 0) - err = renumber_file_descriptor (err, err); - do dup2 (in, 0); while (errno == EINTR); - close (in); - } - if (out > 1) - { - if (err == 1) - err = renumber_file_descriptor (err, err); - do dup2 (out, 1); while (errno == EINTR); - close (out); - } - if (err > 2) - { - do dup2 (err, 2); while (errno == EINTR); - close (err); - } + /* Dup each non-yet-dup2'd fd that's in the way to the next available fd, + so that we can safely dup2 to 0/1/2 without potentially overwriting + in/out/err. Note that dup2 doesn't do anything if its arguments are + equal. */ + if (out == 0) + out = dup_handle_error (out, err); + if (err == 0) + err = dup_handle_error (err, err); + dup2_handle_error (in, 0, err); + + if (err == 1) + err = dup_handle_error (err, err); + dup2_handle_error (out, 1, err); + + dup2_handle_error (err, 2, err); + + if (in > 2) close (in); + if (out > 2) close (out); + if (err > 2) close (err); execvp (exec_file, exec_argv); -- 2.36.0