zsh-workers
 help / color / mirror / code / Atom feed
* <(...), >(...) and fds above 9
@ 2019-07-01 10:00 ` Stephane Chazelas
  2019-07-01 10:08   ` Peter Stephenson
  0 siblings, 1 reply; 16+ messages in thread
From: Stephane Chazelas @ 2019-07-01 10:00 UTC (permalink / raw)
  To: Zsh hackers list

/tmp$ zsh -c 'exec {fd}< a; zmodload zsh/system; sysopen -u fd2 /dev/fd/$fd; echo $fd $fd2; exec 7< a; ls -l /proc/self/fd; cat <(ls -l /proc/self/fd)'
12 13
total 0
lrwx------ 1 stephane stephane 64 Jul  1 10:52 0 -> /dev/pts/15
lrwx------ 1 stephane stephane 64 Jul  1 10:52 1 -> /dev/pts/15
lr-x------ 1 stephane stephane 64 Jul  1 10:52 12 -> /tmp/a
lr-x------ 1 stephane stephane 64 Jul  1 10:52 13 -> /tmp/a
lrwx------ 1 stephane stephane 64 Jul  1 10:52 2 -> /dev/pts/15
lr-x------ 1 stephane stephane 64 Jul  1 10:52 3 -> /proc/464/fd
lr-x------ 1 stephane stephane 64 Jul  1 10:52 7 -> /tmp/a
total 0
lrwx------ 1 stephane stephane 64 Jul  1 10:52 0 -> /dev/null
l-wx------ 1 stephane stephane 64 Jul  1 10:52 1 -> pipe:[2044936]
lrwx------ 1 stephane stephane 64 Jul  1 10:52 2 -> /dev/pts/15
lr-x------ 1 stephane stephane 64 Jul  1 10:52 3 -> /proc/465/fd
lr-x------ 1 stephane stephane 64 Jul  1 10:52 7 -> /tmp/a

It seems fds above 9 are closed in those forms of process substitutions. 

It doesn't happen for the =(...) form of process substitution.

Is that intentional?

-- 
Stephane

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: <(...), >(...) and fds above 9
  2019-07-01 10:00 ` <(...), >(...) and fds above 9 Stephane Chazelas
@ 2019-07-01 10:08   ` Peter Stephenson
  2019-07-01 14:39     ` Bart Schaefer
  0 siblings, 1 reply; 16+ messages in thread
From: Peter Stephenson @ 2019-07-01 10:08 UTC (permalink / raw)
  To: zsh-workers

On Mon, 2019-07-01 at 11:00 +0100, Stephane Chazelas wrote:
> /tmp$ zsh -c 'exec {fd}< a; zmodload zsh/system; sysopen -u fd2 /dev/fd/$fd; echo $fd $fd2; exec 7< a; ls -l /proc/self/fd; cat <(ls -l /proc/self/fd)'
> 12 13
> total 0
> lrwx------ 1 stephane stephane 64 Jul  1 10:52 0 -> /dev/pts/15
> lrwx------ 1 stephane stephane 64 Jul  1 10:52 1 -> /dev/pts/15
> lr-x------ 1 stephane stephane 64 Jul  1 10:52 12 -> /tmp/a
> lr-x------ 1 stephane stephane 64 Jul  1 10:52 13 -> /tmp/a
> lrwx------ 1 stephane stephane 64 Jul  1 10:52 2 -> /dev/pts/15
> lr-x------ 1 stephane stephane 64 Jul  1 10:52 3 -> /proc/464/fd
> lr-x------ 1 stephane stephane 64 Jul  1 10:52 7 -> /tmp/a
> total 0
> lrwx------ 1 stephane stephane 64 Jul  1 10:52 0 -> /dev/null
> l-wx------ 1 stephane stephane 64 Jul  1 10:52 1 -> pipe:[2044936]
> lrwx------ 1 stephane stephane 64 Jul  1 10:52 2 -> /dev/pts/15
> lr-x------ 1 stephane stephane 64 Jul  1 10:52 3 -> /proc/465/fd
> lr-x------ 1 stephane stephane 64 Jul  1 10:52 7 -> /tmp/a
> 
> It seems fds above 9 are closed in those forms of process substitutions. 
> 
> It doesn't happen for the =(...) form of process substitution.
> 
> Is that intentional?

I'd be surprised if there was any deliberate intention to make these different,
but there may be some detail I can't think of.

In general closing file descriptors from 10 up that don't have a good reason
to be open is handled by a call to closem() when forking.  There may
be a difference in the arguments used here, and "a good reason to be open"
is a matter of opinion and documentation rather than logical rigour.

pws


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: <(...), >(...) and fds above 9
  2019-07-01 10:08   ` Peter Stephenson
@ 2019-07-01 14:39     ` Bart Schaefer
  2019-07-01 15:28       ` Peter Stephenson
  0 siblings, 1 reply; 16+ messages in thread
From: Bart Schaefer @ 2019-07-01 14:39 UTC (permalink / raw)
  To: zsh-workers

