diff mbox series

[v2] redir: Clear saved redirections in subshell

Message ID 20200117152825.ll4kge4ehy36xhxw@gondor.apana.org.au (mailing list archive)
State Superseded
Delegated to: Herbert Xu
Headers show
Series [v2] redir: Clear saved redirections in subshell | expand

Commit Message

Herbert Xu Jan. 17, 2020, 3:28 p.m. UTC 
On Fri, Jan 17, 2020 at 05:57:55PM +0800, Herbert Xu wrote:
> 
> Something like this should fix the redir problem.  I'll address the
> other issue in a subsequent patch.

Here is a new version that adds a forkreset hook to mkinit so
that we do the clearing even when we enter a subshell without
forking.

---8<---
When we enter a subshell we need to drop the saved redirections
as otherwise a subsequent unwindredir could produce incorrect
results.

This patch does this by simply clearing redirlist.  While we
could actually free the memory underneath for subshells it isn't
really worth the trouble for now.

In order to ensure that this is done in every place where we enter
a subshell, this patch adds a new mkinit hook called forkreset.
The calls closescript, clear_traps and reset_handler are also added
to the forkreset hook.

This fixes a bug where the first two functions weren't called
if we enter a subshell without forking.

Reported-by: Harald van Dijk <harald@gigawatt.nl>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

Comments

Harald van Dijk Jan. 17, 2020, 11:11 p.m. UTC | #1 
On 17/01/2020 15:28, Herbert Xu wrote:
> On Fri, Jan 17, 2020 at 05:57:55PM +0800, Herbert Xu wrote:
>>
>> Something like this should fix the redir problem.  I'll address the
>> other issue in a subsequent patch.
> 
> Here is a new version that adds a forkreset hook to mkinit so
> that we do the clearing even when we enter a subshell without
> forking.

Yup, that's something I've had for a while for the same reason. I called 
it envreset since it's not only called after forking, but it's the same 
thing.

The clearing of redirlist is also what I ended up doing, except I did 
free the memory by adding a drop parameter to unwindredir and passing 
that on to popredir. That should not affect the user-visible behaviour.

Cheers,
Harald van Dijk
diff mbox series

Patch

diff --git a/src/eval.c b/src/eval.c
index 6ee2e1a..1b5d61d 100644
--- a/src/eval.c
+++ b/src/eval.c
@@ -41,6 +41,7 @@ 
  * Evaluate a command.
  */
 
+#include "init.h"
 #include "main.h"
 #include "shell.h"
 #include "nodes.h"
@@ -483,17 +484,18 @@  evalsubshell(union node *n, int flags)
 		lineno -= funcline - 1;
 
 	expredir(n->nredir.redirect);
-	if (!backgnd && flags & EV_EXIT && !have_traps())
-		goto nofork;
 	INTOFF;
+	if (!backgnd && flags & EV_EXIT && !have_traps()) {
+		forkreset();
+		goto nofork;
+	}
 	jp = makejob(n, 1);
 	if (forkshell(jp, n, backgnd) == 0) {
-		INTON;
 		flags |= EV_EXIT;
 		if (backgnd)
 			flags &=~ EV_TESTED;
 nofork:
-		reset_handler();
+		INTON;
 		redirect(n->nredir.redirect, 0);
 		evaltreenr(n->nredir.n, flags);
 		/* never returns */
@@ -576,7 +578,6 @@  evalpipe(union node *n, int flags)
 			}
 		}
 		if (forkshell(jp, lp->n, n->npipe.backgnd) == 0) {
-			reset_handler();
 			INTON;
 			if (pip[1] >= 0) {
 				close(pip[0]);
@@ -633,7 +634,6 @@  evalbackcmd(union node *n, struct backcmd *result)
 		sh_error("Pipe call failed");
 	jp = makejob(n, 1);
 	if (forkshell(jp, n, FORK_NOJOB) == 0) {
-		reset_handler();
 		FORCEINTON;
 		close(pip[0]);
 		if (pip[1] != 1) {
diff --git a/src/init.h b/src/init.h
index 49791a0..d56fb28 100644
--- a/src/init.h
+++ b/src/init.h
@@ -36,4 +36,5 @@ 
 
 void init(void);
 void exitreset(void);
+void forkreset(void);
 void reset(void);
diff --git a/src/input.c b/src/input.c
index ae0c4c8..177fd0a 100644
--- a/src/input.c
+++ b/src/input.c
@@ -78,6 +78,7 @@  static int preadbuffer(void);
 
 #ifdef mkinit
 INCLUDE <stdio.h>
+INCLUDE <unistd.h>
 INCLUDE "input.h"
 INCLUDE "error.h"
 
@@ -91,6 +92,14 @@  RESET {
 	basepf.lleft = basepf.nleft = 0;
 	popallfiles();
 }
+
+FORKRESET {
+	popallfiles();
+	if (parsefile->fd > 0) {
+		close(parsefile->fd);
+		parsefile->fd = 0;
+	}
+}
 #endif
 
 
@@ -495,20 +504,3 @@  popallfiles(void)
 {
 	unwindfiles(&basepf);
 }
-
-
-
-/*
- * Close the file(s) that the shell is reading commands from.  Called
- * after a fork is done.
- */
-
-void
-closescript(void)
-{
-	popallfiles();
-	if (parsefile->fd > 0) {
-		close(parsefile->fd);
-		parsefile->fd = 0;
-	}
-}
diff --git a/src/input.h b/src/input.h
index a9c0517..8acc6e9 100644
--- a/src/input.h
+++ b/src/input.h
@@ -99,4 +99,3 @@  void setinputstring(char *);
 void popfile(void);
 void unwindfiles(struct parsefile *);
 void popallfiles(void);
-void closescript(void);
diff --git a/src/jobs.c b/src/jobs.c
index 26a6248..f377d8c 100644
--- a/src/jobs.c
+++ b/src/jobs.c
@@ -55,6 +55,7 @@ 
 #endif
 #include "exec.h"
 #include "eval.h"
+#include "init.h"
 #include "redir.h"
 #include "show.h"
 #include "main.h"
@@ -857,8 +858,7 @@  static void forkchild(struct job *jp, union node *n, int mode)
 	if (!lvforked) {
 		shlvl++;
 
-		closescript();
-		clear_traps();
+		forkreset();
 
 #if JOBS
 		/* do job control only in root shell */
diff --git a/src/main.c b/src/main.c
index b2712cb..36431fc 100644
--- a/src/main.c
+++ b/src/main.c
@@ -71,7 +71,7 @@  int *dash_errno;
 short profile_buf[16384];
 extern int etext();
 #endif
-static struct jmploc main_handler;
+MKINIT struct jmploc main_handler;
 
 STATIC void read_profile(const char *);
 STATIC char *find_dot_file(char *);
@@ -354,7 +354,10 @@  exitcmd(int argc, char **argv)
 	/* NOTREACHED */
 }
 
-void reset_handler(void)
-{
+#ifdef mkinit
+INCLUDE "error.h"
+
+FORKRESET {
 	handler = &main_handler;
 }
+#endif
diff --git a/src/memalloc.h b/src/memalloc.h
index b9c63da..b9adf76 100644
--- a/src/memalloc.h
+++ b/src/memalloc.h
@@ -35,6 +35,7 @@ 
  */
 
 #include <stddef.h>
+#include <stdlib.h>
 
 struct stackmark {
 	struct stack_block *stackp;
diff --git a/src/mkinit.c b/src/mkinit.c
index 5bca9ee..9025862 100644
--- a/src/mkinit.c
+++ b/src/mkinit.c
@@ -113,6 +113,11 @@  char exitreset[] = "\
  * but prior to exitshell. \n\
  */\n";
 
+char forkreset[] = "\
+/*\n\
+ * This routine is called when we enter a subshell.\n\
+ */\n";
+
 char reset[] = "\
 /*\n\
  * This routine is called when an error or an interrupt occurs in an\n\
@@ -123,6 +128,7 @@  char reset[] = "\
 struct event event[] = {
 	{"INIT", "init", init},
 	{"EXITRESET", "exitreset", exitreset},
+	{"FORKRESET", "forkreset", forkreset},
 	{"RESET", "reset", reset},
 	{NULL, NULL}
 };
diff --git a/src/redir.c b/src/redir.c
index 6c81dd0..895140c 100644
--- a/src/redir.c
+++ b/src/redir.c
@@ -401,6 +401,10 @@  EXITRESET {
 	unwindredir(0);
 }
 
+FORKRESET {
+	redirlist = NULL;
+}
+
 #endif
 
 
diff --git a/src/redir.h b/src/redir.h
index 8e56995..8bad4cb 100644
--- a/src/redir.h
+++ b/src/redir.h
@@ -45,9 +45,7 @@  struct redirtab;
 union node;
 void redirect(union node *, int);
 void popredir(int);
-void clearredir(void);
 int savefd(int, int);
 int redirectsafe(union node *, int);
 void unwindredir(struct redirtab *stop);
 struct redirtab *pushredir(union node *redir);
-
diff --git a/src/trap.c b/src/trap.c
index 58a7c60..82e7ece 100644
--- a/src/trap.c
+++ b/src/trap.c
@@ -66,7 +66,7 @@ 
 
 
 /* trap handler commands */
-static char *trap[NSIG];
+MKINIT char *trap[NSIG];
 /* number of non-null traps */
 int trapcnt;
 /* current value of signal */
@@ -83,11 +83,29 @@  extern char *signal_names[];
 static int decode_signum(const char *);
 
 #ifdef mkinit
+INCLUDE "memalloc.h"
 INCLUDE "trap.h"
+
 INIT {
 	sigmode[SIGCHLD - 1] = S_DFL;
 	setsignal(SIGCHLD);
 }
+
+FORKRESET {
+	char **tp;
+
+	INTOFF;
+	for (tp = trap ; tp < &trap[NSIG] ; tp++) {
+		if (*tp && **tp) {	/* trap not NULL or SIG_IGN */
+			ckfree(*tp);
+			*tp = NULL;
+			if (tp != &trap[0])
+				setsignal(tp - trap);
+		}
+	}
+	trapcnt = 0;
+	INTON;
+}
 #endif
 
 /*
@@ -151,30 +169,6 @@  trapcmd(int argc, char **argv)
 
 
 /*
- * Clear traps on a fork.
- */
-
-void
-clear_traps(void)
-{
-	char **tp;
-
-	INTOFF;
-	for (tp = trap ; tp < &trap[NSIG] ; tp++) {
-		if (*tp && **tp) {	/* trap not NULL or SIG_IGN */
-			ckfree(*tp);
-			*tp = NULL;
-			if (tp != &trap[0])
-				setsignal(tp - trap);
-		}
-	}
-	trapcnt = 0;
-	INTON;
-}
-
-
-
-/*
  * Set the signal handler for the specified signal.  The routine figures
  * out what it should be set to.
  */
diff --git a/src/trap.h b/src/trap.h
index 5fd65af..4c455a8 100644
--- a/src/trap.h
+++ b/src/trap.h
@@ -42,7 +42,6 @@  extern volatile sig_atomic_t pending_sig;
 extern int gotsigchld;
 
 int trapcmd(int, char **);
-void clear_traps(void);
 void setsignal(int);
 void ignoresig(int);
 void onsig(int);