[PATCH] trap: Preserve parent shell traps for simple commands

[PATCH] trap: Preserve parent shell traps for simple commands

[Herbert Xu]

Traps are reset when subshell is started.  When a subshell is
started for a simple command, preserve the parent trap text so
that they can be printed.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---
 src/eval.c   |  4 ++--
 src/init.h   |  2 +-
 src/jobs.c   |  2 +-
 src/mkinit.c |  5 +++--
 src/trap.c   | 51 +++++++++++++++++++++++++++++++++++++--------------
 5 files changed, 44 insertions(+), 20 deletions(-)

diff --git a/src/eval.c b/src/eval.c
index fce5314..a0f4df2 100644
--- a/src/eval.c
+++ b/src/eval.c
@@ -491,11 +491,11 @@ evalsubshell(union node *n, int flags)
 	expredir(n->nredir.redirect);
 	INTOFF;
 	if (!backgnd && flags & EV_EXIT && !have_traps()) {
-		forkreset();
+		forkreset(0);
 		goto nofork;
 	}
 	jp = makejob(1);
-	if (forkshell(jp, n, backgnd) == 0) {
+	if (forkshell(jp, n->nredir.n, backgnd) == 0) {
 		flags |= EV_EXIT;
 		if (backgnd)
 			flags &=~ EV_TESTED;
diff --git a/src/init.h b/src/init.h
index d56fb28..705ffd7 100644
--- a/src/init.h
+++ b/src/init.h
@@ -36,5 +36,5 @@
 
 void init(void);
 void exitreset(void);
-void forkreset(void);
+void forkreset(int);
 void reset(void);
diff --git a/src/jobs.c b/src/jobs.c
index 2a2fe22..0abdcaa 100644
--- a/src/jobs.c
+++ b/src/jobs.c
@@ -872,7 +872,7 @@ static void forkchild(struct job *jp, union node *n, int mode)
 	if (!lvforked) {
 		shlvl++;
 
-		forkreset();
+		forkreset(n && n->type == NCMD);
 
 #if JOBS
 		/* do job control only in root shell */
diff --git a/src/mkinit.c b/src/mkinit.c
index 9025862..f07e288 100644
--- a/src/mkinit.c
+++ b/src/mkinit.c
@@ -91,6 +91,7 @@ struct event {
 	char *name;		/* name of event (e.g. INIT) */
 	char *routine;		/* name of routine called on event */
 	char *comment;		/* comment describing routine */
+	char *args;		/* arguments to routine */
 	struct text code;	/* code for handling event */
 };
 
@@ -128,7 +129,7 @@ char reset[] = "\
 struct event event[] = {
 	{"INIT", "init", init},
 	{"EXITRESET", "exitreset", exitreset},
-	{"FORKRESET", "forkreset", forkreset},
+	{"FORKRESET", "forkreset", forkreset, "int simplecmd"},
 	{"RESET", "reset", reset},
 	{NULL, NULL}
 };
@@ -388,7 +389,7 @@ output(void)
 	for (ep = event ; ep->name ; ep++) {
 		fputs("\n\n\n", fp);
 		fputs(ep->comment, fp);
-		fprintf(fp, "\nvoid\n%s() {\n", ep->routine);
+		fprintf(fp, "\nvoid\n%s(%s) {\n", ep->routine, ep->args ?: "");
 		writetext(&ep->code, fp);
 		fprintf(fp, "}\n");
 	}
diff --git a/src/trap.c b/src/trap.c
index cd84814..4587144 100644
--- a/src/trap.c
+++ b/src/trap.c
@@ -66,7 +66,9 @@
 
 
 /* trap handler commands */
-MKINIT char *trap[NSIG];
+static char *trap[NSIG];
+/* traps have not been fully cleared */
+static int ptrap;
 /* number of non-null traps */
 int trapcnt;
 /* current value of signal */
@@ -81,6 +83,7 @@ volatile sig_atomic_t gotsigchld;
 extern char *signal_names[];
 
 static int decode_signum(const char *);
+MKINIT void clear_traps(int);
 
 #ifdef mkinit
 INCLUDE "memalloc.h"
@@ -92,19 +95,7 @@ INIT {
 }
 
 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;
+	clear_traps(simplecmd);
 }
 #endif
 
@@ -133,6 +124,8 @@ trapcmd(int argc, char **argv)
 		}
 		return 0;
 	}
+	if (ptrap)
+		clear_traps(0);
 	if (!ap[1] || decode_signum(*ap) >= 0)
 		action = NULL;
 	else
@@ -168,6 +161,36 @@ trapcmd(int argc, char **argv)
 
 
 
