Re: Any comment on file descriptor behavior in functions?

[Peter Stephenson <pws@xxxxxxx>]

Bart Schaefer wrote:
> On Mar 22,  2:04pm, Peter Stephenson wrote:
> } Subject: Re: Any comment on file descriptor behavior in functions?
> }
> } > > It could be argued that descriptors opened by process substitution
> } > > should remain open across external command execution within functions
> } 
> } Is this it?  Does it have other effects?
> 
> That's not it, or rather it's too much of it.  That also leaves open the
> xtrace output descriptor, etc.  The fix needs to be more specific.

(I can't see any evidence for ", etc.", but I presume you just didn't
perform an exhaustive search.)

Maybe making the problem less obscure makes the solution more likely to
work.  The following doesn't look obviously broken to me, although I'm
not completely certain what was going on with the relative values
assigned to the xtrace output and the process substitutions.

As far as I can see, when entering a function the values for process
subsitutions would be incremented so that both types would be closed for
an external program, but on a nested function they'd both be incremented
further so they wouldn't.  That does seem to be what was happening:

% fn() { fn2 $1; }
% fn2() { cat $1; }
% fn <(echo 1)
1
% fn2 <(echo 1)
cat: /proc/self/fd/12: No such file or directory

I can't believe that was intended (though it gives a nasty workaround
for older shells).

Both should now work like the "fn" example, and the xtrace behaviour
should have become disengaged from the process substitution behaviour.
In other words, I'm assuming that anything that made an xtrace entry in
fdtable other than 3 wasn't doing so usefully.  The "#ifdef
PATH_DEV_FD"s suggest as much.

I will be less communicative until Thursday next week.

Index: Src/exec.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/exec.c,v
retrieving revision 1.84
diff -u -r1.84 exec.c
--- Src/exec.c	6 Feb 2005 20:36:42 -0000	1.84
+++ Src/exec.c	22 Mar 2005 18:15:25 -0000
@@ -77,7 +77,7 @@
  * by zclose.                                                      */
 
 /**/
-char *fdtable;
+unsigned char *fdtable;
 
 /* The allocated size of fdtable */
 
@@ -496,7 +496,11 @@
     }
 
     argv = makecline(args);
-    closem(3);
+    /*
+     * Note that we don't close fd's attached to process substitution
+     * here, which should be visible to external processes.
+     */
+    closem(FDT_XTRACE);
     child_unblock();
     if ((int) strlen(arg0) >= PATH_MAX) {
 	zerr("command too long: %s", arg0, 0);
@@ -1053,7 +1057,7 @@
 	mpipe(opipe);
 	coprocin = ipipe[0];
 	coprocout = opipe[1];
-	fdtable[coprocin] = fdtable[coprocout] = 0;
+	fdtable[coprocin] = fdtable[coprocout] = FDT_UNUSED;
     }
     /* This used to set list_pipe_pid=0 unconditionally, but in things
      * like `ls|if true; then sleep 20; cat; fi' where the sleep was
@@ -1576,7 +1580,7 @@
 	    mfds[fd1]->fds[mfds[fd1]->ct++] = movefd(fd2);
 	}
     }
-    if (subsh_close >= 0 && !fdtable[subsh_close])
+    if (subsh_close >= 0 && fdtable[subsh_close] == FDT_UNUSED)
 	subsh_close = -1;
 }
 
@@ -2132,7 +2136,7 @@
 	    read(synch[0], &dummy, 1);
 	    close(synch[0]);
 #ifdef PATH_DEV_FD
-	    closem(2);
+	    closem(FDT_PROC_SUBST);
 #endif
 	    if (how & Z_ASYNC) {
 		lastpid = (zlong) pid;
@@ -2196,7 +2200,7 @@
 	if (!(xtrerr = fdopen(movefd(dup(fileno(stderr))), "w")))
 	    xtrerr = stderr;
 	else
-	    fdtable[fileno(xtrerr)] = 3;
+	    fdtable[fileno(xtrerr)] = FDT_XTRACE;
     }
 
     /* Add pipeline input/output to mnodes */
