From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Paul Eggert Newsgroups: gmane.emacs.bugs Subject: bug#14803: Setting close-on-exec flag consistently Date: Sat, 06 Jul 2013 10:18:07 -0700 Organization: UCLA Computer Science Department Message-ID: <51D8514F.8090307@cs.ucla.edu> References: <51D7DFAF.3030500@cs.ucla.edu> <83obafylf1.fsf@gnu.org> NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Trace: ger.gmane.org 1373131152 32325 80.91.229.3 (6 Jul 2013 17:19:12 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Sat, 6 Jul 2013 17:19:12 +0000 (UTC) Cc: 14803@debbugs.gnu.org To: Eli Zaretskii Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Sat Jul 06 19:19:12 2013 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by plane.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1UvW8d-0003WM-I8 for geb-bug-gnu-emacs@m.gmane.org; Sat, 06 Jul 2013 19:19:11 +0200 Original-Received: from localhost ([::1]:59250 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UvW8d-00018D-40 for geb-bug-gnu-emacs@m.gmane.org; Sat, 06 Jul 2013 13:19:11 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:44939) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UvW8Y-00017u-La for bug-gnu-emacs@gnu.org; Sat, 06 Jul 2013 13:19:07 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UvW8U-0008Ma-Sf for bug-gnu-emacs@gnu.org; Sat, 06 Jul 2013 13:19:06 -0400 Original-Received: from debbugs.gnu.org ([140.186.70.43]:40681) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UvW8U-0008M4-Q3 for bug-gnu-emacs@gnu.org; Sat, 06 Jul 2013 13:19:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.80) (envelope-from ) id 1UvW8U-0002gQ-Bn for bug-gnu-emacs@gnu.org; Sat, 06 Jul 2013 13:19:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Paul Eggert Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sat, 06 Jul 2013 17:19:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 14803 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch Original-Received: via spool by 14803-submit@debbugs.gnu.org id=B14803.137313110310250 (code B ref 14803); Sat, 06 Jul 2013 17:19:02 +0000 Original-Received: (at 14803) by debbugs.gnu.org; 6 Jul 2013 17:18:23 +0000 Original-Received: from localhost ([127.0.0.1]:34994 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1UvW7p-0002fG-Ux for submit@debbugs.gnu.org; Sat, 06 Jul 2013 13:18:22 -0400 Original-Received: from smtp.cs.ucla.edu ([131.179.128.62]:45769) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1UvW7m-0002es-Fw for 14803@debbugs.gnu.org; Sat, 06 Jul 2013 13:18:19 -0400 Original-Received: from localhost (localhost.localdomain [127.0.0.1]) by smtp.cs.ucla.edu (Postfix) with ESMTP id AB2E8A60039; Sat, 6 Jul 2013 10:18:12 -0700 (PDT) X-Virus-Scanned: amavisd-new at smtp.cs.ucla.edu Original-Received: from smtp.cs.ucla.edu ([127.0.0.1]) by localhost (smtp.cs.ucla.edu [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id LhVhJwyPForA; Sat, 6 Jul 2013 10:18:12 -0700 (PDT) Original-Received: from [192.168.1.9] (pool-71-108-49-126.lsanca.fios.verizon.net [71.108.49.126]) by smtp.cs.ucla.edu (Postfix) with ESMTPSA id EBD1DA60001; Sat, 6 Jul 2013 10:18:11 -0700 (PDT) User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130623 Thunderbird/17.0.7 In-Reply-To: <83obafylf1.fsf@gnu.org> X-Enigmail-Version: 1.5.1 X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.15 Precedence: list X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 140.186.70.43 X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.bugs:76005 Archived-At: On 07/06/2013 04:31 AM, Eli Zaretskii wrote: > The Windows build has its own implementations > of 'pipe' and 'socket' (because it needs to make them behave like > Posix APIs, and also because it needs to watch each subprocess and > each socket for the benefit of 'select', 'waitpid', SIGCHLD, etc.). > So the replacements from gnulib won't do. The gnulib replacements do that sort of thing too. Emacs uses a hybrid approach, which uses gnulib for some things and its own (pre-gnulib) solution for others, and it should be possible to keep the hybrid working in this case. In the long run it may be better to merge the two approaches rather than maintain a hybrid, but that issue doesn't need to be addressed now. > Socket handles _are_ inherited, but making them non-inheritable is not > easy (some say impossible in the general case), and since no one > complained till now, I'm willing to live with that until we hear some > real problems. That's the intent of this patch. On systems like w32 that lack SOCK_CLOEXEC, the code creates sockets just as it did before (using plain 'socket' and 'accept' rather than 'socket' and 'accept4' with SOCK_CLOEXEC). The only difference is that the proposed code then invokes fcntl (fd, F_SETFD, FD_CLOEXEC) to set the close-on-exec flag; this is needed on POSIXish systems that don't have SOCK_CLOEXEC. From what you say, on w32 platforms it's OK if this is a no-op. w32.c's fcntl currently returns SOCKET_ERROR for this fcntl call, which should be fine, since Emacs doesn't check for an error. > For mktemp/mkstemp in filelock.c, sys_open in w32.c already makes all > handles not inheritable by default, so that problem doesn't exist, > either. This should be OK, since the w32 port doesn't define HAVE_MKOSTEMP, so Emacs should use mkstemp or mktemp as it did before. Emacs will now invoke fcntl (fd, F_SETFD, FD_CLOEXEC) on the resulting file descriptor but that should be OK as described above. > As for emacs_open, it needs to use O_NOINHERIT instead of O_CLOEXEC > (or just use zero, since w32.c already adds the O_NOINHERIT flag in > sys_open). The proposed patch does this, as fcntl.h should #define O_CLOEXEC to O_NOINHERIT on w32. > So I think we should simply not compile lib/fcntl.c and lib/pipe2.c > (including their dependency modules) on MS-Windows. That should be OK. w32.c will need minor tweaks to support F_DUPFD_CLOEXEC and pipe2, something along the following lines. === modified file 'nt/ChangeLog' --- nt/ChangeLog 2013-07-06 09:44:23 +0000 +++ nt/ChangeLog 2013-07-06 16:55:23 +0000 @@ -3,6 +3,8 @@ Make file descriptors close-on-exec when possible (Bug#14803). * gnulib.mk: Remove empty gl_GNULIB_ENABLED_verify section; otherwise, gnulib-tool complains given close-on-exec changes. + * inc/ms-w32.h (pipe): Remove. + (pipe2): New macro. 2013-06-25 Juanma Barranquero === modified file 'nt/inc/ms-w32.h' --- nt/inc/ms-w32.h 2013-05-15 17:06:26 +0000 +++ nt/inc/ms-w32.h 2013-07-06 16:55:23 +0000 @@ -211,7 +211,7 @@ #define mktemp sys_mktemp #undef open #define open sys_open -#define pipe sys_pipe +#define pipe2 sys_pipe2 #undef read #define read sys_read #define rename sys_rename === modified file 'src/ChangeLog' --- src/ChangeLog 2013-07-06 16:37:12 +0000 +++ src/ChangeLog 2013-07-06 17:12:42 +0000 @@ -33,6 +33,8 @@ Make newly-created socket close-on-exec. * sysdep.c (emacs_open, emacs_fopen): Make new-created descriptor close-on-exec. + * w32.c (fcntl): Support F_DUPFD_CLOEXEC well enough for Emacs. + * w32.c, w32.h (sys_pipe2): Rename from 'pipe', with new flags arg. 2013-07-06 Eli Zaretskii === modified file 'src/w32.c' --- src/w32.c 2013-06-21 20:11:44 +0000 +++ src/w32.c 2013-07-06 17:12:42 +0000 @@ -6719,10 +6719,16 @@ } /* Windows does not have an fcntl function. Provide an implementation - solely for making sockets non-blocking. */ + good enough for Emacs. */ int fcntl (int s, int cmd, int options) { + /* In the w32 Emacs port, fcntl (fd, F_DUPFD_CLOEXEC, fd1) is always + invoked in a context where fd1 is closed and all descriptors less + than fd1 are open, so sys_dup is an adequate implementation. */ + if (cmd == F_DUPFD_CLOEXEC) + return sys_dup (s); + if (winsock_lib == NULL) { errno = ENETDOWN; @@ -6864,13 +6870,14 @@ return rc; } -/* Unix pipe() has only one arg */ int -sys_pipe (int * phandles) +sys_pipe2 (int * phandles, int pipe2_flags) { int rc; unsigned flags; + eassert (pipe2_flags == O_CLOEXEC); + /* make pipe handles non-inheritable; when we spawn a child, we replace the relevant handle with an inheritable one. Also put pipes into binary mode; we will do text mode translation ourselves === modified file 'src/w32.h' --- src/w32.h 2013-03-05 22:35:41 +0000 +++ src/w32.h 2013-07-06 16:55:23 +0000 @@ -188,7 +188,7 @@ extern int fchmod (int, mode_t); extern int sys_rename_replace (char const *, char const *, BOOL); -extern int sys_pipe (int *); +extern int sys_pipe2 (int *, int); extern void set_process_dir (char *); extern int sys_spawnve (int, char *, char **, char **);