Re: 4.3.12-test-2

[Peter Stephenson <p.w.stephenson@xxxxxxxxxxxx>]

On Fri, 21 Oct 2011 12:08:48 +0100
Peter Stephenson <Peter.Stephenson@xxxxxxx> wrote:
> > /home/phil/.zshenv:133: failed to close file descriptor 1: bad file descriptor
> 
> Hmm... I'm getting an error from closing stdout (just once), too.  That
> doesn't seem right and needs looking into.  Possibly the error is
> failing to close it, rather than the new error message (which is
> deliberate if the shell fails to do an operation on file descriptors
> you've explicitly asked for), so the real problem may not be new.

It looks like there isn't really a problem, apart from the error: we
were attempting to close an fd that had already been closed, which would
have caused a system error but not actually done any damage.  This stops
the error.  Could do with a test.

I got a bit worried about why we'd be moving the fd rather than just
closing it completely in the case of exec (which was showing up the same
error), but there's a special case later on for an exec that's just
doing redirections where we close the saved fd --- a bit baroque, but
presumably OK.

Index: Src/exec.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/exec.c,v
retrieving revision 1.204
diff -p -u -r1.204 exec.c
--- Src/exec.c	28 Aug 2011 16:38:28 -0000	1.204
+++ Src/exec.c	23 Oct 2011 17:15:05 -0000
@@ -2912,6 +2912,7 @@ execcmd(Estate state, int input, int out
 	    }
 	    addfd(forked, save, mfds, fn->fd1, fn->fd2, 1, fn->varid);
 	} else {
+	    int closed;
 	    if (fn->type != REDIR_HERESTR && xpandredir(fn, redir))
 		continue;
 	    if (errflag) {
@@ -3002,11 +3003,20 @@ execcmd(Estate state, int input, int out
 		 * 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)
+		closed = 0;
+		if (!forked && fn->fd1 < 10 && save[fn->fd1] == -2) {
 		    save[fn->fd1] = movefd(fn->fd1);
+		    if (save[fn->fd1] >= 0) {
+			/*
+			 * The original fd is now closed, we don't need
+			 * to do it below.
+			 */
+			closed = 1;
+		    }
+		}
 		if (fn->fd1 < 10)
 		    closemn(mfds, fn->fd1);
-		if (zclose(fn->fd1) < 0) {
+		if (!closed && zclose(fn->fd1) < 0) {
 		    zwarn("failed to close file descriptor %d: %e",
 			  fn->fd1, errno);
 		}


-- 
Peter Stephenson <p.w.stephenson@xxxxxxxxxxxx>
Web page now at http://homepage.ntlworld.com/p.w.stephenson/