PATCH: allocating a new file descriptor

PATCH: allocating a new file descriptor

[Peter Stephenson]

David Korn suggested it would be a good idea to have a syntax for the
shell to allocate a new file descriptor, rather than relying on picking
a number which (i) has to be under 10 (ii) may be in use by something
else.  Oliver suggested a syntax which looks fairly clean.  This
implements it in zsh.  (It sounds like David already has something
working in ksh, too.)

It works basically like this:

  integer myfd

  # Allocate a new fd, storing the value in $myfd.
  # The fd will be at least 10.
  exec {myfd}>~/tmp/mylogfile

  # This syntax already worked.
  print This is a log message. >&$myfd

  # Close the allocated fd.
  exec {myfd}>&-

This gives us much better encapsulation for manipulating file
descriptors, something along the lines of files or file handles in other
languages.

The shell checks that the braces only contain valid characters for a
parameter ID, so the only likely clash with this syntax is for avid
users of the BRACE_CCL option where {myfd}>~/tmp/logfile would perform a
normal redirection of fd 1 and add the words d, f, m, y to the
command arguments.  I don't think this widely used.

As it relies on braces it is turned off if the option IGNORE_BRACES is
set.

Index: Doc/Zsh/redirect.yo
===================================================================
RCS file: /cvsroot/zsh/zsh/Doc/Zsh/redirect.yo,v
retrieving revision 1.7
diff -u -r1.7 redirect.yo
--- Doc/Zsh/redirect.yo	28 Jun 2004 15:38:13 -0000	1.7
+++ Doc/Zsh/redirect.yo	12 Apr 2005 11:28:22 -0000
@@ -149,6 +149,31 @@
 with the terminal (assuming file descriptor 1 had been)
 and then file descriptor 1 would be associated with file var(fname).
 
+If instead of a digit one of the operators above is preceded by
+a valid identifier enclosed in braces, the shell will open a new
+file descriptor that is guaranteed to be at least 10 and set the
+parameter named by the identifier to the file descriptor opened.
+No whitespace is allowed between the closing brace and the redirection
+character.  The option tt(IGNORE_BRACES) must not be set.
+For example:
+
+indent(... {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({)var(varid)tt(}>&-), for example tt({myfd}>&-), may be used
+to close a file descriptor opened in this fashion.  Note that the
+parameter given by var(varid) must previously be set to a file descriptor
+in this case.
+
+It is an error to open or close a file descriptor in this fashion when the
+parameter is readonly.  However, it is not an error to read or write a file
+descriptor using tt(<&$)var(param) or tt(>&$)var(param) if var(param) is
+readonly.
+
 The `tt(|&)' command separator described in
 ifzman(em(Simple Commands & Pipelines) in zmanref(zshmisc))\
 ifnzman(noderef(Simple Commands & Pipelines))
Index: Src/exec.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/exec.c,v
retrieving revision 1.86
diff -u -r1.86 exec.c
--- Src/exec.c	4 Apr 2005 09:35:43 -0000	1.86
+++ Src/exec.c	12 Apr 2005 11:28:22 -0000
@@ -1293,7 +1293,8 @@
 	wordcode code;
 
 	state->pc++;
-	for (pc = state->pc; wc_code(code = *pc) == WC_REDIR; pc += 3);
+	for (pc = state->pc; wc_code(code = *pc) == WC_REDIR;
+	     pc += WC_REDIR_WORDS(code));
 
 	mpipe(pipes);
 
@@ -1532,32 +1533,52 @@
 	}
 }
 
-/* A multio is a list of fds associated with a certain fd.       *
- * Thus if you do "foo >bar >ble", the multio for fd 1 will have *
- * two fds, the result of open("bar",...), and the result of     *
- * open("ble",....).                                             */
-
-/* Add a fd to an multio.  fd1 must be < 10, and may be in any state. *
- * fd2 must be open, and is `consumed' by this function.  Note that   *
- * fd1 == fd2 is possible, and indicates that fd1 was really closed.  *
- * We effectively do `fd2 = movefd(fd2)' at the beginning of this     *
- * function, but in most cases we can avoid an extra dup by delaying  *
- * the movefd: we only >need< to move it if we're actually doing a    *
- * multiple redirection.                                              */
+/*
+ * A multio is a list of fds associated with a certain fd.
+ * Thus if you do "foo >bar >ble", the multio for fd 1 will have
+ * two fds, the result of open("bar",...), and the result of
+ * open("ble",....).
+ */
+
+/*
+ * Add a fd to an multio.  fd1 must be < 10, and may be in any state.
+ * fd2 must be open, and is `consumed' by this function.  Note that
+ * fd1 == fd2 is possible, and indicates that fd1 was really closed.
+ * We effectively do `fd2 = movefd(fd2)' at the beginning of this
+ * function, but in most cases we can avoid an extra dup by delaying
+ * the movefd: we only >need< to move it if we're actually doing a
+ * multiple redirection.
+ *
+ * If varid is not NULL, we open an fd above 10 and set the parameter
+ * named varid to that value.  fd1 is not used.
+ */
 
 /**/
 static void
-addfd(int forked, int *save, struct multio **mfds, int fd1, int fd2, int rflag)
+addfd(int forked, int *save, struct multio **mfds, int fd1, int fd2, int rflag,
+      char *varid)
 {
     int pipes[2];
 
-    if (!mfds[fd1] || unset(MULTIOS)) {
+    if (varid) {
+	/* fd will be over 10, don't touch mfds */
+	fd1 = movefd(fd2);
+	fdtable[fd1] = FDT_EXTERNAL;
+	setiparam(varid, (zlong)fd1);
+	/*
+	 * If setting the parameter failed, close the fd else
+	 * it will leak.
+	 */
+	if (errflag)
+	    zclose(fd1);
+    } else if (!mfds[fd1] || unset(MULTIOS)) {
 	if(!mfds[fd1]) {		/* starting a new multio */
 	    mfds[fd1] = (struct multio *) zhalloc(sizeof(struct multio));
 	    if (!forked && save[fd1] == -2)
 		save[fd1] = (fd1 == fd2) ? -1 : movefd(fd1);
 	}
-	redup(fd2, fd1);
+	if (!varid)
+	    redup(fd2, fd1);
 	mfds[fd1]->ct = 1;
 	mfds[fd1]->fds[0] = fd1;
 	mfds[fd1]->rflag = rflag;
@@ -2207,9 +2228,9 @@
 
     /* Add pipeline input/output to mnodes */
     if (input)
-	addfd(forked, save, mfds, 0, input, 0);
+	addfd(forked, save, mfds, 0, input, 0, NULL);
     if (output)
-	addfd(forked, save, mfds, 1, output, 1);
+	addfd(forked, save, mfds, 1, output, 1, NULL);
 
     /* Do process substitutions */
     if (redir)
@@ -2226,14 +2247,14 @@
 		fixfds(save);
 		execerr();
 	    }
-	    addfd(forked, save, mfds, fn->fd1, fn->fd2, 0);
+	    addfd(forked, save, mfds, fn->fd1, fn->fd2, 0, fn->varid);
 	} else if (fn->type == REDIR_OUTPIPE) {
 	    if (fn->fd2 == -1) {
 		closemnodes(mfds);
 		fixfds(save);
 		execerr();
 	    }
-	    addfd(forked, save, mfds, fn->fd1, fn->fd2, 1);
+	    addfd(forked, save, mfds, fn->fd1, fn->fd2, 1, fn->varid);
 	} else {
 	    if (fn->type != REDIR_HERESTR && xpandredir(fn, redir))
 		continue;
@@ -2258,7 +2279,7 @@
 			zwarn("%e", NULL, errno);
 		    execerr();
 		}
-		addfd(forked, save, mfds, fn->fd1, fil, 0);
+		addfd(forked, save, mfds, fn->fd1, fil, 0, fn->varid);
 		break;
 	    case REDIR_READ:
 	    case REDIR_READWRITE:
@@ -2274,7 +2295,7 @@
 			zwarn("%e: %s", fn->name, errno);
 		    execerr();
 		}
-		addfd(forked, save, mfds, fn->fd1, fil, 0);
+		addfd(forked, save, mfds, fn->fd1, fil, 0, fn->varid);
 		/* If this is 'exec < file', read from stdin, *
 		 * not terminal, unless `file' is a terminal. */
 		if (nullexec == 1 && fn->fd1 == 0 &&
@@ -2282,9 +2303,32 @@
 		    init_io();
 		break;
 	    case REDIR_CLOSE:
+		if (fn->varid) {
+		    char *s = fn->varid;
+		    struct value vbuf;
+		    Value v;
+		    int bad = 0;
+
+		    if (!(v = getvalue(&vbuf, &s, 0))) {
+			bad = 1;
+		    } else if (v->pm->flags & PM_READONLY) {
+			bad = 2;
+		    } else {
+			fn->fd1 = (int)getintvalue(v);
+			bad = errflag;
+		    }
+		    if (bad) {
+			zwarn(bad == 2 ?
+			      "can't close file descriptor from readonly parameter" :
+			      "parameter %s does not contain a file descriptor",
+			      fn->varid, 0);
+			execerr();
+		    }
+		}
 		if (!forked && fn->fd1 < 10 && save[fn->fd1] == -2)
 		    save[fn->fd1] = movefd(fn->fd1);
-		closemn(mfds, fn->fd1);
+		if (fn->fd1 < 10)
+		    closemn(mfds, fn->fd1);
 		zclose(fn->fd1);
 		break;
 	    case REDIR_MERGEIN:
@@ -2292,7 +2336,8 @@
 		if (fn->fd2 < 10)
 		    closemn(mfds, fn->fd2);
 		if (fn->fd2 > 9 &&
-		    (fdtable[fn->fd2] != FDT_UNUSED ||
+		    ((fdtable[fn->fd2] != FDT_UNUSED &&
+		      fdtable[fn->fd2] != FDT_EXTERNAL) ||
 		     fn->fd2 == coprocin ||
 		     fn->fd2 == coprocout)) {
 		    fil = -1;
@@ -2313,7 +2358,8 @@
 		    zwarn("%s: %e", fn->fd2 == -2 ? "coprocess" : fdstr, errno);
 		    execerr();
 		}
-		addfd(forked, save, mfds, fn->fd1, fil, fn->type == REDIR_MERGEOUT);
+		addfd(forked, save, mfds, fn->fd1, fil,
+		      fn->type == REDIR_MERGEOUT, fn->varid);
 		break;
 	    default:
 		if (IS_APPEND_REDIR(fn->type))
@@ -2336,11 +2382,14 @@
 			zwarn("%e: %s", fn->name, errno);
 		    execerr();
 		}
-		addfd(forked, save, mfds, fn->fd1, fil, 1);
+		addfd(forked, save, mfds, fn->fd1, fil, 1, fn->varid);
 		if(IS_ERROR_REDIR(fn->type))
-		    addfd(forked, save, mfds, 2, dfil, 1);
+		    addfd(forked, save, mfds, 2, dfil, 1, NULL);
 		break;
 	    }
+	    if (errflag) {
+		execerr();
+	    }
 	}
     }
 
@@ -2845,6 +2894,7 @@
 	WC_SUBLIST_TYPE(pc[1]) == WC_SUBLIST_END &&
 	wc_code(pc[2]) == WC_PIPE && WC_PIPE_TYPE(pc[2]) == WC_PIPE_END &&
 	wc_code(pc[3]) == WC_REDIR && WC_REDIR_TYPE(pc[3]) == REDIR_READ && 