+/*
+ * Clear traps on a fork.
+ */
+
+void clear_traps(int simplecmd)
+{
+	char **tp;
+
+	INTOFF;
+	for (tp = trap ; tp < &trap[NSIG] ; tp++) {
+		if (*tp && **tp) {	/* trap not NULL or SIG_IGN */
+			char *otp = *tp;
+
+			*tp = NULL;
+			if (tp != &trap[0])
+				setsignal(tp - trap);
+
+			if (simplecmd)
+				*tp = otp;
+			else
+				ckfree(*tp);
+		}
+	}
+	trapcnt = 0;
+	ptrap = simplecmd;
+	INTON;
+}
+
+
+
 /*
  * Set the signal handler for the specified signal.  The routine figures
  * out what it should be set to.
-- 
2.39.2

-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[v2 PATCH] trap: Preserve parent traps for trap-only command substitution

[Herbert Xu]

v2 restricts the preservation to command substitution only and
only if the command's first word is "trap".

---8<---
Traps are reset when a subshell is started.  When a subshell is
started for command substitution with a simple command whose first
word is "trap", preserve the parent trap text so that they can be
printed.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---
 src/eval.c   |  4 ++--
 src/init.h   |  4 +++-
 src/jobs.c   |  2 +-
 src/mkinit.c |  5 +++--
 src/trap.c   | 55 +++++++++++++++++++++++++++++++++++++++-------------
 5 files changed, 50 insertions(+), 20 deletions(-)

diff --git a/src/eval.c b/src/eval.c
index fce5314..d169eb8 100644
--- a/src/eval.c
+++ b/src/eval.c
@@ -491,11 +491,11 @@ evalsubshell(union node *n, int flags)
 	expredir(n->nredir.redirect);
 	INTOFF;
 	if (!backgnd && flags & EV_EXIT && !have_traps()) {
-		forkreset();
+		forkreset(NULL);
 		goto nofork;
 	}
 	jp = makejob(1);
-	if (forkshell(jp, n, backgnd) == 0) {
+	if (forkshell(jp, n->nredir.n, backgnd) == 0) {
 		flags |= EV_EXIT;
 		if (backgnd)
 			flags &=~ EV_TESTED;
diff --git a/src/init.h b/src/init.h
index d56fb28..4f98b5d 100644
--- a/src/init.h
+++ b/src/init.h
@@ -34,7 +34,9 @@
  *	@(#)init.h	8.2 (Berkeley) 5/4/95
  */
 
+union node;
+
 void init(void);
 void exitreset(void);
-void forkreset(void);
+void forkreset(union node *);
 void reset(void);
diff --git a/src/jobs.c b/src/jobs.c
index 2a2fe22..dce8e22 100644
--- a/src/jobs.c
+++ b/src/jobs.c
@@ -872,7 +872,7 @@ static void forkchild(struct job *jp, union node *n, int mode)
 	if (!lvforked) {
 		shlvl++;
 
-		forkreset();
+		forkreset(mode == FORK_NOJOB ? n : NULL);
 
 #if JOBS
 		/* do job control only in root shell */
diff --git a/src/mkinit.c b/src/mkinit.c
index 9025862..870b64d 100644
--- a/src/mkinit.c
+++ b/src/mkinit.c
@@ -91,6 +91,7 @@ struct event {
 	char *name;		/* name of event (e.g. INIT) */
 	char *routine;		/* name of routine called on event */
 	char *comment;		/* comment describing routine */
+	char *args;		/* arguments to routine */
 	struct text code;	/* code for handling event */
 };
 
@@ -128,7 +129,7 @@ char reset[] = "\
 struct event event[] = {
 	{"INIT", "init", init},
 	{"EXITRESET", "exitreset", exitreset},
-	{"FORKRESET", "forkreset", forkreset},
+	{"FORKRESET", "forkreset", forkreset, "union node *n"},
 	{"RESET", "reset", reset},
 	{NULL, NULL}
 };
@@ -388,7 +389,7 @@ output(void)
 	for (ep = event ; ep->name ; ep++) {
 		fputs("\n\n\n", fp);
 		fputs(ep->comment, fp);
-		fprintf(fp, "\nvoid\n%s() {\n", ep->routine);
+		fprintf(fp, "\nvoid\n%s(%s) {\n", ep->routine, ep->args ?: "");
 		writetext(&ep->code, fp);
 		fprintf(fp, "}\n");
 	}
diff --git a/src/trap.c b/src/trap.c
index cd84814..525a009 100644
--- a/src/trap.c
+++ b/src/trap.c
@@ -66,7 +66,9 @@
 
 
 /* trap handler commands */
