Re: [JOBS] Do not close stderr when /dev/tty fails to open

[Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>]

On Sun, Feb 22, 2009 at 07:33:04PM +0800, Herbert Xu wrote:
> [JOBS] Do not close stderr when /dev/tty fails to open

Turns out that there was more to this than jobs.c  The use of
savefd in redir ended up closing the wrong file descriptor too,
albeit in a harmless manner.  I'm going to throw this fix in.

commit 3db215abfe4c0079cc932d7070ad675e5f6273ea
Author: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>
Date:   Sat May 23 12:21:26 2009 +1000

    [REDIR] Fix incorrect savefd conversions
    
    When I added savefd we ended up closing the wrong fd in two of
    the three places where it was used.  This is because we often
    want to close an fd other than the one that was dupped.  This
    patch fixes this by adding a second argument to savefd denoting
    the fd to be closed.
    
    Signed-off-by: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>

diff --git a/src/input.c b/src/input.c
index 27c4fd1..1e198e9 100644
--- a/src/input.c
+++ b/src/input.c
@@ -410,7 +410,7 @@ setinputfile(const char *fname, int flags)
 		sh_error("Can't open %s", fname);
 	}
 	if (fd < 10)
-		fd = savefd(fd);
+		fd = savefd(fd, fd);
 	setinputfd(fd, flags & INPUT_PUSH_FILE);
 out:
 	INTON;
diff --git a/src/jobs.c b/src/jobs.c
index b1ab7ab..a4fada0 100644
--- a/src/jobs.c
+++ b/src/jobs.c
@@ -189,17 +189,15 @@ setjobctl(int on)
 	if (on == jobctl || rootshell == 0)
 		return;
 	if (on) {
-		fd = open(_PATH_TTY, O_RDWR);
+		int ofd;
+		ofd = fd = open(_PATH_TTY, O_RDWR);
 		if (fd < 0) {
 			fd += 3;
 			while (!isatty(fd))
 				if (--fd < 0)
 					goto out;
-			fd = dup(fd);
-			if (fd < 0)
-				goto out;
 		}
-		fd = savefd(fd);
+		fd = savefd(fd, ofd);
 		do { /* while we are in the background */
 			if ((pgrp = tcgetpgrp(fd)) < 0) {
 out:
diff --git a/src/redir.c b/src/redir.c
index ce34db0..27450fb 100644
--- a/src/redir.c
+++ b/src/redir.c
@@ -145,7 +145,7 @@ redirect(union node *redir, int flags)
 			if (likely(i == EMPTY)) {
 				i = CLOSED;
 				if (fd != newfd) {
-					i = savefd(fd);
+					i = savefd(fd, newfd);
 					fd = -1;
 				}
 			}
@@ -399,7 +399,7 @@ RESET {
  */
 
 int
-savefd(int from)
+savefd(int from, int ofd)
 {
 	int newfd;
 	int err;
@@ -407,7 +407,7 @@ savefd(int from)
 	newfd = fcntl(from, F_DUPFD, 10);
 	err = newfd < 0 ? errno : 0;
 	if (err != EBADF) {
-		close(from);
+		close(ofd);
 		if (err)
 			sh_error("%d: %s", from, strerror(err));
 		else
diff --git a/src/redir.h b/src/redir.h
index a8e6630..d1d160e 100644
--- a/src/redir.h
+++ b/src/redir.h
@@ -45,6 +45,6 @@ union node;
 void redirect(union node *, int);
 void popredir(int);
 void clearredir(void);
-int savefd(int);
+int savefd(int, int);
 int redirectsafe(union node *, int);
 
Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@xxxxxxxxxxxxxxxxxxx>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
To unsubscribe from this list: send the line "unsubscribe dash" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html