+	!WC_REDIR_VARID(pc[3]) &&
 	!pc[4] &&
 	wc_code(pc[6]) == WC_SIMPLE && !WC_SIMPLE_ARGC(pc[6])) {
 	/* $(< word) */
Index: Src/parse.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/parse.c,v
retrieving revision 1.49
diff -u -r1.49 parse.c
--- Src/parse.c	6 Feb 2005 20:36:42 -0000	1.49
+++ Src/parse.c	12 Apr 2005 11:28:23 -0000
@@ -111,6 +111,8 @@
  *     - must precede command-code (or WC_ASSIGN)
  *     - data contains type (<, >, ...)
  *     - followed by fd1 and name from struct redir
+ *     - for the extended form {var}>... where the fd is assigned
+ *       to var, there is an extra item to contain var
  *
  *   WC_ASSIGN
  *     - data contains type (scalar, array) and number of array-elements
@@ -737,7 +739,8 @@
     } else if (tok == BARAMP) {
 	int r;
 
-	for (r = p + 1; wc_code(ecbuf[r]) == WC_REDIR; r += 3);
+	for (r = p + 1; wc_code(ecbuf[r]) == WC_REDIR;
+	     r += WC_REDIR_WORDS(ecbuf[r]));
 
 	ecispace(r, 3);
 	ecbuf[r] = WCB_REDIR(REDIR_MERGEOUT);
@@ -779,8 +782,7 @@
     if (IS_REDIROP(tok)) {
 	*complex = 1;
 	while (IS_REDIROP(tok)) {
-	    nr++;
-	    par_redir(&r);
+	    nr += par_redir(&r, NULL);
 	}
     }
     switch (tok) {
@@ -871,10 +873,10 @@
 		if (!nr)
 		    return 0;
 	    } else {
-		/* Three codes per redirection. */
+		/* Take account of redirections */
 		if (sr > 1) {
 		    *complex = 1;
-		    r += (sr - 1) * 3;
+		    r += sr - 1;
 		}
 	    }
 	}
@@ -883,7 +885,7 @@
     if (IS_REDIROP(tok)) {
 	*complex = 1;
 	while (IS_REDIROP(tok))
-	    par_redir(&r);
+	    (void)par_redir(&r, NULL);
     }
     incmdpos = 1;
     incasepat = 0;
@@ -1510,6 +1512,9 @@
  * simple	: { COMMAND | EXEC | NOGLOB | NOCORRECT | DASH }
 					{ STRING | ENVSTRING | ENVARRAY wordlist OUTPAR | redir }
 					[ INOUTPAR { SEPER } ( list1 | INBRACE list OUTBRACE ) ]
+ *
+ * Returns 0 if no code, else 1 plus the number of code words
+ * used up by redirections.
  */
 
 /**/
@@ -1517,7 +1522,7 @@
 par_simple(int *complex, int nr)
 {
     int oecused = ecused, isnull = 1, r, argc = 0, p, isfunc = 0, sr = 0;
-    int c = *complex;
+    int c = *complex, nrediradd;
 
     r = ecused;
     for (;;) {
@@ -1576,16 +1581,55 @@
 
     for (;;) {
 	if (tok == STRING) {
+	    int redir_var = 0;
+
 	    *complex = 1;
 	    incmdpos = 0;
-	    ecstr(tokstr);
-	    argc++;
-	    yylex();
+
+	    if (!isset(IGNOREBRACES) && *tokstr == Inbrace)
+	    {
+		char *eptr = tokstr + strlen(tokstr) - 1;
+		char *ptr = eptr;
+
+		if (*ptr == Outbrace && ptr > tokstr + 1)
+		{
+		    while (--ptr > tokstr)
+			if (!iident(*ptr))
+			    break;
+		    if (ptr == tokstr)
+		    {
+			char *toksave = tokstr;
+			char *idstring = dupstrpfx(tokstr+1, eptr-tokstr-1);
+			redir_var = 1;
+			yylex();
+
+			if (IS_REDIROP(tok) && tokfd == -1)
+			{
+			    *complex = c = 1;
+			    nrediradd = par_redir(&r, idstring);
+			    p += nrediradd;
+			    sr += nrediradd;
+			}
+			else
+			{
+			    ecstr(toksave);
+			    argc++;
+			}
+		    }
+		}
+	    }
+
+	    if (!redir_var)
+	    {
+		ecstr(tokstr);
+		argc++;
+		yylex();
+	    }
 	} else if (IS_REDIROP(tok)) {
 	    *complex = c = 1;
-	    par_redir(&r);
-	    p += 3;		/* 3 codes per redirection */
-	    sr++;
+	    nrediradd = par_redir(&r, NULL);
+	    p += nrediradd;
+	    sr += nrediradd;
 	} else if (tok == INOUTPAR) {
 	    int oldlineno = lineno, onp, so, oecssub = ecssub;
 
@@ -1670,6 +1714,8 @@
 
 /*
  * redir	: ( OUTANG | ... | TRINANG ) STRING
+ *
+ * Return number of code words required for redirection
  */
 
 static int redirtab[TRINANG - OUTANG + 1] = {
@@ -1691,10 +1737,10 @@
 };
 
 /**/
-static void
-par_redir(int *rp)
+static int
+par_redir(int *rp, char *idstring)
 {
-    int r = *rp, type, fd1, oldcmdpos, oldnc;
+    int r = *rp, type, fd1, oldcmdpos, oldnc, ncodes;
     char *name;
 
     oldcmdpos = incmdpos;
@@ -1706,7 +1752,7 @@
     fd1 = tokfd;
     yylex();
     if (tok != STRING && tok != ENVSTRING)
-	YYERRORV(ecused);
+	YYERROR(ecused);
     incmdpos = oldcmdpos;
     nocorrect = oldnc;
 
@@ -1721,23 +1767,35 @@
     case REDIR_HEREDOCDASH: {
 	/* <<[-] name */
 	struct heredocs **hd;
+	int htype = type;
 
-	/* If we ever need more than three codes (or less), we have to change
-	 * the factors in par_cmd() and par_simple(), too. */
-	ecispace(r, 3);
-	*rp = r + 3;
+	if (idstring)
+	{
+	    type |= REDIR_VARID_MASK;
+	    ncodes = 4;
+	}
+	else
+	    ncodes = 3;
+
+	/* If we ever to change the number of codes, we have to change
+	 * the definition of WC_REDIR_WORDS. */
+	ecispace(r, ncodes);
+	*rp = r + ncodes;
 	ecbuf[r] = WCB_REDIR(type);
 	ecbuf[r + 1] = fd1;
 
+	if (idstring)
+	    ecbuf[r + 3] = ecstrcode(idstring);
+
 	for (hd = &hdocs; *hd; hd = &(*hd)->next);
 	*hd = zalloc(sizeof(struct heredocs));
 	(*hd)->next = NULL;
-	(*hd)->type = type;
+	(*hd)->type = htype;
 	(*hd)->pc = r;
 	(*hd)->str = tokstr;
 
 	yylex();
-	return;
+	return ncodes;
     }
     case REDIR_WRITE:
     case REDIR_WRITENOW:
@@ -1745,14 +1803,14 @@
 	    /* > >(...) */
 	    type = REDIR_OUTPIPE;
 	else if (tokstr[0] == Inang && tokstr[1] == Inpar)
-	    YYERRORV(ecused);
+	    YYERROR(ecused);
 	break;
     case REDIR_READ:
 	if (tokstr[0] == Inang && tokstr[1] == Inpar)
 	    /* < <(...) */
 	    type = REDIR_INPIPE;
 	else if (tokstr[0] == Outang && tokstr[1] == Inpar)
-	    YYERRORV(ecused);
+	    YYERROR(ecused);
 	break;
     case REDIR_READWRITE:
 	if ((tokstr[0] == Inang || tokstr[0] == Outang) && tokstr[1] == Inpar)
@@ -1761,13 +1819,25 @@
     }
     yylex();
 
-    /* If we ever need more than three codes (or less), we have to change
-     * the factors in par_cmd() and par_simple(), too. */
-    ecispace(r, 3);
-    *rp = r + 3;
+    /* If we ever to change the number of codes, we have to change
+     * the definition of WC_REDIR_WORDS. */
+    if (idstring)
+    {
+	type |= REDIR_VARID_MASK;
+	ncodes = 4;
+    }
+    else
+	ncodes = 3;
+
+    ecispace(r, ncodes);
+    *rp = r + ncodes;
     ecbuf[r] = WCB_REDIR(type);
     ecbuf[r + 1] = fd1;
     ecbuf[r + 2] = ecstrcode(name);
+    if (idstring)
+	ecbuf[r + 3] = ecstrcode(idstring);
+
+    return ncodes;
 }
 
 /**/
@@ -2316,6 +2386,10 @@
 	r->type = WC_REDIR_TYPE(code);
 	r->fd1 = *s->pc++;
 	r->name = ecgetstr(s, EC_DUP, NULL);
+	if (WC_REDIR_VARID(code))
+	    r->varid = ecgetstr(s, EC_DUP, NULL);
+	else
+	    r->varid = NULL;
 
 	addlinknode(ret, r);
 
Index: Src/text.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/text.c,v
retrieving revision 1.14
diff -u -r1.14 text.c
--- Src/text.c	22 Jun 2004 13:10:02 -0000	1.14
+++ Src/text.c	12 Apr 2005 11:28:23 -0000
@@ -789,10 +789,15 @@
 	case REDIR_MERGEOUT:
 	case REDIR_INPIPE:
 	case REDIR_OUTPIPE:
-	    if (f->fd1 != (IS_READFD(f->type) ? 0 : 1))
+	    if (f->varid) {
+		taddchr('{');
+		taddstr(f->varid);
+		taddchr('}');
+	    } else if (f->fd1 != (IS_READFD(f->type) ? 0 : 1))
 		taddchr('0' + f->fd1);
 	    taddstr(fstr[f->type]);
-	    taddchr(' ');
+	    if (f->type != REDIR_MERGEIN && f->type != REDIR_MERGEOUT)
+		taddchr(' ');
 	    if (f->type == REDIR_HERESTR) {
                 if (has_token(f->name)) {
                     taddchr('\"');
Index: Src/zsh.h
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/zsh.h,v
retrieving revision 1.72
diff -u -r1.72 zsh.h
--- Src/zsh.h	31 Mar 2005 09:55:00 -0000	1.72
+++ Src/zsh.h	12 Apr 2005 11:28:23 -0000
@@ -246,6 +246,8 @@
     REDIR_INPIPE,		/* < <(...) */
     REDIR_OUTPIPE		/* > >(...) */
 };
+#define REDIR_TYPE_MASK	(0x1f)
+#define REDIR_VARID_MASK (0x20)
 
 #define IS_WRITE_FILE(X)      ((X)>=REDIR_WRITE && (X)<=REDIR_READWRITE)
 #define IS_APPEND_REDIR(X)    (IS_WRITE_FILE(X) && ((X) & 2))
@@ -267,9 +269,14 @@
  */
 #define FDT_INTERNAL		1
 /*
+ * Entry visible to other processes, for example created using
+ * the {varid}> file syntax.
+ */
+#define FDT_EXTERNAL		2
+/*
  * Entry used by output from the XTRACE option.
  */
-#define FDT_XTRACE		2
+#define FDT_XTRACE		3
 #ifdef PATH_DEV_FD
 /*
  * Entry used by a process substition.
@@ -277,7 +284,7 @@
  * decremented on exit; we don't close entries greater than
  * FDT_PROC_SUBST except when closing everything.
  */
-#define FDT_PROC_SUBST		3
+#define FDT_PROC_SUBST		4
 #endif
 
 /* Flags for input stack */
@@ -453,6 +460,7 @@
     int type;
     int fd1, fd2;
     char *name;
+    char *varid;
 };
 
 /* The number of fds space is allocated for  *
@@ -629,8 +637,11 @@
 #define WC_PIPE_LINENO(C)   (wc_data(C) >> 1)
 #define WCB_PIPE(T,L)       wc_bld(WC_PIPE, ((T) | ((L) << 1)))
 
-#define WC_REDIR_TYPE(C)    wc_data(C)
+#define WC_REDIR_TYPE(C)    (wc_data(C) & REDIR_TYPE_MASK)
+#define WC_REDIR_VARID(C)   (wc_data(C) & REDIR_VARID_MASK)
 #define WCB_REDIR(T)        wc_bld(WC_REDIR, (T))
+/* Size of redir is 4 words if REDIR_VARID_MASK is set, else 3 */
+#define WC_REDIR_WORDS(C)   (WC_REDIR_VARID(C) ? 4 : 3)
 
 #define WC_ASSIGN_TYPE(C)   (wc_data(C) & ((wordcode) 1))
 #define WC_ASSIGN_TYPE2(C)  ((wc_data(C) & ((wordcode) 2)) >> 1)
Index: Test/A04redirect.ztst
===================================================================
RCS file: /cvsroot/zsh/zsh/Test/A04redirect.ztst,v
retrieving revision 1.5
diff -u -r1.5 A04redirect.ztst
--- Test/A04redirect.ztst	30 Jun 2004 10:01:05 -0000	1.5
+++ Test/A04redirect.ztst	12 Apr 2005 11:28:23 -0000
@@ -235,3 +235,28 @@
 0:null redir with NULLCMD=cat
 <input
 >input
+
+  exec {myfd}>logfile
+  print This is my logfile. >&$myfd
+  print Examining contents of logfile...
+  cat logfile
+0:Using {fdvar}> syntax to open a new file descriptor
+>Examining contents of logfile...
+>This is my logfile.
+
+  exec {myfd}>&-
+  print This message should disappear >&$myfd
+1q:Closing file descriptor using brace syntax
+?(eval):2: $myfd: bad file descriptor
+
+  typeset -r myfd
+  echo This should not appear {myfd}>nologfile
+1:Error opening file descriptor using readonly variable
+?(eval):2: read-only variable: myfd
+
+  typeset +r myfd
+  exec {myfd}>newlogfile
+  typeset -r myfd
+  exec {myfd}>&-
+1:Error closing file descriptor using readonly variable
+?(eval):4: can't close file descriptor from readonly parameter

-- 
Peter Stephenson <pws@csr.com>                  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.

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

Re: PATCH: allocating a new file descriptor

[Peter Stephenson]

The error message from a bad file descriptor is the system error
message, so it can change.  We can work it out by using a file
descriptor which is almost certainly invalid.

Index: Test/A04redirect.ztst
===================================================================
RCS file: /cvsroot/zsh/zsh/Test/A04redirect.ztst,v
retrieving revision 1.6
diff -u -r1.6 A04redirect.ztst
--- Test/A04redirect.ztst	12 Apr 2005 15:11:12 -0000	1.6
+++ Test/A04redirect.ztst	12 Apr 2005 17:33:43 -0000
@@ -3,6 +3,10 @@
 %prep
   mkdir redir.tmp && cd redir.tmp
 
+  myfd=99
+  (echo >&$myfd) 2>msg
+  bad_fd_msg="${$(<msg)##*:}"
+
 %test
 
   print 'This is file redir' >redir  &&  cat redir
@@ -247,7 +251,7 @@
   exec {myfd}>&-
   print This message should disappear >&$myfd
 1q:Closing file descriptor using brace syntax
-?(eval):2: $myfd: bad file descriptor
+?(eval):2: $myfd:$bad_fd_msg
 
   typeset -r myfd
   echo This should not appear {myfd}>nologfile

-- 
Peter Stephenson <pws@csr.com>                  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.

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

Re: PATCH: allocating a new file descriptor

[Bart Schaefer]

On Apr 12,  1:57pm, Peter Stephenson wrote:
}
} This gives us much better encapsulation for manipulating file
} descriptors, something along the lines of files or file handles in
} other languages.