On Mon, Jul 1, 2019 at 3:09 AM Peter Stephenson
<p.stephenson@samsung.com> wrote:
>
> On Mon, 2019-07-01 at 11:00 +0100, Stephane Chazelas wrote:
> >
> > It seems fds above 9 are closed in those forms of process substitutions.
> >
> > It doesn't happen for the =(...) form of process substitution.
> >
> > Is that intentional?
>
> I'd be surprised if there was any deliberate intention to make these different,
> but there may be some detail I can't think of.

<(...) does
entersubsh(ESUB_ASYNC|ESUB_PGRP, NULL);

whereas =(...) does
entersubsh(ESUB_PGRP|ESUB_NOMONITOR, NULL);

The fds above 9 are only closed for ASYNC, I think.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: <(...), >(...) and fds above 9
  2019-07-01 14:39     ` Bart Schaefer
@ 2019-07-01 15:28       ` Peter Stephenson
  2019-07-01 16:22         ` Stephane Chazelas
  0 siblings, 1 reply; 16+ messages in thread
From: Peter Stephenson @ 2019-07-01 15:28 UTC (permalink / raw)
  To: zsh-workers

On Mon, 2019-07-01 at 07:39 -0700, Bart Schaefer wrote:
> On Mon, Jul 1, 2019 at 3:09 AM Peter Stephenson
> <p.stephenson@samsung.com> wrote:
> > 
> > 
> > On Mon, 2019-07-01 at 11:00 +0100, Stephane Chazelas wrote:
> > > 
> > > 
> > > It seems fds above 9 are closed in those forms of process substitutions.
> > > 
> > > It doesn't happen for the =(...) form of process substitution.
> > > 
> > > Is that intentional?
> > I'd be surprised if there was any deliberate intention to make these different,
> > but there may be some detail I can't think of.
> <(...) does
> entersubsh(ESUB_ASYNC|ESUB_PGRP, NULL);
> 
> whereas =(...) does
> entersubsh(ESUB_PGRP|ESUB_NOMONITOR, NULL);
> 
> The fds above 9 are only closed for ASYNC, I think.

I don't think entersubsh() calls closem() --- it's
usually done at other points, either in execute()
or a special execution function in the case of the
various process-style substitutions.

Looks like =(...) doesn't call closem() at all when
forking, hence the difference in behaviour.  So
=(...) is the odd one out.

pws


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: <(...), >(...) and fds above 9
  2019-07-01 15:28       ` Peter Stephenson
@ 2019-07-01 16:22         ` Stephane Chazelas
  2019-07-01 16:52           ` Peter Stephenson
  0 siblings, 1 reply; 16+ messages in thread
From: Stephane Chazelas @ 2019-07-01 16:22 UTC (permalink / raw)
  To: Peter Stephenson; +Cc: zsh-workers

2019-07-01 16:28:28 +0100, Peter Stephenson:
[...]
> Looks like =(...) doesn't call closem() at all when
> forking, hence the difference in behaviour.  So
> =(...) is the odd one out.
[...]

But is there a good reason why we should close those fds?

I had to work around it in my case (something like:
join <(sort /dev/fd/$fd) <(sort /dev/fd/$fd2)
)


Another difference is that the stdin redirection to /dev/null is
not done for the =(...) form:

$ zsh -c 'cat =(ls -l /proc/self/fd)'
total 0
lrwx------ 1 stephane stephane 64 Jul  1 17:17 0 -> /dev/pts/13
l-wx------ 1 stephane stephane 64 Jul  1 17:17 1 -> /tmp/zshiMCdiC
lrwx------ 1 stephane stephane 64 Jul  1 17:17 2 -> /dev/pts/13
lr-x------ 1 stephane stephane 64 Jul  1 17:17 3 -> /proc/818/fd
$ zsh -c 'cat <(ls -l /proc/self/fd)'
total 0
lrwx------ 1 stephane stephane 64 Jul  1 17:17 0 -> /dev/null
l-wx------ 1 stephane stephane 64 Jul  1 17:17 1 -> pipe:[2995406]
lrwx------ 1 stephane stephane 64 Jul  1 17:17 2 -> /dev/pts/13
lr-x------ 1 stephane stephane 64 Jul  1 17:17 3 -> /proc/829/fd

-- 
Stephane

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: <(...), >(...) and fds above 9
  2019-07-01 16:22         ` Stephane Chazelas
@ 2019-07-01 16:52           ` Peter Stephenson
  2019-07-01 17:00             ` Peter Stephenson
                               ` (2 more replies)
  0 siblings, 3 replies; 16+ messages in thread
From: Peter Stephenson @ 2019-07-01 16:52 UTC (permalink / raw)
  To: Zsh Hackers' List

On Mon, 2019-07-01 at 17:22 +0100, Stephane Chazelas wrote:
> 2019-07-01 16:28:28 +0100, Peter Stephenson:
> [...]
> > 
> > Looks like =(...) doesn't call closem() at all when
> > forking, hence the difference in behaviour.  So
> > =(...) is the odd one out.
> [...]
> 
> But is there a good reason why we should close those fds?

The shell has no idea whether you're happy to have lingering file
descriptors when executing any old external software, so has to play
safe in general.  It assumes you know what you're doing with file
descriptors 0 to 7 and certain other types, and that others are
dangerous.

{fd}-opened file descriptors are marked FDT_EXTERNAL.  We could default
to leaving those open on fork and document this.  This would then
apply to file descriptors created with zsocket, which I presume you
could think of as being in the same category.

sysopen file descriptors are marked FDT_INTERNAL, which we'd certainly
expect to close.  It looks like we make an exception if you use an
explicit file descriptor number rather than ask for one to be assigned
to a variable, I think because that's constrained to be 0 to 9.  So these
could be changed to FDT_EXTERNAL, too, which would probably be more
consistent.

As I said, this is just based on needs and documentation rather than any
definite logic.

Probably =(...) should use closem() consisent with <(...), too.

pws


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: <(...), >(...) and fds above 9
  2019-07-01 16:52           ` Peter Stephenson
@ 2019-07-01 17:00             ` Peter Stephenson
  2019-07-01 20:00             ` Stephane Chazelas
  2019-07-02  8:59             ` Peter Stephenson
  2 siblings, 0 replies; 16+ messages in thread
From: Peter Stephenson @ 2019-07-01 17:00 UTC (permalink / raw)
  To: Zsh Hackers' List

On Mon, 2019-07-01 at 17:52 +0100, Peter Stephenson wrote:
> sysopen file descriptors are marked FDT_INTERNAL, which we'd certainly
> expect to close.

This must surely be a bug in this case, since the documentation for
sysopen mentions the "cloexec" option for close-on-exec, implying it
doesn't happen otherwise (duh).

pws


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: <(...), >(...) and fds above 9
  2019-07-01 16:52           ` Peter Stephenson
  2019-07-01 17:00             ` Peter Stephenson
@ 2019-07-01 20:00             ` Stephane Chazelas
  2019-07-01 20:06               ` Bart Schaefer
  2019-07-02  8:59             ` Peter Stephenson
  2 siblings, 1 reply; 16+ messages in thread
From: Stephane Chazelas @ 2019-07-01 20:00 UTC (permalink / raw)
  To: Peter Stephenson; +Cc: Zsh Hackers' List

2019-07-01 17:52:53 +0100, Peter Stephenson:
> On Mon, 2019-07-01 at 17:22 +0100, Stephane Chazelas wrote:
> > 2019-07-01 16:28:28 +0100, Peter Stephenson:
> > [...]
> > > 
> > > Looks like =(...) doesn't call closem() at all when
> > > forking, hence the difference in behaviour.  So
> > > =(...) is the odd one out.
> > [...]
> > 
> > But is there a good reason why we should close those fds?
> 
> The shell has no idea whether you're happy to have lingering file
> descriptors when executing any old external software, so has to play
> safe in general.  It assumes you know what you're doing with file
> descriptors 0 to 7 and certain other types, and that others are
> dangerous.
[...]

Thanks, but I don't follow.

In

exec 7< file

I have file open on fd 7 which I can use anywhere, after fork(),
after execve().

With

exec {fd}< file

It's the same except the fd is assigned dynamically but I don't
see how they should be treated differently. Presumably, if I'm
doing that, I want to be able to use that fd later on, most
probably in some command which unless the command happens to be
a builtin involves both a fork() and execve().

In

$ ls -l /proc/self/fd
total 0
lrwx------ 1 chazelas chazelas 64 Jul  1 20:47 0 -> /dev/pts/1
lrwx------ 1 chazelas chazelas 64 Jul  1 20:47 1 -> /dev/pts/1
lr-x------ 1 chazelas chazelas 64 Jul  1 20:47 12 -> /home/chazelas/a
lrwx------ 1 chazelas chazelas 64 Jul  1 20:47 2 -> /dev/pts/1
lr-x------ 1 chazelas chazelas 64 Jul  1 20:47 3 -> /proc/26481/fd

There is both a fork() and a execve() and that fd 12 survived.

It survives in

(ls -l /proc/self/fd)
echo "$(ls -l /proc/self/fd)"
ls -l /proc/self/fd | cat
cat =(ls -l /proc/self/fd)

Not in

ls /proc/self/fd &
coproc ls /proc/self/fd
cat <(ls /proc/self/fd)

It looks like the key is *background* here, and it sounds like
the reason is not so much about what the user expects but
possibly some implementation limitation/consideration, maybe
something to do with job control and/or the fact that some of the
fds above 9 are not user ones but some created internally by zsh
for its own internal soup?

Is it documented anywhere that fds above 9 are closed for all
commands started asynchronously?

-- 
Stephane

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: <(...), >(...) and fds above 9
  2019-07-01 20:00             ` Stephane Chazelas