@@ -2286,7 +2290,7 @@
 		if (fn->fd2 < 10)
 		    closemn(mfds, fn->fd2);
 		if (fn->fd2 > 9 &&
-		    (fdtable[fn->fd2] ||
+		    (fdtable[fn->fd2] != FDT_UNUSED ||
 		     fn->fd2 == coprocin ||
 		     fn->fd2 == coprocout)) {
 		    fil = -1;
@@ -2414,7 +2418,7 @@
 		int i;
 
 		for (i = 10; i <= max_zsh_fd; i++)
-		    if (fdtable[i] > 1)
+		    if (fdtable[i] >= FDT_PROC_SUBST)
 			fdtable[i]++;
 #endif
 		if (subsh_close >= 0)
@@ -2424,17 +2428,17 @@
 		execshfunc((Shfunc) hn, args);
 #ifdef PATH_DEV_FD
 		for (i = 10; i <= max_zsh_fd; i++)
-		    if (fdtable[i] > 1)
-			if (--(fdtable[i]) <= 2)
+		    if (fdtable[i] >= FDT_PROC_SUBST)
+			if (--(fdtable[i]) <= FDT_PROC_SUBST)
 			    zclose(i);
 #endif
 	    } else {
 		/* It's a builtin */
 		if (forked)
-		    closem(1);
+		    closem(FDT_INTERNAL);
 		lastval = execbuiltin(args, (Builtin) hn);
 #ifdef PATH_DEV_FD
-		closem(2);
+		closem(FDT_PROC_SUBST);
 #endif
 		fflush(stdout);
 		if (save[1] == -2) {
@@ -2478,7 +2482,7 @@
 		    if (errflag)
 			_exit(1);
 		}
-		closem(1);
+		closem(FDT_INTERNAL);
 		if (coprocin)
 		    zclose(coprocin);
 		if (coprocout)
@@ -2711,7 +2715,12 @@
     forklevel = locallevel;
 }
 
-/* close internal shell fds */
+/*
+ * Close internal shell fds.
+ *
+ * Close any that are marked as used if "how" is FDT_UNUSED, else
+ * close any with the value "how".
+ */
 
 /**/
 mod_export void
@@ -2720,7 +2729,8 @@
     int i;
 
     for (i = 10; i <= max_zsh_fd; i++)
-	if (fdtable[i] && (!how || fdtable[i] == how))
+	if (fdtable[i] != FDT_UNUSED &&
+	    (how == FDT_UNUSED || fdtable[i] == how))
 	    zclose(i);
 }
 
@@ -2865,7 +2875,7 @@
 
 	zclose(pipes[1]);
 	retval = readoutput(pipes[0], qt);
-	fdtable[pipes[0]] = 0;
+	fdtable[pipes[0]] = FDT_UNUSED;
 	waitforpid(pid);		/* unblocks */
 	lastval = cmdoutval;
 	return retval;
@@ -3069,7 +3079,7 @@
 	    addproc(pid, NULL, 1, &bgtime);
 	return pnam;
     }
-    closem(0);
+    closem(FDT_UNUSED);
     fd = open(pnam, out ? O_WRONLY | O_NOCTTY : O_RDONLY | O_NOCTTY);
     if (fd == -1) {
 	zerr("can't open %s: %e", pnam, errno);
@@ -3094,7 +3104,7 @@
 	    zclose(pipes[!out]);
 	    return NULL;
 	}
-	fdtable[pipes[!out]] = 2;
+	fdtable[pipes[!out]] = FDT_PROC_SUBST;
 	if (!out)
 	{
 	    addproc(pid, NULL, 1, &bgtime);
@@ -3103,7 +3113,7 @@
     }
     entersubsh(Z_ASYNC, 1, 0, 0);
     redup(pipes[out], out);
-    closem(0);   /* this closes pipes[!out] as well */
+    closem(FDT_UNUSED);   /* this closes pipes[!out] as well */
 #endif /* PATH_DEV_FD */
 
     cmdpush(CS_CMDSUBST);
@@ -3147,7 +3157,7 @@
     }
     entersubsh(Z_ASYNC, 1, 0, 0);
     redup(pipes[out], out);
-    closem(0);	/* this closes pipes[!out] as well */
+    closem(FDT_UNUSED);	/* this closes pipes[!out] as well */
     cmdpush(CS_CMDSUBST);
     execode(prog, 0, 1);
     cmdpop();
Index: Src/init.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/init.c,v
retrieving revision 1.48
diff -u -r1.48 init.c
--- Src/init.c	7 Feb 2005 15:53:23 -0000	1.48
+++ Src/init.c	22 Mar 2005 18:15:25 -0000
@@ -1048,7 +1048,7 @@
 	freeeprog(prog);
     else {
 	fclose(bshin);
-	fdtable[SHIN] = 0;
+	fdtable[SHIN] = FDT_UNUSED;
 	SHIN = fd;		     /* the shell input fd                   */
 	bshin = obshin;		     /* file handle for buffered shell input */
     }