I think some oddball behaviors here might be considered bugs.

For example,

    print foo {myfd}>/dev/null

assigns a new fd to $myfd, but does not redirect the output of "print"
to it, and leaves it open after "print" is finished.

Futher, executing the same command again assigns a second new fd to $myfd
but leaves the first one "dangling" (no way to close it, because you no
longer know what its number is).

I don't think Korn's summary implied either of these behaviors, but if
it does they should probably be brought to his attention.

} The shell checks that the braces only contain valid characters for a
} parameter ID, so the only likely clash with this syntax is for avid
} users of the BRACE_CCL option where {myfd}>~/tmp/logfile would perform a
} normal redirection

Perhaps setopt BRACE_CCL should simply disable/override this behavior?

Re: PATCH: allocating a new file descriptor

[Peter Stephenson]

Bart Schaefer wrote:
> For example,
> 
>     print foo {myfd}>/dev/null
> 
> assigns a new fd to $myfd, but does not redirect the output of "print"
> to it, and leaves it open after "print" is finished.
> 
> Futher, executing the same command again assigns a second new fd to $myfd
> but leaves the first one "dangling" (no way to close it, because you no
> longer know what its number is).

That's just how it works.  It's not supposed to redirect print to the
new fd, it's specifically a syntax to allocate an fd only, and it's
quite explicit that the number is stored in the variable, so if you want
to close it you keep the variable around.  It's only a bug if you
confuse allocation and closing of fd's with their use (which doesn't
mean people won't, nor not neither using double negatives).

I can add documentation to point this out if you think it would be
useful.

It would also be possible to make NO_CLOBBER check that in {myfd}>stuff,
$myfd doesn't already point to an fd with an FDT_EXTERNAL flag.  That
would prevent overwriting.  I quite like that idea.  You could still
loop:

  for file in $filelist; do
     exec {myfd}>$file

     do_stuff_with >&$myfd

     exec {myfd}>&-
  done

because the test wouldn't trigger after $myfd was closed.  For more
complicated cases you might need to set myfd to 0 first (fd's below 10
are never marked with the flag).  Obviously NO_CLOBBER wouldn't apply to
closing the fd.

I don't think there's a good argument for adding automatic redirection,
though.  Fortuitously,

  print This is a log file {myfd}>~/tmp/logfile.txt >&$myfd

works; $myfd is substituted at the right point.  That's quite lucky,
since most expansions have been done by this point.

However, there's also the point that if you use this after an external
command the shell has already forked and the fd doesn't appear in the
parent shell.  Allocation of fd's is most naturally done after "exec"
only.

-- 
Peter Stephenson <pws@csr.com>                  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.

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

Re: PATCH: allocating a new file descriptor

[Bart Schaefer]

On Apr 14, 10:49am, Peter Stephenson wrote:
} Subject: Re: PATCH: allocating a new file descriptor
}
} I can add documentation to point this out if you think it would be
} useful.

Possibly so.

} It would also be possible to make NO_CLOBBER check that in {myfd}>stuff,
} $myfd doesn't already point to an fd with an FDT_EXTERNAL flag.  That
} would prevent overwriting.  I quite like that idea.

I do, too.

} For more complicated cases you might need to set myfd to 0 first

Or (more appropriately, I would think) unset it, no?

(Given discipline functions, one could even arrange that myfd closes
its descriptor upon unset.)

} Fortuitously,
} 
}   print This is a log file {myfd}>~/tmp/logfile.txt >&$myfd
} 
} works; $myfd is substituted at the right point.  That's quite lucky,
} since most expansions have been done by this point.

At first I thought that was a little odd, but I guess really it's OK,
because this ...

    print ${myfd::=2} $myfd

... also prints "2 2".

It might be useful to add a note somewhere (the "Expansion" section?)
about the ordering of parameter expansion relative to redirections.

} However, there's also the point that if you use this after an external
} command the shell has already forked and the fd doesn't appear in the
} parent shell. 

