From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Josselin Poiret via "Bug reports for GUILE, GNU's Ubiquitous Extension Language" Newsgroups: gmane.lisp.guile.bugs Subject: bug#52835: [PATCH v8 2/2] Make system* and piped-process internally use spawn. Date: Sat, 7 Jan 2023 17:07:47 +0100 Message-ID: References: <54884b48615fa3291c637eda80e02f94c359485f.1673107558.git.dev@jpoiret.xyz> Reply-To: Josselin Poiret Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="37685"; mail-complaints-to="usenet@ciao.gmane.io" Cc: 52835@debbugs.gnu.org To: Josselin Poiret , Ludovic =?UTF-8?Q?Court=C3=A8s?= Original-X-From: bug-guile-bounces+guile-bugs=m.gmane-mx.org@gnu.org Sat Jan 07 17:08:32 2023 Return-path: Envelope-to: guile-bugs@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1pEBkG-0009e4-CM for guile-bugs@m.gmane-mx.org; Sat, 07 Jan 2023 17:08:32 +0100 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1pEBjo-0005bV-Vk; Sat, 07 Jan 2023 11:08:05 -0500 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1pEBjn-0005aq-CH for bug-guile@gnu.org; Sat, 07 Jan 2023 11:08:03 -0500 Original-Received: from debbugs.gnu.org ([209.51.188.43]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1pEBjn-0000TX-3Z for bug-guile@gnu.org; Sat, 07 Jan 2023 11:08:03 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1pEBjm-0006de-VD for bug-guile@gnu.org; Sat, 07 Jan 2023 11:08:02 -0500 X-Loop: help-debbugs@gnu.org Resent-From: Josselin Poiret Original-Sender: "Debbugs-submit" Resent-CC: bug-guile@gnu.org Resent-Date: Sat, 07 Jan 2023 16:08:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 52835 X-GNU-PR-Package: guile X-GNU-PR-Keywords: patch Original-Received: via spool by 52835-submit@debbugs.gnu.org id=B52835.167310767525489 (code B ref 52835); Sat, 07 Jan 2023 16:08:02 +0000 Original-Received: (at 52835) by debbugs.gnu.org; 7 Jan 2023 16:07:55 +0000 Original-Received: from localhost ([127.0.0.1]:58778 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1pEBje-0006d2-Su for submit@debbugs.gnu.org; Sat, 07 Jan 2023 11:07:55 -0500 Original-Received: from jpoiret.xyz ([206.189.101.64]:50722) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1pEBjb-0006cm-Ql for 52835@debbugs.gnu.org; Sat, 07 Jan 2023 11:07:52 -0500 Original-Received: from authenticated-user (jpoiret.xyz [206.189.101.64]) by jpoiret.xyz (Postfix) with ESMTPA id A11FC185318; Sat, 7 Jan 2023 16:07:50 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=jpoiret.xyz; s=dkim; t=1673107671; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=gE7CwTy11pqrqawSZQk2AWa2T5DSIirG2e8sxd+l9nc=; b=ZGrK6dgG719jaRibfztDsYgJhnOJD/NU070khDKJUXT3lXNrsovZEv0HT0ryW4IM73S7s8 06D1Xv/DrciDSpqC64bh4UowjIa75yQfQmKd6/yp0MbYsv8Wq6czZ0g/hjNA42IjJVwOIt iJjwoGNgKcB760Z2DWhEgLZl2olcylVZwnMIVFgS1w4DdleiLiswK7DizWinpMoTwRy/ee W0591vcT/8mIxEz+KWBy1e9OLaIxrx3y17ZTBHX8WFwxjFKcK61YusDvHk6BFjGIUfrP74 dDzpqsqRr4GENAaXjOI7FAzeSD8qoMzxLz9OsAw1+FargApJuCPVOPWNUKILLg== In-Reply-To: <54884b48615fa3291c637eda80e02f94c359485f.1673107558.git.dev@jpoiret.xyz> Authentication-Results: jpoiret.xyz; auth=pass smtp.auth=jpoiret@jpoiret.xyz smtp.mailfrom=dev@jpoiret.xyz X-Spamd-Bar: --- X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-guile@gnu.org List-Id: "Bug reports for GUILE, GNU's Ubiquitous Extension Language" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-guile-bounces+guile-bugs=m.gmane-mx.org@gnu.org Original-Sender: bug-guile-bounces+guile-bugs=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.lisp.guile.bugs:10501 Archived-At: * libguile/posix.c (scm_system_star, scm_piped_process): Use do_spawn. (start_child): Remove function. Co-authored-by: Ludovic Courtès --- libguile/posix.c | 231 ++++++++++++++++------------------------------- 1 file changed, 77 insertions(+), 154 deletions(-) diff --git a/libguile/posix.c b/libguile/posix.c index f79875075..2c6c3c84a 100644 --- a/libguile/posix.c +++ b/libguile/posix.c @@ -78,6 +78,7 @@ #include "threads.h" #include "values.h" #include "vectors.h" +#include "verify.h" #include "version.h" #if (SCM_ENABLE_DEPRECATED == 1) @@ -96,6 +97,13 @@ # define WIFEXITED(stat_val) (((stat_val) & 255) == 0) #endif +#ifndef W_EXITCODE +/* Macro for constructing a status value. Found in glibc. */ +# define W_EXITCODE(ret, sig) ((ret) << 8 | (sig)) +#endif +verify (WEXITSTATUS (W_EXITCODE (127, 0)) == 127); + + #include #ifdef HAVE_GRP_H @@ -1309,125 +1317,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 - static pid_t do_spawn (char *exec_file, char **exec_argv, char **exec_env, int in, int out, int err, int spawnp) @@ -1580,18 +1469,18 @@ SCM_DEFINE (scm_spawn_process, "spawn", 1, 0, 1, } #undef FUNC_NAME -#ifdef HAVE_START_CHILD -static SCM -scm_piped_process (SCM prog, SCM args, SCM from, SCM to) +#ifdef HAVE_FORK +static int +piped_process (pid_t *pid, SCM prog, SCM args, SCM from, SCM to) #define FUNC_NAME "piped-process" { int reading, writing; int c2p[2]; /* Child to parent. */ int p2c[2]; /* Parent to child. */ int in = -1, out = -1, err = -1; - int pid; char *exec_file; char **exec_argv; + char **exec_env = environ; exec_file = scm_to_locale_string (prog); exec_argv = scm_i_allocate_string_pointers (scm_cons (prog, args)); @@ -1624,32 +1513,57 @@ scm_piped_process (SCM prog, SCM args, SCM from, SCM to) in = SCM_FPORT_FDES (port); } - pid = start_child (exec_file, exec_argv, reading, c2p, writing, p2c, - in, out, err); - - if (pid == -1) - { - int errno_save = errno; - free (exec_file); - if (reading) - { - close (c2p[0]); - close (c2p[1]); - } - if (writing) - { - close (p2c[0]); - close (p2c[1]); - } - errno = errno_save; - SCM_SYSERROR; - } + *pid = do_spawn (exec_file, exec_argv, exec_env, in, out, err, 1); + int errno_save = (*pid < 0) ? errno : 0; if (reading) close (c2p[1]); if (writing) close (p2c[0]); + if (*pid == -1) + switch (errno_save) + { + /* Errors that seemingly come from fork. */ + case EAGAIN: + case ENOMEM: + case ENOSYS: + errno = err; + free (exec_file); + SCM_SYSERROR; + break; + + default: /* ENOENT, etc. */ + /* Create a dummy process that exits with value 127. */ + dprintf (err, "In execvp of %s: %s\n", exec_file, + strerror (errno_save)); + } + + free (exec_file); + + return errno_save; +} +#undef FUNC_NAME + +static SCM +scm_piped_process (SCM prog, SCM args, SCM from, SCM to) +#define FUNC_NAME "piped-process" +{ + pid_t pid; + + (void) piped_process (&pid, prog, args, from, to); + if (pid == -1) + { + /* Create a dummy process that exits with value 127 to mimic the + previous fork + exec implementation. TODO: This is a + compatibility shim to remove in the next stable series. */ + pid = fork (); + if (pid == -1) + SCM_SYSERROR; + if (pid == 0) + _exit (127); + } + return scm_from_int (pid); } #undef FUNC_NAME @@ -1695,8 +1609,9 @@ SCM_DEFINE (scm_system_star, "system*", 0, 0, 1, "Example: (system* \"echo\" \"foo\" \"bar\")") #define FUNC_NAME s_scm_system_star { - SCM prog, pid; - int status, wait_result; + SCM prog; + pid_t pid; + int err, status, wait_result; if (scm_is_null (args)) SCM_WRONG_NUM_ARGS (); @@ -1714,17 +1629,27 @@ SCM_DEFINE (scm_system_star, "system*", 0, 0, 1, SCM_UNDEFINED); #endif - pid = scm_piped_process (prog, args, SCM_UNDEFINED, SCM_UNDEFINED); - SCM_SYSCALL (wait_result = waitpid (scm_to_int (pid), &status, 0)); - if (wait_result == -1) - SCM_SYSERROR; + err = piped_process (&pid, prog, args, + SCM_UNDEFINED, SCM_UNDEFINED); + if (err != 0) + /* ERR might be ENOENT or similar. For backward compatibility with + the previous implementation based on fork + exec, pretend the + child process exited with code 127. TODO: Remove this + compatibility shim in the next stable series. */ + status = W_EXITCODE (127, 0); + else + { + SCM_SYSCALL (wait_result = waitpid (pid, &status, 0)); + if (wait_result == -1) + SCM_SYSERROR; + } scm_dynwind_end (); return scm_from_int (status); } #undef FUNC_NAME -#endif /* HAVE_START_CHILD */ +#endif /* HAVE_FORK */ #ifdef HAVE_UNAME SCM_DEFINE (scm_uname, "uname", 0, 0, 0, @@ -2570,13 +2495,13 @@ SCM_DEFINE (scm_gethostname, "gethostname", 0, 0, 0, #endif /* HAVE_GETHOSTNAME */ -#ifdef HAVE_START_CHILD +#ifdef HAVE_FORK static void scm_init_popen (void) { scm_c_define_gsubr ("piped-process", 2, 2, 0, scm_piped_process); } -#endif /* HAVE_START_CHILD */ +#endif /* HAVE_FORK */ void scm_init_posix () @@ -2694,8 +2619,6 @@ scm_init_posix () #ifdef HAVE_FORK scm_add_feature ("fork"); -#endif /* HAVE_FORK */ -#ifdef HAVE_START_CHILD scm_add_feature ("popen"); scm_c_register_extension ("libguile-" SCM_EFFECTIVE_VERSION, "scm_init_popen", -- 2.38.1