@ 2019-07-01 20:06               ` Bart Schaefer
  0 siblings, 0 replies; 16+ messages in thread
From: Bart Schaefer @ 2019-07-01 20:06 UTC (permalink / raw)
  To: Peter Stephenson, Zsh Hackers' List

On Mon, Jul 1, 2019 at 1:01 PM Stephane Chazelas
<stephane.chazelas@gmail.com> wrote:
>
> It looks like the key is *background* here, and it sounds like
> the reason is not so much about what the user expects but
> possibly some implementation limitation/consideration
>
> Is it documented anywhere that fds above 9 are closed for all
> commands started asynchronously?

This is a process security thing, to prevent rogue processes that
might be spawned from the shell having access to open files that they
shouldn't.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: <(...), >(...) and fds above 9
  2019-07-01 16:52           ` Peter Stephenson
  2019-07-01 17:00             ` Peter Stephenson
  2019-07-01 20:00             ` Stephane Chazelas
@ 2019-07-02  8:59             ` Peter Stephenson
  2019-07-02 12:20               ` Stephane Chazelas
  2 siblings, 1 reply; 16+ messages in thread
From: Peter Stephenson @ 2019-07-02  8:59 UTC (permalink / raw)
  To: Zsh Hackers' List

On Mon, 2019-07-01 at 17:52 +0100, Peter Stephenson wrote:
> {fd}-opened file descriptors are marked FDT_EXTERNAL.  We could default
> to leaving those open on fork and document this.  This would then
> apply to file descriptors created with zsocket, which I presume you
> could think of as being in the same category.
> 
> sysopen file descriptors are marked FDT_INTERNAL, which we'd certainly
> expect to close.  It looks like we make an exception if you use an
> explicit file descriptor number rather than ask for one to be assigned
> to a variable, I think because that's constrained to be 0 to 9.  So these
> could be changed to FDT_EXTERNAL, too, which would probably be more
> consistent.
> 
> As I said, this is just based on needs and documentation rather than any
> definite logic.
> 
> Probably =(...) should use closem() consisent with <(...), too.

This fixes this up --- I think for file descriptors known to and hence
managed by the user the behaviour of silently closing is probably
uncontroversially unhelpful.  Historically, it's really just there to
ensure the shell doesn't leak its own bilge, so shouldn't really be
visible to the user as an effect.  I've explicitly noted the
file descriptors aren't closed (except with close-on-exec, obviously).

One thing I haven't changed is module-owned file descriptors ---
FDT_MODULE.  So currently, for example, file descriptors owned by
zsh/net/tcp are closed when exec'ing or doing substitutions involving
forks.  The rationale for these file descriptors is a little different
--- they're visible to the user, but they're managed by the module
rather than closed by the user.  So not sure whether to change this.
If we don't it should at least be documented.

pws

diff --git a/Doc/Zsh/mod_socket.yo b/Doc/Zsh/mod_socket.yo
index 867f6081f..78d9254e8 100644
--- a/Doc/Zsh/mod_socket.yo
+++ b/Doc/Zsh/mod_socket.yo
@@ -43,7 +43,8 @@ startitem()
 item(tt(zsocket) tt(-l) [ tt(-v) ] [ tt(-d) var(fd) ] var(filename))(
 tt(zsocket -l) will open a socket listening on var(filename).
 The shell parameter tt(REPLY) will be set to the file descriptor
-associated with that listener.
+associated with that listener.  The file descriptor remains open in subshells
+and forked external executables.
 
 If tt(-d) is specified, its argument
 will be taken as the target file descriptor for
@@ -56,7 +57,8 @@ tt(zsocket -a) will accept an incoming connection
 to the socket associated with var(listenfd).
 The shell parameter tt(REPLY) will
 be set to the file descriptor associated with
-the inbound connection.
+the inbound connection.  The file descriptor remains open in subshells
+and forked external executables.
 
 If tt(-d) is specified, its argument
 will be taken as the target file descriptor for the
diff --git a/Doc/Zsh/mod_system.yo b/Doc/Zsh/mod_system.yo
index 3a85e760f..6292af071 100644
--- a/Doc/Zsh/mod_system.yo
+++ b/Doc/Zsh/mod_system.yo
@@ -45,7 +45,9 @@ specified as a comma-separated list. The following is a list of possible
 options. Note that, depending on the system, some may not be available.
 startitem()
 item(tt(cloexec))(
-mark file to be closed when other programs are executed
+mark file to be closed when other programs are executed (else
+the file descriptor remains open in subshells and forked external
+executables)
 )
 xitem(tt(create))
 item(tt(creat))(
diff --git a/Doc/Zsh/redirect.yo b/Doc/Zsh/redirect.yo
index 7e38cd0c3..13496d8d3 100644
--- a/Doc/Zsh/redirect.yo
+++ b/Doc/Zsh/redirect.yo
@@ -182,7 +182,8 @@ indent(... tt({myfd}>&1))
 This opens a new file descriptor that is a duplicate of file descriptor
 1 and sets the parameter tt(myfd) to the number of the file descriptor,
 which will be at least 10.  The new file descriptor can be written to using
-the syntax tt(>&$myfd).
+the syntax tt(>&$myfd).  The file descriptor remains open in subshells
+and forked external executables.
 
 The syntax tt({)var(varid)tt(}>&-), for example tt({myfd}>&-), may be used
 to close a file descriptor opened in this fashion.  Note that the
diff --git a/Src/Modules/system.c b/Src/Modules/system.c
index 7a4f4ee13..50de59cf9 100644
--- a/Src/Modules/system.c
+++ b/Src/Modules/system.c
@@ -316,7 +316,7 @@ bin_sysopen(char *nam, char **args, Options ops, UNUSED(int func))
     int o, fd, moved_fd, explicit = -1;
     mode_t perms = 0666;
 #if defined(FD_CLOEXEC) && !defined(O_CLOEXEC)
-    int fdflags;
+    int fdflags = 0;
 #endif
 
     if (!OPT_ISSET(ops, 'u')) {
@@ -396,8 +396,8 @@ bin_sysopen(char *nam, char **args, Options ops, UNUSED(int func))
 #endif /* O_CLOEXEC */
 	fcntl(moved_fd, F_SETFD, FD_CLOEXEC);
 #endif /* FD_CLOEXEC */
+    fdtable[moved_fd] = FDT_EXTERNAL;
     if (explicit == -1) {
-	fdtable[moved_fd] = FDT_EXTERNAL;
 	setiparam(fdvar, moved_fd);
 	/* if setting the variable failed, close moved_fd to avoid leak */
 	if (errflag)
diff --git a/Src/exec.c b/Src/exec.c
index 60ab0acf8..2acb2c0bc 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -4407,8 +4407,10 @@ closem(int how, int all)
 	    /*
 	     * Process substitution needs to be visible to user;
 	     * fd's are explicitly cleaned up by filelist handling.
+	     * External FDs are managed directly by the user.
 	     */
-	    (all || fdtable[i] != FDT_PROC_SUBST) &&
+	    (all || (fdtable[i] != FDT_PROC_SUBST &&
+		     fdtable[i] != FDT_EXTERNAL)) &&
 	    (how == FDT_UNUSED || (fdtable[i] & FDT_TYPE_MASK) == how)) {
 	    if (i == SHTTY)
 		SHTTY = -1;
@@ -4823,6 +4825,7 @@ getoutputfile(char *cmd, char **eptr)
     }
 
     /* pid == 0 */
+    closem(FDT_UNUSED, 0);
     redup(fd, 1);
     entersubsh(ESUB_PGRP|ESUB_NOMONITOR, NULL);
     cmdpush(CS_CMDSUBST);



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: <(...), >(...) and fds above 9
  2019-07-02  8:59             ` Peter Stephenson
@ 2019-07-02 12:20               ` Stephane Chazelas
  2019-07-02 13:12                 ` Peter Stephenson
  2019-07-02 16:43                 ` Sebastian Gniazdowski
  0 siblings, 2 replies; 16+ messages in thread
From: Stephane Chazelas @ 2019-07-02 12:20 UTC (permalink / raw)
  To: Zsh hackers list

2019-07-02 09:59:36 +0100, Peter Stephenson:
[...]
> This fixes this up --- I think for file descriptors known to and hence
> managed by the user the behaviour of silently closing is probably
> uncontroversially unhelpful.
[...]

Thanks.

Note that ksh93 marks the fds above 2 that are open with
exec (exec 7< file or exec {fd}< file) with close-on-exec
(independently of whether the exec is done in background or
not)

Neither ksh93 nor bash close the fds opened with
{ cat <(ls -l /proc/self/fd; } {fd}< file
(that syntax doesn't work in zsh as already reported).

Note another related problem that had also been reported earlier
shared with bash but not ksh93: the fd is not closed after the
command returned when the redirection is done on a builtin,
function or compound command.

$ zsh -c 'eval "echo \$fd" {fd}< a; ls -l /proc/self/fd/$fd'
12
lr-x------ 1 chazelas chazelas 64 Jul  2 13:14 /proc/self/fd/12 -> /home/chazelas/a

-- 
Stephane

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: <(...), >(...) and fds above 9
  2019-07-02 12:20               ` Stephane Chazelas
@ 2019-07-02 13:12                 ` Peter Stephenson
  2019-07-02 15:19                   ` Stephane Chazelas
  2019-07-02 16:43                 ` Sebastian Gniazdowski
  1 sibling, 1 reply; 16+ messages in thread
From: Peter Stephenson @ 2019-07-02 13:12 UTC (permalink / raw)
  To: zsh-workers

On Tue, 2019-07-02 at 13:20 +0100, Stephane Chazelas wrote:
> 2019-07-02 09:59:36 +0100, Peter Stephenson:
> Note that ksh93 marks the fds above 2 that are open with
> exec (exec 7< file or exec {fd}< file) with close-on-exec
> (independently of whether the exec is done in background or
> not)

This will do that for file descriptors managed with varid or builtins from
modules, though it doesn't change the behaviour for file descriptors
managed directly by number.  I'm wondering if that's too traditional to
be worth any pain of changing --- {fd} is a much more manageable
interface.

pws

diff --git a/Doc/Zsh/mod_socket.yo b/Doc/Zsh/mod_socket.yo
index 78d9254e8..5b4355158 100644
--- a/Doc/Zsh/mod_socket.yo
+++ b/Doc/Zsh/mod_socket.yo
@@ -57,8 +57,8 @@ tt(zsocket -a) will accept an incoming connection
 to the socket associated with var(listenfd).
 The shell parameter tt(REPLY) will
 be set to the file descriptor associated with
-the inbound connection.  The file descriptor remains open in subshells
-and forked external executables.
+the inbound connection.  The file descriptor remains open in subshells,
+but is marked as "close on execute" on systems where this is possible.
 
 If tt(-d) is specified, its argument
 will be taken as the target file descriptor for the
diff --git a/Doc/Zsh/mod_tcp.yo b/Doc/Zsh/mod_tcp.yo
index bf17ec82a..441504a4c 100644
--- a/Doc/Zsh/mod_tcp.yo
+++ b/Doc/Zsh/mod_tcp.yo
@@ -25,6 +25,8 @@ item(File descriptor)(
 The file descriptor in use for the connection.  For normal inbound (tt(I))
 and outbound (tt(O)) connections this may be read and written by the usual
 shell mechanisms.  However, it should only be close with `tt(ztcp -c)'.
+All file descriptors opened by the tt(zsh/net/tcp) module
+are marked as "close on execute" on systems where this is possible.
 )
 item(Connection type)(
 A letter indicating how the session was created:
diff --git a/Doc/Zsh/redirect.yo b/Doc/Zsh/redirect.yo
index 13496d8d3..9b6b78efa 100644
--- a/Doc/Zsh/redirect.yo
+++ b/Doc/Zsh/redirect.yo
@@ -182,8 +182,8 @@ indent(... tt({myfd}>&1))
 This opens a new file descriptor that is a duplicate of file descriptor
 1 and sets the parameter tt(myfd) to the number of the file descriptor,
 which will be at least 10.  The new file descriptor can be written to using
-the syntax tt(>&$myfd).  The file descriptor remains open in subshells
-and forked external executables.
+the syntax tt(>&$myfd).  The file descriptor remains open in subshells,
+but is marked as "close on execute" on systems where this is possible.
 
 The syntax tt({)var(varid)tt(}>&-), for example tt({myfd}>&-), may be used
 to close a file descriptor opened in this fashion.  Note that the
diff --git a/Src/Modules/socket.c b/Src/Modules/socket.c
index c65b7dfce..6083ee85b 100644
--- a/Src/Modules/socket.c
+++ b/Src/Modules/socket.c
@@ -129,8 +129,8 @@ bin_zsocket(char *nam, char **args, Options ops, UNUSED(int func))
 	    return 1;
 	}
 
-	/* allow to be closed explicitly */
-	fdtable[sfd] = FDT_EXTERNAL;
+	/* allow to be closed explicitly, close on exec */
+	markfd(sfd, FDT_EXTERNAL, true);
 
 	setiparam_no_convert("REPLY", (zlong)sfd);
 
@@ -214,7 +214,7 @@ bin_zsocket(char *nam, char **args, Options ops, UNUSED(int func))
 		zclose(rfd);
 		return 1;
 	    }
-	    fdtable[sfd] = FDT_EXTERNAL;
+	    markfd(sfd, FDT_EXTERNAL, true);
 	}
 	else {
 	    sfd = rfd;
@@ -258,7 +258,7 @@ bin_zsocket(char *nam, char **args, Options ops, UNUSED(int func))
 		    return 1;
 		}
 		sfd = targetfd;
-		fdtable[sfd] = FDT_EXTERNAL;
+		markfd(sfd, FDT_EXTERNAL, true);
 	    }
 
 	    setiparam_no_convert("REPLY", (zlong)sfd);
diff --git a/Src/Modules/system.c b/Src/Modules/system.c
index 50de59cf9..d3b47429a 100644
--- a/Src/Modules/system.c
+++ b/Src/Modules/system.c
@@ -315,9 +315,7 @@ bin_sysopen(char *nam, char **args, Options ops, UNUSED(int func))
     char *opt, *ptr, *nextopt, *fdvar;
     int o, fd, moved_fd, explicit = -1;
     mode_t perms = 0666;
-#if defined(FD_CLOEXEC) && !defined(O_CLOEXEC)
-    int fdflags = 0;
-#endif
+    bool close_on_exec = 0;
 
     if (!OPT_ISSET(ops, 'u')) {
 	zwarnnam(nam, "file descriptor not specified");
@@ -349,7 +347,7 @@ bin_sysopen(char *nam, char **args, Options ops, UNUSED(int func))
 	    }
 #if defined(FD_CLOEXEC) && !defined(O_CLOEXEC)
 	    if (!openopts[o].oflag)
-		fdflags = FD_CLOEXEC;
+		close_on_exec = 1;
 #endif
 	    flags |= openopts[o].oflag;
 	    opt = nextopt;
@@ -382,8 +380,7 @@ bin_sysopen(char *nam, char **args, Options ops, UNUSED(int func))
 	return 1;
     }
 
-#ifdef FD_CLOEXEC
-#ifdef O_CLOEXEC
+#if defined(FD_CLOEXEC) && defined(O_CLOEXEC)
     /*
      * the O_CLOEXEC is a flag attached to the *file descriptor*, not the
      * *open file description* so it doesn't survive a dup(). If that flag was
@@ -391,12 +388,9 @@ bin_sysopen(char *nam, char **args, Options ops, UNUSED(int func))
      * even if the original one was open with O_CLOEXEC
      */
     if ((flags & O_CLOEXEC) && fd != moved_fd)
-#else
-    if (fdflags)
-#endif /* O_CLOEXEC */
-	fcntl(moved_fd, F_SETFD, FD_CLOEXEC);
-#endif /* FD_CLOEXEC */
-    fdtable[moved_fd] = FDT_EXTERNAL;
+	close_on_exec = 1;
+#endif /* defined(FD_CLOEXEC) && defined(O_CLOEXEC) */
+    markfd(moved_fd, FDT_EXTERNAL, close_on_exec);
     if (explicit == -1) {
 	setiparam(fdvar, moved_fd);
 	/* if setting the variable failed, close moved_fd to avoid leak */
diff --git a/Src/exec.c b/Src/exec.c
index 2acb2c0bc..3ab538908 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -2300,7 +2300,8 @@ addfd(int forked, int *save, struct multio **mfds, int fd1, int fd2, int rflag,
 	    zerr("cannot moved fd %d: %e", fd2, errno);
 	    return;
 	} else {
-	    fdtable[fd1] = FDT_EXTERNAL;
+	    /* Mark for close-on-exec, consistent with ksh93 */
+	    markfd(fd1, FDT_EXTERNAL, true);
 	    setiparam(varid, (zlong)fd1);
 	    /*
 	     * If setting the parameter failed, close the fd else
diff --git a/Src/utils.c b/Src/utils.c
index 46cf7bcf6..f4440fc70 100644
--- a/Src/utils.c
+++ b/Src/utils.c
@@ -2078,8 +2078,30 @@ redup(int x, int y)
     return ret;
 }
 
+
 /*
- * Add an fd opened ithin a module.
+ * Mark an fd as a particular type fdt: see addmodulefd() for details.
+ *
+ * Handle any additional business, e.g. marking fd's for close on exec
+ * which is normal behaviour for externally visible fds unless the
+ * caller wishes for special arrangements.
+ */
+
+/**/
+mod_export void
+markfd(int fd, int fdt, bool close_on_exec)
+{
+    fdtable[fd] = fdt;
+#ifdef FD_CLOEXEC
+    if (close_on_exec)
+	fcntl(fd, F_SETFD, FD_CLOEXEC);
+#else
+    (void)close_on_exec;
+#endif
+}
+
+/*
+ * Add an fd opened within a module.
  *
  * fdt is the type of the fd; see the FDT_ definitions in zsh.h.
  * The most likely falures are:
@@ -2096,6 +2118,9 @@ redup(int x, int y)
  * exec's or performs an exec with a fork optimised out.
  *
  * Safe if fd is -1 to indicate failure.
+ *
+ * It is assumed file descriptors from modules should be set to
+ * close-on-exec.
  */
 /**/
 mod_export void
@@ -2103,7 +2128,7 @@ addmodulefd(int fd, int fdt)
 {
     if (fd >= 0) {
 	check_fd_table(fd);
-	fdtable[fd] = fdt;
+	markfd(fd, fdt, true);
     }
 }
 


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: <(...), >(...) and fds above 9
  2019-07-02 13:12                 ` Peter Stephenson
@ 2019-07-02 15:19                   ` Stephane Chazelas
  2019-07-02 15:21                     ` Peter Stephenson
  0 siblings, 1 reply; 16+ messages in thread
From: Stephane Chazelas @ 2019-07-02 15:19 UTC (permalink / raw)
  To: Peter Stephenson; +Cc: zsh-workers

2019-07-02 13:12:35 +0000, Peter Stephenson:
> On Tue, 2019-07-02 at 13:20 +0100, Stephane Chazelas wrote:
> > 2019-07-02 09:59:36 +0100, Peter Stephenson:
> > Note that ksh93 marks the fds above 2 that are open with
> > exec (exec 7< file or exec {fd}< file) with close-on-exec
> > (independently of whether the exec is done in background or
> > not)
> 
> This will do that for file descriptors managed with varid or builtins from
> modules, though it doesn't change the behaviour for file descriptors
> managed directly by number.  I'm wondering if that's too traditional to
> be worth any pain of changing --- {fd} is a much more manageable
> interface.
[...]

Note that I was not suggesting that zsh should do the same. That
close-on-exec on ksh93 adds more confusion than it helps. No
other shell does it AFAIK. That means that you need to work
around it like:

exec 7<> some-file
...
cmd /dev/fd/7 7>&7

to work around it.

Or make sure to use

{
  ...
  cmd /dev/fd/7
  ...
} 7<> some-file

And not "exec".

-- 
Stephane

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: <(...), >(...) and fds above 9
  2019-07-02 15:19                   ` Stephane Chazelas
@ 2019-07-02 15:21                     ` Peter Stephenson
  0 siblings, 0 replies; 16+ messages in thread
From: Peter Stephenson @ 2019-07-02 15:21 UTC (permalink / raw)
  To: zsh-workers

On Tue, 2019-07-02 at 16:19 +0100, Stephane Chazelas wrote:
> 2019-07-02 13:12:35 +0000, Peter Stephenson:
> > 
> > On Tue, 2019-07-02 at 13:20 +0100, Stephane Chazelas wrote:
> > > 
> > > 2019-07-02 09:59:36 +0100, Peter Stephenson:
> > > Note that ksh93 marks the fds above 2 that are open with
> > > exec (exec 7< file or exec {fd}< file) with close-on-exec
> > > (independently of whether the exec is done in background or
> > > not)
> > This will do that for file descriptors managed with varid or builtins from
> > modules, though it doesn't change the behaviour for file descriptors
> > managed directly by number.  I'm wondering if that's too traditional to
> > be worth any pain of changing --- {fd} is a much more manageable
> > interface.
> [...]
> 
> Note that I was not suggesting that zsh should do the same. That
> close-on-exec on ksh93 adds more confusion than it helps. No
> other shell does it AFAIK. That means that you need to work
> around it like:

That's fine, no need to commit it.

pws


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: <(...), >(...) and fds above 9
  2019-07-02 12:20               ` Stephane Chazelas
  2019-07-02 13:12                 ` Peter Stephenson
@ 2019-07-02 16:43                 ` Sebastian Gniazdowski
  2019-07-02 18:08                   ` Stephane Chazelas
  1 sibling, 1 reply; 16+ messages in thread
From: Sebastian Gniazdowski @ 2019-07-02 16:43 UTC (permalink / raw)
  To: Zsh hackers list

On Tue, 2 Jul 2019 at 14:21, Stephane Chazelas
<stephane.chazelas@gmail.com> wrote:
> Neither ksh93 nor bash close the fds opened with
> { cat <(ls -l /proc/self/fd; } {fd}< file
> (that syntax doesn't work in zsh as already reported).

Can I ask you what do you mean by closing the descriptor? I recently
often do exec {FD}< <(some code); zle -F $FD handler and it is
working, which to me appears as the FD not being closed?

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: <(...), >(...) and fds above 9
  2019-07-02 16:43                 ` Sebastian Gniazdowski
@ 2019-07-02 18:08                   ` Stephane Chazelas
  0 siblings, 0 replies; 16+ messages in thread
From: Stephane Chazelas @ 2019-07-02 18:08 UTC (permalink / raw)
  To: zsh-workers

2019-07-02 18:43:16 +0200, Sebastian Gniazdowski:
> On Tue, 2 Jul 2019 at 14:21, Stephane Chazelas
> <stephane.chazelas@gmail.com> wrote:
> > Neither ksh93 nor bash close the fds opened with
> > { cat <(ls -l /proc/self/fd; } {fd}< file
> > (that syntax doesn't work in zsh as already reported).
> 
> Can I ask you what do you mean by closing the descriptor? I recently
> often do exec {FD}< <(some code); zle -F $FD handler and it is
> working, which to me appears as the FD not being closed?

The problem is not about fds redirected to/from process
substitutions but fds (opened earlier) used *inside* process
substitution.

exec {fd}< some-file
cat <(cat <&$fd)

-- 
Stephane



^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2019-07-02 18:09 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <CGME20190701100058epcas2p25e5f8dbd14d048fe2be1d831f3cf60ab@epcas2p2.samsung.com>
2019-07-01 10:00 ` <(...), >(...) and fds above 9 Stephane Chazelas
2019-07-01 10:08   ` Peter Stephenson
2019-07-01 14:39     ` Bart Schaefer
2019-07-01 15:28       ` Peter Stephenson
2019-07-01 16:22         ` Stephane Chazelas
2019-07-01 16:52           ` Peter Stephenson
2019-07-01 17:00             ` Peter Stephenson
2019-07-01 20:00             ` Stephane Chazelas
2019-07-01 20:06               ` Bart Schaefer
2019-07-02  8:59             ` Peter Stephenson
2019-07-02 12:20               ` Stephane Chazelas
2019-07-02 13:12                 ` Peter Stephenson
2019-07-02 15:19                   ` Stephane Chazelas
2019-07-02 15:21                     ` Peter Stephenson
2019-07-02 16:43                 ` Sebastian Gniazdowski
2019-07-02 18:08                   ` Stephane Chazelas

Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/zsh/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).