That's a very interesting point.  This works:

    /bin/echo foo {myfd}>>(tr a-z A-Z) >&$myfd

But this causes the shell to hang forever:

    print foo {myfd}>>(tr a-z A-Z) >&$myfd

-- 
Bart Schaefer                                 Brass Lantern Enterprises
http://www.well.com/user/barts              http://www.brasslantern.com

Zsh: http://www.zsh.org | PHPerl Project: http://phperl.sourceforge.net   

Re: PATCH: allocating a new file descriptor

[Peter Stephenson]

Bart Schaefer wrote:
> On Apr 14, 10:49am, Peter Stephenson wrote:
> } Subject: Re: PATCH: allocating a new file descriptor
> }
> } I can add documentation to point this out if you think it would be
> } useful.
> 
> Possibly so.

I've tried to improve it.

> } It would also be possible to make NO_CLOBBER check that in {myfd}>stuff,
> } $myfd doesn't already point to an fd with an FDT_EXTERNAL flag.  That
> } would prevent overwriting.  I quite like that idea.
> 
> I do, too.

I've done this, and in the process handled some other error cases
better, though it's meant the code is a little more verbose.  It now
doesn't create the file on {myfd}>file if myfd is readonly and I picked
up some possible leaks on errors.

> } For more complicated cases you might need to set myfd to 0 first
> 
> Or (more appropriately, I would think) unset it, no?

Probably, I was thinking of the case where you'd just declared it as an
integer, but in practice the effect is the same.

> It might be useful to add a note somewhere (the "Expansion" section?)
> about the ordering of parameter expansion relative to redirections.

I couldn't think of anywhere obvious in that section, but I've made it
more explicit in the redirection section.

> But this causes the shell to hang forever:
> 
>     print foo {myfd}>>(tr a-z A-Z) >&$myfd

The fix to make the shell wait until the process exited (to make this
synchronous when used as a pipe for stdout or stderr) was coming into
effect, but shouldn't have since the fd stays valid until explicitly
closed.  This fixes it.

Index: Doc/Zsh/redirect.yo
===================================================================
RCS file: /cvsroot/zsh/zsh/Doc/Zsh/redirect.yo,v
retrieving revision 1.8
diff -u -r1.8 redirect.yo
--- Doc/Zsh/redirect.yo	12 Apr 2005 15:11:11 -0000	1.8
+++ Doc/Zsh/redirect.yo	14 Apr 2005 16:04:02 -0000
@@ -174,6 +174,27 @@
 descriptor using tt(<&$)var(param) or tt(>&$)var(param) if var(param) is
 readonly.
 
+If the option tt(CLOBBER) is unset, it is an error to open a file
+descriptor using a parameter that is already set to an open file descriptor
+previously allocated by this mechanism.  Unsetting the parameter before
+using it for allocating a file descriptor avoids the error.
+
+Note that this mechanism merely allocates or closes a file descriptor; it
+does not perform any redirections from or to it.  It is usually convenient
+to allocate a file descriptor prior to use as an argument to tt(exec).  The
+following shows a typical sequence of allocation, use, and closing of a
+file descriptor:
+
+example(integer myfd
+exec {myfd}>~/logs/mylogfile.txt
+print This is a log message. >&$myfd
+exec {myfd}>&-)
+
+Note that the expansion of the variable in the expression tt(>&$myfd)
+occurs at the point the redirection is opened.  This is after the expansion
+of command arguments and after any redirections to the left on the command
+line have been processed.
+
 The `tt(|&)' command separator described in
 ifzman(em(Simple Commands & Pipelines) in zmanref(zshmisc))\
 ifnzman(noderef(Simple Commands & Pipelines))
@@ -230,6 +251,10 @@
 
 is equivalent to `tt(cat foo fubar | sort)'.
 
+Expansion of the redirection argument occurs at the point the redirection
+is opened, at the point described above for the expansion of the variable
+in tt(>&$myfd).
+
 Note that a pipe is an implicit redirection; thus
 
 example(cat bar | sort <foo)
Index: Src/exec.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/exec.c,v
retrieving revision 1.87
diff -u -r1.87 exec.c
--- Src/exec.c	12 Apr 2005 15:11:11 -0000	1.87
+++ Src/exec.c	14 Apr 2005 16:04:03 -0000
@@ -1405,6 +1405,41 @@
     }
 }
 
+/* Check that we can use a parameter for allocating a file descriptor. */
+
+static int
+checkclobberparam(struct redir *f)
+{
+    struct value vbuf;
+    Value v;
+    char *s = f->varid;
+    int fd;
+
+    if (!s)
+	return 1;
+
+    if (!(v = getvalue(&vbuf, &s, 0)))
+	return 1;
+
+    if (v->pm->flags & PM_READONLY) {
+	zwarn("can't allocate file descriptor to readonly parameter %s",
+	      f->varid, 0);
+	/* don't flag a system error for this */
+	errno = 0;
+	return 0;
+    }
+
+    if (!isset(CLOBBER) && (fd = (int)getintvalue(v)) &&
+	fd <= max_zsh_fd && fdtable[fd] == FDT_EXTERNAL) {
+	zwarn("can't clobber parameter %s containing file descriptor %d",
+	     f->varid, fd);
+	/* don't flag a system error for this */
+	errno = 0;
+	return 0;
+    }
+    return 1;
+}
+
 /* Open a file for writing redirection */
 
 /**/
