bug#52835: [PATCH v5 2/3] Add spawn*.
* libguile/posix.c: Include spawn.h from Gnulib.
(scm_spawn_process): New function.
(scm_init_popen): Define spawn*.
---
libguile/posix.c | 71
1 file changed, 71 insertions(+)
diff --git a/libguile/posix.c b/libguile/posix.c
index f4ca72d3e..5d287ff2a 100644
--- a/libguile/posix.c
+++ b/libguile/posix.c
@@ -32,6 +32,7 @@
#include
#include
#include
+#include
#ifdef HAVE_SCHED_H
# include
@@ -1472,6 +1473,75 @@ scm_piped_process (SCM prog, SCM args, SCM from, SCM to)
}
#undef FUNC_NAME
+static SCM
+scm_spawn_process (SCM prog, SCM args, SCM scm_in, SCM scm_out, SCM scm_err)
+#define FUNC_NAME "spawn*"
+{
+ int in, out, err;
+ int pid;
+ char *exec_file;
+ char **exec_argv;
+ char **exec_env = NULL;
+
+ posix_spawn_file_actions_t actions;
+ posix_spawnattr_t *attrp = NULL;
+
+ exec_file = scm_to_locale_string (prog);
+ exec_argv = scm_i_allocate_string_pointers (args);
+
+ in = scm_to_int (scm_in);
+ out = scm_to_int (scm_out);
+ err = scm_to_int (scm_err);
+
+ int max_fd = 1024;
+
+#if defined (HAVE_GETRLIMIT) && defined (RLIMIT_NOFILE)
+ {
+struct rlimit lim = { 0, 0 };
+if (getrlimit (RLIMIT_NOFILE, &lim) == 0)
+ max_fd = lim.rlim_cur;
+ }
+#endif
+
+ posix_spawn_file_actions_init (&actions);
+
+ int free_fd_slots = 0;
+ int fd_slot[3];
+
+ for (int fdnum = 3;free_fd_slots < 3 && fdnum < max_fd;fdnum++)
+{
+ if (fdnum != in && fdnum != out && fdnum != err)
+{
+ fd_slot[free_fd_slots] = fdnum;
+ free_fd_slots++;
+}
+}
+
+ /* Move the fds out of the way, so that duplicate fds or fds equal
+ to 0, 1, 2 don't trample each other */
+
+ posix_spawn_file_actions_adddup2 (&actions, in, fd_slot[0]);
+ posix_spawn_file_actions_adddup2 (&actions, out, fd_slot[1]);
+ posix_spawn_file_actions_adddup2 (&actions, err, fd_slot[2]);
+ posix_spawn_file_actions_adddup2 (&actions, fd_slot[0], 0);
+ posix_spawn_file_actions_adddup2 (&actions, fd_slot[1], 1);
+ posix_spawn_file_actions_adddup2 (&actions, fd_slot[2], 2);
+
+ while (--max_fd > 2)
+posix_spawn_file_actions_addclose (&actions, max_fd);
+
+ if (posix_spawnp (&pid, exec_file, &actions, attrp, exec_argv, environ) != 0)
+{
+ int errno_save = errno;
+ free (exec_file);
+ errno = errno_save;
+ SCM_SYSERROR;
+}
+
+ return scm_from_int (pid);
+}
+#undef FUNC_NAME
+
static void
restore_sigaction (SCM pair)
{
@@ -2381,6 +2451,7 @@ static void
scm_init_popen (void)
{
scm_c_define_gsubr ("piped-process", 2, 2, 0, scm_piped_process);
+ scm_c_define_gsubr ("spawn*", 5, 0, 0, scm_spawn_process);
}
#endif /* HAVE_START_CHILD */
--
2.37.2
bug#52835: [PATCH v5 3/3] Move popen and posix procedures to spawn*.
* libguile/posix.c (renumber_file_descriptor, start_child,
scm_piped_process): Remove functions.
(scm_port_to_fd_with_default): New helper function.
(scm_system_star): Rewrite using scm_spawn_process.
(scm_init_popen): Remove the definition of piped-process.
(scm_init_posix): Now make popen available unconditionally.
* module/ice-9/popen.scm (port-with-defaults): New helper procedure.
(spawn): New procedure.
(open-process): Rewrite using spawn.
(pipeline): Rewrite using spawn*.
* test-suite/tests/popen.test ("piped-process", "piped-process:
with-output"): Removed tests.
("spawn", "spawn: with output"): Added tests.
* test-suite/tests/posix.test ("http://bugs.gnu.org/13166";, "exit code
for nonexistent file", "https://bugs.gnu.org/55596";): Remove obsolete
tests.
("exception for nonexistent file"): Add test.
---
libguile/posix.c| 218 +++-
module/ice-9/popen.scm | 83 ++
test-suite/tests/popen.test | 14 +--
test-suite/tests/posix.test | 36 +++---
4 files changed, 102 insertions(+), 249 deletions(-)
diff --git a/libguile/posix.c b/libguile/posix.c
index 5d287ff2a..c35346f9f 100644
--- a/libguile/posix.c
+++ b/libguile/posix.c
@@ -73,6 +73,7 @@
#include "fports.h"
#include "gettext.h"
#include "gsubr.h"
+#include "ioext.h"
#include "list.h"
#include "modules.h"
#include "numbers.h"
@@ -1280,199 +1281,6 @@ SCM_DEFINE (scm_fork, "primitive-fork", 0, 0, 0,
#undef FUNC_NAME
#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. */
-static int
-renumber_file_descriptor (int fd, int err)
-{
- int new_fd;
-
- do
-new_fd = dup (fd);
- while (new_fd == -1 && errno == EINTR);
-
- 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. */
-}
-
- close (fd);
- return new_fd;
-}
-#endif /* HAVE_FORK */
-
-#ifdef HAVE_FORK
-#define HAVE_START_CHILD 1
-/* Since Guile uses threads, we have to be very careful to avoid calling
- functions that are not async-signal-safe in the child. That's why
- this function is implemented in C. */
-static pid_t
-start_child (const char *exec_file, char **exec_argv,
-int reading, int c2p[2], int writing, int p2c[2],
- int in, int out, int err)
-{
- int pid;
- int max_fd = 1024;
-
-#if defined (HAVE_GETRLIMIT) && defined (RLIMIT_NOFILE)
- {
-struct rlimit lim = { 0, 0 };
-if (getrlimit (RLIMIT_NOFILE, &lim) == 0)
- max_fd = lim.rlim_cur;
- }
-#endif
-
- pid = fork ();
-
- if (pid != 0)
-/* The parent, with either and error (pid == -1), or the PID of the
- child. Return directly in either case. */
-return pid;
-
- /* The child. */
- if (reading)
-close (c2p[0]);
- if (writing)
-close (p2c[1]);
-
- /* Close all file descriptors in ports inherited from the parent
- except for in, out, and err. Heavy-handed, but robust. */
- while (max_fd--)
-if (max_fd != in && max_fd != out && max_fd != err)
- close (max_fd);
-
- /* Ignore errors on these open() calls. */
- if (in == -1)
-in = open ("/dev/null", O_RDONLY);
- if (out == -1)
-out = open ("/dev/null", O_WRONLY);
- 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);
- if (out > 2)
-close (out);
-}
- if (err > 2)
-{
- do dup2 (err, 2); while (errno == EINTR);
- close (err);
-}
-
- execvp (exec_file, exec_argv);
-
- /* The exec failed! There is nothing sensible to do. */
- {
-const char *msg = strerror (errno);
-fprintf (fdopen (2, "a"), "In execvp of %s: %s\n",
- exec_file, msg);
- }
-
- /* Use exit status 127, like shells in this case, as per POSIX
-
.
*/
- _exit (127);
-
- /* Not reached. */
- return -1;
-}
-#endif
-
-#ifdef HAVE_START_CHILD
-static SCM
-scm_piped_process (SCM prog, SCM args, SCM from, SCM to)
-#define FUNC_NAME "piped-process"
-{
- int read
bug#52835: [PATCH v5 0/3] Move spawning procedures to posix_spawn.
Hi everyone, As was discussed on IRC, if we're going to rewrite a non-negligible part of posix.c, let's at least do it right and use posix_spawn to handle the process spawning side of things. This is quite complex to get right in general (highlighted by this very bug) and so people have already done the hard work for us. Additionally, we use Gnulib's posix_spawn, so that it is available on all supported systems. I then adjusted all the procedures to use posix_spawn instead of scm_piped_process and removed the latter, and the tests in popen.test, posix.test. There are two inderminates here: * I don't have anything other than a Linux system to test. This would need some feedback for at least Mach and win32. * This changes the interfaces (for the better, in my opinion): whenever possible, posix_spawn reports child starting failures as a parent errno, meaning that for eg. non- existing binaries, system* now throws an exception instead of returning a pid that will have an exit status code of 127. This means that existing code that relies on that behavior will need to be changed, the first example being the test suite which I adapted to actually check for exceptions instead. Some tests were removed because they no longer make sense: in posix.test, https://bugs.gnu.org/13166, exit code for nonexistent file and https://bugs.gnu.org/55596 are superseded by "exception for nonexistent file". Also, I have no experience in using Gnulib so I'm not 100% sure I committed exactly the right files, I'd love it if someone could check this is ok. What do you all think about this approach? Josselin Poiret (3): Update gnulib to 0.1.5414-8204d and add posix_spawn, posix_spawnp. Add spawn*. Move popen and posix procedures to spawn*. GNUmakefile|2 +- build-aux/announce-gen | 69 +- build-aux/gendocs.sh | 50 +- build-aux/git-version-gen | 13 +- build-aux/gitlog-to-changelog |4 +- build-aux/gnu-web-doc-update |4 +- build-aux/gnupload |4 +- build-aux/useless-if-before-free |6 +- build-aux/vc-list-files|2 +- doc/gendocs_template |4 +- doc/gendocs_template_min |2 +- gnulib-local/m4/clock_time.m4.diff | 12 +- lib/Makefile.am| 1252 +--- lib/_Noreturn.h|2 +- lib/accept.c |2 +- lib/accept4.c |4 +- lib/access.c | 31 + lib/alignof.h |2 +- lib/alloca.c | 35 - lib/alloca.in.h|2 +- lib/arg-nonnull.h |2 +- lib/arpa_inet.in.h |2 +- lib/asnprintf.c|2 +- lib/assure.h |2 +- lib/attribute.h| 10 +- lib/basename-lgpl.c|2 +- lib/basename-lgpl.h|2 +- lib/binary-io.c|2 +- lib/binary-io.h|4 +- lib/bind.c |2 +- lib/btowc.c|2 +- lib/byteswap.in.h |2 +- lib/c++defs.h |2 +- lib/c-ctype.c |2 +- lib/c-ctype.h |2 +- lib/c-strcase.h|2 +- lib/c-strcasecmp.c |2 +- lib/c-strcaseeq.h |2 +- lib/c-strncasecmp.c|2 +- lib/canonicalize-lgpl.c|2 +- lib/cdefs.h| 76 +- lib/ceil.c |4 +- lib/cloexec.c |2 +- lib/cloexec.h |2 +- lib/close.c|2 +- lib/concat-filename.c | 73 + lib/concat-filename.h | 46 + lib/connect.c |2 +- lib/copysign.c |4 +- lib/dirent.in.h| 24 +- lib/dirfd.c|2 +- lib/dirname-lgpl.c |2 +- lib/dirname.h |2 +- lib/dup2.c |2 +- lib/duplocale.c|4 +- lib/dynarray.h |2 +- lib/eloop-threshold.h |2 +- lib/errno.in.h |2 +- lib/fcntl.c|2 +- lib/fcntl.in.h |
