29561: Allow closing of fd's not recorded by the shell

3 files changed, 31 insertions, 22 deletions

diff --git a/ChangeLog b/ChangeLog
index 3f8e821c9..b5a2baddf 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@
+2011-07-25  Peter Stephenson  <pws@csr.com>
+
+	* 29561: Src/exec.c, Src/utils.c, Test/A04redirect.ztst: Allow
+	closing of file descriptors not recorded internally by the shell.
+
 2011-07-22  Mikael Magnusson <mikachu@gmail.com>
 
 	* 29596: Completion/compinit: Fix syntax to work with KSH_ARRAYS
@@ -15154,5 +15159,5 @@
 
 *****************************************************
 * This is used by the shell to define $ZSH_PATCHLEVEL
-* $Revision: 1.5406 $
+* $Revision: 1.5407 $
 *****************************************************
diff --git a/Src/exec.c b/Src/exec.c
index f5b59a36e..6320f6a67 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -2975,17 +2975,16 @@ execcmd(Estate state, int input, int output, int how, int last1)
 			fn->fd1 = (int)getintvalue(v);
 			if (errflag)
 			    bad = 1;
-			else if (fn->fd1 > max_zsh_fd)
-			    bad = 3;
-			else if (fn->fd1 >= 10 &&
+			else if (fn->fd1 <= max_zsh_fd) {
+			    if (fn->fd1 >= 10 &&
 				 fdtable[fn->fd1] == FDT_INTERNAL)
-			    bad = 4;
+				bad = 3;
+			}
 		    }
 		    if (bad) {
 			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"
 			};
 			if (bad > 2)
@@ -2995,11 +2994,18 @@ execcmd(Estate state, int input, int output, int how, int last1)
 			execerr();
 		    }
 		}
+		/*
+		 * Note we may attempt to close an fd beyond max_zsh_fd:
+		 * OK as long as we never look in fdtable for it.
+ 		 */
 		if (!forked && fn->fd1 < 10 && save[fn->fd1] == -2)
 		    save[fn->fd1] = movefd(fn->fd1);
 		if (fn->fd1 < 10)
 		    closemn(mfds, fn->fd1);
-		zclose(fn->fd1);
+		if (zclose(fn->fd1) < 0) {
+		    zwarn("failed to close file descriptor %d: %e",
+			  fn->fd1, errno);
+		}
 		break;
 	    case REDIR_MERGEIN:
 	    case REDIR_MERGEOUT:
diff --git a/Src/utils.c b/Src/utils.c
index d50311637..f460c0fb3 100644
--- a/Src/utils.c
+++ b/Src/utils.c
@@ -1802,22 +1802,20 @@ zclose(int fd)
 {
     if (fd >= 0) {
 	/*
-	 * We sometimes zclose() an fd twice where the second
-	 * time is a catch-all in case there was a failure using
-	 * the fd.  This is harmless but we need to trap it
-	 * for the error check here.
+	 * Careful: we allow closing of arbitrary fd's, beyond
+	 * max_zsh_fd.  In that case we don't try anything clever.
 	 */
-	DPUTS2(fd > max_zsh_fd && fdtable[fd] != FDT_UNUSED,
-	       "BUG: fd is %d, max_zsh_fd is %d", fd, max_zsh_fd);
-	if (fdtable[fd] == FDT_FLOCK)
-	    fdtable_flocks--;
-	fdtable[fd] = FDT_UNUSED;
-	while (max_zsh_fd > 0 && fdtable[max_zsh_fd] == FDT_UNUSED)
-	    max_zsh_fd--;
-	if (fd == coprocin)
-	    coprocin = -1;
-	if (fd == coprocout)
-	    coprocout = -1;
+	if (fd <= max_zsh_fd) {
+	    if (fdtable[fd] == FDT_FLOCK)
+		fdtable_flocks--;
+	    fdtable[fd] = FDT_UNUSED;
+	    while (max_zsh_fd > 0 && fdtable[max_zsh_fd] == FDT_UNUSED)
+		max_zsh_fd--;
+	    if (fd == coprocin)
+		coprocin = -1;
+	    if (fd == coprocout)
+		coprocout = -1;
+	}
 	return close(fd);
     }
     return -1;