@@ -2239,17 +2274,22 @@
     /* Do io redirections */
     while (redir && nonempty(redir)) {
 	fn = (Redir) ugetnode(redir);
+
 	DPUTS(fn->type == REDIR_HEREDOC || fn->type == REDIR_HEREDOCDASH,
 	      "BUG: unexpanded here document");
 	if (fn->type == REDIR_INPIPE) {
-	    if (fn->fd2 == -1) {
+	    if (!checkclobberparam(fn) || fn->fd2 == -1) {
+		if (fn->fd2 != -1)
+		    zclose(fn->fd2);
 		closemnodes(mfds);
 		fixfds(save);
 		execerr();
 	    }
 	    addfd(forked, save, mfds, fn->fd1, fn->fd2, 0, fn->varid);
 	} else if (fn->type == REDIR_OUTPIPE) {
-	    if (fn->fd2 == -1) {
+	    if (!checkclobberparam(fn) || fn->fd2 == -1) {
+		if (fn->fd2 != -1)
+		    zclose(fn->fd2);
 		closemnodes(mfds);
 		fixfds(save);
 		execerr();
@@ -2271,11 +2311,14 @@
 		continue;
 	    switch(fn->type) {
 	    case REDIR_HERESTR:
-		fil = getherestr(fn);
+		if (!checkclobberparam(fn))
+		    fil = -1;
+		else
+		    fil = getherestr(fn);
 		if (fil == -1) {
 		    closemnodes(mfds);
 		    fixfds(save);
-		    if (errno != EINTR)
+		    if (errno && errno != EINTR)
 			zwarn("%e", NULL, errno);
 		    execerr();
 		}
@@ -2283,7 +2326,9 @@
 		break;
 	    case REDIR_READ:
 	    case REDIR_READWRITE:
-		if (fn->type == REDIR_READ)
+		if (!checkclobberparam(fn))
+		    fil = -1;
+		else if (fn->type == REDIR_READ)
 		    fil = open(unmeta(fn->name), O_RDONLY | O_NOCTTY);
 		else
 		    fil = open(unmeta(fn->name),
@@ -2335,7 +2380,9 @@
 	    case REDIR_MERGEOUT:
 		if (fn->fd2 < 10)
 		    closemn(mfds, fn->fd2);
-		if (fn->fd2 > 9 &&
+		if (!checkclobberparam(fn))
+		    fil = -1;
+		else if (fn->fd2 > 9 &&
 		    ((fdtable[fn->fd2] != FDT_UNUSED &&
 		      fdtable[fn->fd2] != FDT_EXTERNAL) ||
 		     fn->fd2 == coprocin ||
@@ -2355,14 +2402,18 @@
 		    fixfds(save);
 		    if (fn->fd2 != -2)
 		    	sprintf(fdstr, "%d", fn->fd2);
-		    zwarn("%s: %e", fn->fd2 == -2 ? "coprocess" : fdstr, errno);
+		    if (errno)
+			zwarn("%s: %e", fn->fd2 == -2 ? "coprocess" : fdstr,
+			      errno);
 		    execerr();
 		}
 		addfd(forked, save, mfds, fn->fd1, fil,
 		      fn->type == REDIR_MERGEOUT, fn->varid);
 		break;
 	    default:
-		if (IS_APPEND_REDIR(fn->type))
+		if (!checkclobberparam(fn))
+		    fil = -1;
+		else if (IS_APPEND_REDIR(fn->type))
 		    fil = open(unmeta(fn->name),
 			       (unset(CLOBBER) && !IS_CLOBBER_REDIR(fn->type)) ?
 			       O_WRONLY | O_APPEND | O_NOCTTY :
@@ -2378,7 +2429,7 @@
 			close(fil);
 		    closemnodes(mfds);
 		    fixfds(save);
-		    if (errno != EINTR)
+		    if (errno && errno != EINTR)
 			zwarn("%e: %s", fn->name, errno);
 		    execerr();
 		}
@@ -2387,7 +2438,10 @@
 		    addfd(forked, save, mfds, 2, dfil, 1, NULL);
 		break;
 	    }
+	    /* May be error in addfd due to setting parameter. */
 	    if (errflag) {
+		closemnodes(mfds);
+		fixfds(save);
 		execerr();
 	    }
 	}
@@ -3234,6 +3288,9 @@
  * If the second argument is 1, this is part of
  * an "exec < <(...)" or "exec > >(...)" and we shouldn't
  * wait for the job to finish before continuing.
+ * Likewise, we shouldn't wait if we are opening the file
+ * descriptor using the {fd}>>(...) notation since it stays
+ * valid for subsequent commands.
  */
 
 /**/
@@ -3249,7 +3306,7 @@
 	f = (Redir) getdata(n);
 	if (f->type == REDIR_OUTPIPE || f->type == REDIR_INPIPE) {
 	    str = f->name;
-	    f->fd2 = getpipe(str, nullexec);
+	    f->fd2 = getpipe(str, nullexec || f->varid);
 	}
     }
 }
Index: Test/A04redirect.ztst
===================================================================
RCS file: /cvsroot/zsh/zsh/Test/A04redirect.ztst,v
retrieving revision 1.7
diff -u -r1.7 A04redirect.ztst
--- Test/A04redirect.ztst	12 Apr 2005 17:40:06 -0000	1.7
+++ Test/A04redirect.ztst	14 Apr 2005 16:04:03 -0000
@@ -248,6 +248,11 @@
 >Examining contents of logfile...
 >This is my logfile.
 
+  setopt noclobber
+  exec {myfd}>logfile2
+1q:NO_CLOBBER prevents overwriting parameter with allocated fd
+?(eval):2: can't clobber parameter myfd containing file descriptor $myfd
+
   exec {myfd}>&-
   print This message should disappear >&$myfd
 1q:Closing file descriptor using brace syntax
@@ -256,7 +261,7 @@
   typeset -r myfd
   echo This should not appear {myfd}>nologfile
 1:Error opening file descriptor using readonly variable
-?(eval):2: read-only variable: myfd
+?(eval):2: can't allocate file descriptor to readonly parameter myfd
 
   typeset +r myfd
   exec {myfd}>newlogfile


-- 
Peter Stephenson <pws@csr.com>                  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.

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

Re: PATCH: allocating a new file descriptor

[Stephane Chazelas]

I understand that

exec {fd}> file

is the shell equivalent of

fd = open(file, O_WRONLY|O_CREAT);

given that we already have ztcp, zsocket... that allocate fds,
and sysread, syswrite..., I would find it more consistent to
have:

sysopen -w file
fd=$REPLY

(with -x (O_EXCL), -r (read), -w (write), -a (append), -n (don't
create), -d <fd> and possibly extensions for O_SYNC, O_NOATIME...)

I find the:

print ... {fd}> file

that keeps the file open a bit confusing and unconsistent (and
useless).

It would be good to have a sysclose as well. At least until
recently, exec 12>&- didn't work. I had the problem with my
mouse support, where I couldn't close the fd opened by zsocket
for gpm interaction.

I also noticed several problems when fidling with fds used
internally by zsh (zsocket -d10 or -d11 ended up in a core
dump).

sysdup2 may come handy as well if exec 12>&13, or exec $a>&$b
don't work.

regards,
Stéphane

Re: PATCH: allocating a new file descriptor

[Peter Stephenson]

Stephane Chazelas wrote:
> I would find it more consistent to
> have:
> 
> sysopen -w file
> fd=$REPLY

Yes, that could be added to the zsh/system module easily enough, but it
would be zsh-specific.  I've simply never needed it up to now.

> I find the:
> 
> print ... {fd}> file
> 
> that keeps the file open a bit confusing and unconsistent (and
> useless).

You mean with print?  That's why the manual now suggests using it with
exec.

The syntax is designed to be consistent with existing standard shell
syntax, which imposes quite strong limitations.

One annoyance is that if you use an older version of the shell, the exec
tries to execute {fd} and disappears, which isn't a particularly
graceful way of failing.

> It would be good to have a sysclose as well. At least until
> recently, exec 12>&- didn't work. I had the problem with my
> mouse support, where I couldn't close the fd opened by zsocket
> for gpm interaction.

That syntax is deliberately restricted to single digits and will remain
so.  You can now do:

fd=12
exec {fd}>&-

(Presumably you already have the fd in a parameter, in fact.)  The
socket module is more limited than the tcp module; it doesn't record
sessions, so has no way of knowing which fd's you might want to close.
That may be why there's no zsocket -c.

> I also noticed several problems when fidling with fds used
> internally by zsh (zsocket -d10 or -d11 ended up in a core
> dump).

Yes, it doesn't check for existing use of the fd by the shell.

The same problem exists with the new syntax when closing fd's; I've made
it check for special fd's, which are those from 10 upwards marked as for
internal use.  This covers 10, but not 11; I'm not sure what opened
that, but I think it's some library rather than the shell itself.
If we can track it down it could be marked as in internal use, too.

(Maybe it's about time we introduced zwarn and friends to stdarg.h...)

Index: Src/exec.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/exec.c,v
retrieving revision 1.88
diff -u -r1.88 exec.c
--- Src/exec.c	14 Apr 2005 16:24:52 -0000	1.88
+++ Src/exec.c	15 Apr 2005 10:18:15 -0000
@@ -77,7 +77,7 @@
  * by zclose.                                                      */
 
 /**/
-unsigned char *fdtable;
+mod_export unsigned char *fdtable;
 
 /* The allocated size of fdtable */
 
@@ -87,7 +87,7 @@
 /* The highest fd that marked with nonzero in fdtable */
 
 /**/
-int max_zsh_fd;
+mod_export int max_zsh_fd;
 
 /* input fd from the coprocess */
 
@@ -2360,13 +2360,22 @@
 			bad = 2;
 		    } else {
 			fn->fd1 = (int)getintvalue(v);
-			bad = errflag;
+			if (errflag)
+			    bad = 1;
+			else if (fn->fd1 > max_zsh_fd)
+			    bad = 3;
+			else if (fn->fd1 >= 10 &&
+				 fdtable[fn->fd1] == FDT_INTERNAL)
+			    bad = 4;
 		    }
 		    if (bad) {
-			zwarn(bad == 2 ?
-			      "can't close file descriptor from readonly parameter" :
-			      "parameter %s does not contain a file descriptor",
-			      fn->varid, 0);
+			const char *bad_msg[] = {
+			    "parameter %s does not contain a file descriptor",
+			    "can't close file descriptor from readonly parameter %s",
+			    "file descriptor %d out of range, not closed",
+			    "file descriptor %d used by shell, not closed"
+			};
+			zwarn(bad_msg[bad-1], fn->varid, fn->fd1);
 			execerr();
 		    }
 		}
Index: Src/Modules/socket.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Modules/socket.c,v
retrieving revision 1.7
diff -u -r1.7 socket.c
--- Src/Modules/socket.c	16 Nov 2004 11:05:04 -0000	1.7
+++ Src/Modules/socket.c	15 Apr 2005 10:18:15 -0000
@@ -78,6 +78,11 @@
 		     OPT_ARG(ops, 'd'), 0);
 	    return 1;
 	}
+	if (targetfd <= max_zsh_fd && fdtable[targetfd] != FDT_UNUSED) {
+	    zwarnnam(nam, "file descriptor %d is in use by the shell",
+		     NULL, targetfd);
+	    return 1;
+	}
     }
 
     if (OPT_ISSET(ops,'l')) {

-- 
Peter Stephenson <pws@csr.com>                  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.

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

Re: PATCH: allocating a new file descriptor

[Bart Schaefer]

On Apr 15, 11:27am, Peter Stephenson wrote:
}
} > I also noticed several problems when fidling with fds used
} > internally by zsh (zsocket -d10 or -d11 ended up in a core
} > dump).
} 
} Yes, it doesn't check for existing use of the fd by the shell.
} 
} The same problem exists with the new syntax when closing fd's; I've made
} it check for special fd's, which are those from 10 upwards marked as for
} internal use.  This covers 10, but not 11; I'm not sure what opened
} that, but I think it's some library rather than the shell itself.
} If we can track it down it could be marked as in internal use, too.
} 
} (Maybe it's about time we introduced zwarn and friends to stdarg.h...)

Also maybe it's time we introduced an API in the main part of zsh for
opening and closing files, so that we can stop doing things like this ...

} -unsigned char *fdtable;
} +mod_export unsigned char *fdtable;

[...]

} Index: Src/Modules/socket.c
} ===================================================================
} +	if (targetfd <= max_zsh_fd && fdtable[targetfd] != FDT_UNUSED) {

... and instead encapsulate those sorts of tests in the API functions.
The number of exported globals is starting to bother me; it feels as if
every second patch that's gone by in the past few months has exported
another previously-hidden global symbol.

Re: PATCH: allocating a new file descriptor

[Peter Stephenson]

Bart Schaefer wrote:
> Also maybe it's time we introduced an API in the main part of zsh for
> opening and closing files, so that we can stop doing things like this ...

Fine, although I don't suppose anybody's actually going to do it...
The issue is actually checking the state of fds and fixing up the state
after the open; in several modules, including socket, we need to open fd's
in non-standard ways.  That's about two new functions already.
Wrapping the whole thing up is probably a lot of work.

pws


**********************************************************************
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.

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