ash: ditch dupredirect(), it was only making code harder to read.

 incorporate it in its single callsite.

function                                             old     new   delta
redirect                                            1054    1052      -2
changepath                                           196     194      -2

1 files changed, 24 insertions, 28 deletions

diff --git a/shell/ash.c b/shell/ash.c
index de1f8006d..8f3143669 100644
--- a/shell/ash.c
+++ b/shell/ash.c
@@ -4828,6 +4828,7 @@ openredirect(union node *redir)
 		abort();
 #endif
 		/* Fall through to eliminate warning. */
+/* Our single caller does this itself */
 //	case NTOFD:
 //	case NFROMFD:
 //		f = -1;
@@ -4864,30 +4865,13 @@ copyfd(int from, int to)
 	return newfd;
 }
 
-static void
-dupredirect(union node *redir, int f)
-{
-	int fd = redir->nfile.fd;
-
-	if (f < 0) { /* redir->nfile.type == NTOFD || redir->nfile.type == NFROMFD */
-		if (redir->ndup.dupfd >= 0) {   /* if not ">&-" */
-			copyfd(redir->ndup.dupfd, fd);
-		}
-		return;
-	}
-	if (f != fd) {
-		copyfd(f, fd);
-		close(f);
-	}
-}
-
-/**/
+/* Struct def and variable are moved down to the first usage site */
 struct redirtab {
 	struct redirtab *next;
 	int renamed[10];
 	int nullredirs;
 };
-#define redirlist     (G_var.redirlist    )
+#define redirlist (G_var.redirlist)
 
 /*
  * Process a list of redirection commands.  If the REDIR_PUSH flag is set,
@@ -4918,10 +4902,11 @@ redirect(union node *redir, int flags)
 		sv->next = redirlist;
 		redirlist = sv;
 		sv->nullredirs = g_nullredirs - 1;
+		g_nullredirs = 0;
 		for (i = 0; i < 10; i++)
 			sv->renamed[i] = EMPTY;
-		g_nullredirs = 0;
 	}
+
 	do {
 		fd = redir->nfile.fd;
 		if (redir->nfile.type == NTOFD || redir->nfile.type == NFROMFD) {
@@ -4943,26 +4928,37 @@ redirect(union node *redir, int flags)
 			i = fcntl(fd, F_DUPFD, 10);
 			if (i == -1) {
 				i = errno;
-				if (i != EBADF) { /* strange error */
-					close(newfd);
+				if (i != EBADF) {
+					/* Strange error (e.g. "too many files" EMFILE?) */
+					/*if (newfd >= 0)*/ close(newfd);
 					errno = i;
 					ash_msg_and_raise_error("%d: %m", fd);
 					/* NOTREACHED */
 				}
-				/* it is not open - ok */
+				/* EBADF: it is not open - ok */
 			} else {
-				/* it is open, save its copy */
+				/* fd is open, save its copy */
+//TODO: CLOEXEC the copy? currently these extra "saved" fds are closed
+// in popredir() in the child, preventing them from leaking into child.
+// (popredir() also cleans up the mess in case of failures)
 				sv->renamed[fd] = i;
 				close(fd);
 			}
 		} else {
 			close(fd);
 		}
-		/* Here fd is closed */
-		/* NTOFD/NFROMFD: copy redir->ndup.dupfd to fd */
-		/* else: move newfd to fd */
-		dupredirect(redir, newfd);
+		/* At this point fd is closed */
+		if (newfd < 0) {
+			/* NTOFD/NFROMFD: copy redir->ndup.dupfd to fd */
+			if (redir->ndup.dupfd >= 0) {   /* if not ">&-" */
+				copyfd(redir->ndup.dupfd, fd);
+			}
+		} else { /* move newfd to fd */
+			copyfd(newfd, fd);
+			close(newfd);
+		}
 	} while ((redir = redir->nfile.next) != NULL);
+
 	INT_ON;
 	if ((flags & REDIR_SAVEFD2) && sv && sv->renamed[2] >= 0)
 		preverrout_fd = sv->renamed[2];