@@ -1252,7 +1252,7 @@
     } while (zsh_name);
 
     fdtable_size = zopenmax();
-    fdtable = zshcalloc(fdtable_size);
+    fdtable = zshcalloc(fdtable_size*sizeof(*fdtable));
 
     createoptiontable();
     emulate(zsh_name, 1);   /* initialises most options */
Index: Src/utils.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/utils.c,v
retrieving revision 1.77
diff -u -r1.77 utils.c
--- Src/utils.c	11 Mar 2005 15:22:36 -0000	1.77
+++ Src/utils.c	22 Mar 2005 18:15:25 -0000
@@ -1078,10 +1078,11 @@
     if(fd != -1) {
 	if (fd > max_zsh_fd) {
 	    while (fd >= fdtable_size)
-		fdtable = zrealloc(fdtable, (fdtable_size *= 2));
+		fdtable = zrealloc(fdtable,
+				   (fdtable_size *= 2)*sizeof(*fdtable));
 	    max_zsh_fd = fd;
 	}
-	fdtable[fd] = 1;
+	fdtable[fd] = FDT_INTERNAL;
     }
     return fd;
 }
@@ -1096,7 +1097,7 @@
 	zclose(y);
     else if (x != y) {
 	while (y >= fdtable_size)
-	    fdtable = zrealloc(fdtable, (fdtable_size *= 2));
+	    fdtable = zrealloc(fdtable, (fdtable_size *= 2)*sizeof(*fdtable));
 	dup2(x, y);
 	if ((fdtable[y] = fdtable[x]) && y > max_zsh_fd)
 	    max_zsh_fd = y;
@@ -1111,8 +1112,8 @@
 zclose(int fd)
 {
     if (fd >= 0) {
-	fdtable[fd] = 0;
-	while (max_zsh_fd > 0 && !fdtable[max_zsh_fd])
+	fdtable[fd] = FDT_UNUSED;
+	while (max_zsh_fd > 0 && fdtable[max_zsh_fd] == FDT_UNUSED)
 	    max_zsh_fd--;
 	if (fd == coprocin)
 	    coprocin = -1;
Index: Src/zsh.h
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/zsh.h,v
retrieving revision 1.71
diff -u -r1.71 zsh.h
--- Src/zsh.h	18 Mar 2005 22:40:23 -0000	1.71
+++ Src/zsh.h	22 Mar 2005 18:15:26 -0000
@@ -254,6 +254,32 @@
 #define IS_READFD(X)          (((X)>=REDIR_READWRITE && (X)<=REDIR_MERGEIN) || (X)==REDIR_INPIPE)
 #define IS_REDIROP(X)         ((X)>=OUTANG && (X)<=TRINANG)
 
+/*
+ * Values for the fdtable array.  They say under what circumstances
+ * the fd will be close.  The fdtable is an unsigned char, so these are
+ * #define's rather than an enum.
+ */
+/* Entry not used. */
+#define FDT_UNUSED		0
+/*
+ * Entry used internally by the shell, should not be visible to other
+ * processes.
+ */
+#define FDT_INTERNAL		1
+/*
+ * Entry used by output from the XTRACE option.
+ */
+#define FDT_XTRACE		2
+#ifdef PATH_DEV_FD
+/*
+ * Entry used by a process substition.
+ * The value will be incremented on entering a function and
+ * decremented on exit; we don't close entries greater than
+ * FDT_PROC_SUBST except when closing everything.
+ */
+#define FDT_PROC_SUBST		3
+#endif
+
 /* Flags for input stack */
 #define INP_FREE      (1<<0)	/* current buffer can be free'd            */
 #define INP_ALIAS     (1<<1)	/* expanding alias or history              */

-- 
Peter Stephenson <pws@xxxxxxx>                  Software Engineer
CSR PLC, Churchill House, Cambridge Business Park, Cowley Road
Cambridge, CB4 0WZ, UK                          Tel: +44 (0)1223 692070


**********************************************************************
This email and any files transmitted with it are confidential and
intended solely for the use of the individual or entity to whom they
are addressed. If you have received this email in error please notify
the system manager.

**********************************************************************