-MKINIT char *trap[NSIG];
+static char *trap[NSIG];
+/* traps have not been fully cleared */
+static int ptrap;
 /* number of non-null traps */
 int trapcnt;
 /* current value of signal */
@@ -81,6 +83,7 @@ volatile sig_atomic_t gotsigchld;
 extern char *signal_names[];
 
 static int decode_signum(const char *);
+MKINIT void clear_traps(union node *);
 
 #ifdef mkinit
 INCLUDE "memalloc.h"
@@ -92,19 +95,7 @@ INIT {
 }
 
 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;
+	clear_traps(n);
 }
 #endif
 
@@ -133,6 +124,8 @@ trapcmd(int argc, char **argv)
 		}
 		return 0;
 	}
+	if (ptrap)
+		clear_traps(NULL);
 	if (!ap[1] || decode_signum(*ap) >= 0)
 		action = NULL;
 	else
@@ -168,6 +161,40 @@ trapcmd(int argc, char **argv)
 
 
 
+/*
+ * Clear traps on a fork.
+ */
+
+void clear_traps(union node *n)
+{
+	int simplecmd;
+	char **tp;
+
+	simplecmd = n && n->type == NCMD && n->ncmd.args &&
+		    equal(n->ncmd.args->narg.text, "trap");
+
+	INTOFF;
+	for (tp = trap ; tp < &trap[NSIG] ; tp++) {
+		if (*tp && **tp) {	/* trap not NULL or SIG_IGN */
+			char *otp = *tp;
+
+			*tp = NULL;
+			if (tp != &trap[0])
+				setsignal(tp - trap);
+
+			if (simplecmd)
+				*tp = otp;
+			else
+				ckfree(*tp);
+		}
+	}
+	trapcnt = 0;
+	ptrap = simplecmd;
+	INTON;
+}
+
+
+
 /*
  * Set the signal handler for the specified signal.  The routine figures
  * out what it should be set to.
-- 
2.39.2

-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [v2 PATCH] trap: Preserve parent traps for trap-only command substitution

[Jilles Tjoelker]

On Mon, Apr 15, 2024 at 06:17:36PM +0800, Herbert Xu wrote:
> v2 restricts the preservation to command substitution only and
> only if the command's first word is "trap".

> ---8<---
> Traps are reset when a subshell is started.  When a subshell is
> started for command substitution with a simple command whose first
> word is "trap", preserve the parent trap text so that they can be
> printed.

It looks like this will execute the EXIT trap in a subshell $(trap).

On the other hand, things like $(trap "echo subshell exit >&2" EXIT) and
$(trap "echo other trap >&2" USR1) seem to be handled correctly.

I think the intent of https://austingroupbugs.net/view.php?id=53 is that
things like $('trap') and $(\trap) should also work (although an
expansion that expands to trap need not work).

-- 
Jilles Tjoelker

[v3 PATCH] trap: Preserve parent traps for trap-only command substitution

[Herbert Xu]

On Fri, Apr 19, 2024 at 11:44:25PM +0200, Jilles Tjoelker wrote:
>
> It looks like this will execute the EXIT trap in a subshell $(trap).

Good catch!

> I think the intent of https://austingroupbugs.net/view.php?id=53 is that
> things like $('trap') and $(\trap) should also work (although an
> expansion that expands to trap need not work).

I don't agree with that.  Unless the wording becomes much more
explicit, I'm not going to support something silly like $('trap').

Thanks,

---8<---
Traps are reset when a subshell is started.  When a subshell is
started for command substitution with a simple command whose first
word is "trap", preserve the parent trap text so that they can be
printed.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---
 src/eval.c   |  4 ++--
 src/init.h   |  4 +++-
 src/jobs.c   |  2 +-
 src/mkinit.c |  5 +++--
 src/trap.c   | 57 +++++++++++++++++++++++++++++++++++++++-------------
 5 files changed, 52 insertions(+), 20 deletions(-)

diff --git a/src/eval.c b/src/eval.c
index fce5314..d169eb8 100644
--- a/src/eval.c
+++ b/src/eval.c
@@ -491,11 +491,11 @@ evalsubshell(union node *n, int flags)
 	expredir(n->nredir.redirect);
 	INTOFF;
 	if (!backgnd && flags & EV_EXIT && !have_traps()) {
-		forkreset();
+		forkreset(NULL);
 		goto nofork;
 	}
 	jp = makejob(1);
-	if (forkshell(jp, n, backgnd) == 0) {
+	if (forkshell(jp, n->nredir.n, backgnd) == 0) {
 		flags |= EV_EXIT;
 		if (backgnd)
 			flags &=~ EV_TESTED;
diff --git a/src/init.h b/src/init.h
index d56fb28..4f98b5d 100644
--- a/src/init.h
+++ b/src/init.h
@@ -34,7 +34,9 @@
  *	@(#)init.h	8.2 (Berkeley) 5/4/95
  */
 
+union node;
+
 void init(void);
 void exitreset(void);
-void forkreset(void);
+void forkreset(union node *);
 void reset(void);
diff --git a/src/jobs.c b/src/jobs.c
index 2a2fe22..dce8e22 100644
--- a/src/jobs.c
+++ b/src/jobs.c
@@ -872,7 +872,7 @@ static void forkchild(struct job *jp, union node *n, int mode)
 	if (!lvforked) {
 		shlvl++;
 
-		forkreset();
+		forkreset(mode == FORK_NOJOB ? n : NULL);
 
 #if JOBS
 		/* do job control only in root shell */
diff --git a/src/mkinit.c b/src/mkinit.c
index 9025862..870b64d 100644
--- a/src/mkinit.c
+++ b/src/mkinit.c
@@ -91,6 +91,7 @@ struct event {
 	char *name;		/* name of event (e.g. INIT) */
 	char *routine;		/* name of routine called on event */
 	char *comment;		/* comment describing routine */
+	char *args;		/* arguments to routine */
 	struct text code;	/* code for handling event */
 };
 
@@ -128,7 +129,7 @@ char reset[] = "\
 struct event event[] = {
 	{"INIT", "init", init},
 	{"EXITRESET", "exitreset", exitreset},
-	{"FORKRESET", "forkreset", forkreset},
+	{"FORKRESET", "forkreset", forkreset, "union node *n"},
 	{"RESET", "reset", reset},
 	{NULL, NULL}
 };
@@ -388,7 +389,7 @@ output(void)
 	for (ep = event ; ep->name ; ep++) {
 		fputs("\n\n\n", fp);
 		fputs(ep->comment, fp);
-		fprintf(fp, "\nvoid\n%s() {\n", ep->routine);
+		fprintf(fp, "\nvoid\n%s(%s) {\n", ep->routine, ep->args ?: "");
 		writetext(&ep->code, fp);
 		fprintf(fp, "}\n");
 	}
diff --git a/src/trap.c b/src/trap.c
index cd84814..75501d7 100644
--- a/src/trap.c
+++ b/src/trap.c
@@ -66,7 +66,9 @@
 
 
 /* trap handler commands */
-MKINIT char *trap[NSIG];
+static char *trap[NSIG];
+/* traps have not been fully cleared */
+static int ptrap;
 /* number of non-null traps */
 int trapcnt;
 /* current value of signal */
@@ -81,6 +83,7 @@ volatile sig_atomic_t gotsigchld;
 extern char *signal_names[];
 
 static int decode_signum(const char *);
+MKINIT void clear_traps(union node *);
 
 #ifdef mkinit
 INCLUDE "memalloc.h"
@@ -92,19 +95,7 @@ INIT {
 }
 
 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;
+	clear_traps(n);
 }
 #endif
 
@@ -133,6 +124,8 @@ trapcmd(int argc, char **argv)
 		}
 		return 0;
 	}
+	if (ptrap)
+		clear_traps(NULL);
 	if (!ap[1] || decode_signum(*ap) >= 0)
 		action = NULL;
 	else
@@ -168,6 +161,40 @@ trapcmd(int argc, char **argv)
 
 
 
+/*
+ * Clear traps on a fork.
+ */
+
+void clear_traps(union node *n)
+{
+	int simplecmd;
+	char **tp;
+
+	simplecmd = n && n->type == NCMD && n->ncmd.args &&
+		    equal(n->ncmd.args->narg.text, "trap");
+
+	INTOFF;
+	for (tp = trap ; tp < &trap[NSIG] ; tp++) {
+		if (*tp && **tp) {	/* trap not NULL or SIG_IGN */
+			char *otp = *tp;
+
+			*tp = NULL;
+			if (tp != &trap[0])
+				setsignal(tp - trap);
+
+			if (simplecmd)
+				*tp = otp;
+			else
+				ckfree(*tp);
+		}
+	}
+	trapcnt = 0;
+	ptrap = simplecmd;
+	INTON;
+}
+
+
+
 /*
  * Set the signal handler for the specified signal.  The routine figures
  * out what it should be set to.
@@ -390,6 +417,8 @@ exitshell(void)
 	handler = &loc;
 	if ((p = trap[0])) {
 		trap[0] = NULL;
+		if (ptrap)
+			goto out;
 		evalskip = 0;
 		evalstring(p, 0);
 		evalskip = SKIPFUNCDEF;
-- 
2.39.2

-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt