All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Herbert Xu <herbert@gondor.apana.org.au>
To: Jilles Tjoelker <jilles@stack.nl>
Cc: dash@vger.kernel.org
Subject: [v3 PATCH] trap: Preserve parent traps for trap-only command substitution
Date: Sat, 20 Apr 2024 08:36:04 +0800	[thread overview]
Message-ID: <ZiMN9FzMvg+IhjNJ@gondor.apana.org.au> (raw)
In-Reply-To: <20240419214425.GB24704@stack.nl>

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

      reply	other threads:[~2024-04-20  0:35 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-14  7:52 [PATCH] trap: Preserve parent shell traps for simple commands Herbert Xu
2024-04-15 10:17 ` [v2 PATCH] trap: Preserve parent traps for trap-only command substitution Herbert Xu
2024-04-19 21:44   ` Jilles Tjoelker
2024-04-20  0:36     ` Herbert Xu [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZiMN9FzMvg+IhjNJ@gondor.apana.org.au \
    --to=herbert@gondor.apana.org.au \
    --cc=dash@vger.kernel.org \
    --cc=jilles@stack.nl \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.