Re: bin/51123: /bin/sh fails to close file (N>&-) in some cases (+FIX)

[Robert Elz <kre%munnari.OZ.AU@localhost>]

The following reply was made to PR bin/51123; it has been noted by GNATS.

From: Robert Elz <kre%munnari.OZ.AU@localhost>
To: gnats-bugs%NetBSD.org@localhost
Cc: 
Subject: Re: bin/51123: /bin/sh fails to close file (N>&-) in some cases (+FIX)
Date: Sun, 08 May 2016 14:17:44 +0700

     Date:        Sun,  8 May 2016 04:15:00 +0000 (UTC)
     From:        kre%munnari.OZ.AU@localhost
     Message-ID:  <20160508041500.2E11C7A473%mollari.NetBSD.org@localhost>
 
   | >Fix:
   | 	Apply the following patch (against current).
 
 I have a different patch.  This one works just as well for solving the
 problem in the PR, but also allows
 
 	ls >/dev/stdout
 
 to work.   That's been broken forever - except, as it happens in the cases
 where the bug from this PR exists (so echo hello >/dev/stdout works, as
 does for ... do ... done >/dev/stdout etc, since the bug was installed in 
 2014.)
 
 The problem is/was that the fd we are redirecting was closed, then the
 file was opened, but "close(1); open("/dev/stdout",...)" is pretty much
 guaranteed to fail.   The bug from the PR (which "forgot" to do the close
 in some cases) allowed it to work.
 
 The same applies to </dev/stdin and 2>/dev/stderr of course.   (I don't
 want to even imagine why anyone wants to do any of this, but apparently
 some people do...)
 
 The patch appended handles the PR problem in a different way, and as a side
 effect, never closes the fd before we open the replacement.   This can 
 sometimes save a system call, as we used to do "close(), open(), dup2()"
 and now we just do "open(), dup2()" in the normal case.   But only sometimes
 save, as if open happens to return the fd we want to use, which would be
 common when that fd was 0, 1, or 2, and has just been closed, the dup2()
 was not needed, so now we're swapping a close() for a dup2().
 
 This patch applies to current in CVS (simply ignoring the previous patch
 which was never committed.)
 
 kre
 
 Index: redir.c
 ===================================================================
 RCS file: /cvsroot/src/bin/sh/redir.c,v
 retrieving revision 1.44
 diff -u -u -r1.44 redir.c
 --- redir.c	8 May 2016 03:51:15 -0000	1.44
 +++ redir.c	8 May 2016 07:07:13 -0000
 @@ -219,8 +219,6 @@
  				(void)fcntl(i, F_SETFD, FD_CLOEXEC);
  			fd_rename(sv, fd, i);
  			INTON;
 -		} else {
 -			close(fd);
  		}
  		if (fd == 0)
  			fd0_redirected++;
 @@ -306,7 +304,8 @@
  			else
  				copyfd(redir->ndup.dupfd, fd, 1,
  				    (flags & REDIR_PUSH) != 0);
 -		}
 +		} else
 +			close(fd);
  		INTON;
  		return;
  	case NHERE: