* Re: EXIT trap handling in subshells broken [not found] <c1a9c062-0917-d3f3-8826-811b5a4fd802@gigawatt.nl> @ 2020-01-17 8:51 ` Herbert Xu 2020-01-17 9:57 ` [PATCH] redir: Clear saved redirections in subshell Herbert Xu 2020-01-17 10:15 ` EXIT trap handling in subshells broken Herbert Xu 1 sibling, 1 reply; 7+ messages in thread From: Herbert Xu @ 2020-01-17 8:51 UTC (permalink / raw) To: Harald van Dijk; +Cc: DASH shell mailing list On Mon, Jan 06, 2020 at 09:57:20PM +0000, Harald van Dijk wrote: > > One way to fix this is to install an exception handler in evaltree(nr) to > prevent exceptions bubbling up too far. It can just call exitshell() from > there. It is not clear to me yet whether this is the best way. Thanks for the report. I think what we should do is drop the relevant state when we enter the subshell. Cheers, -- 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] redir: Clear saved redirections in subshell 2020-01-17 8:51 ` EXIT trap handling in subshells broken Herbert Xu @ 2020-01-17 9:57 ` Herbert Xu 2020-01-17 15:28 ` [v2 PATCH] " Herbert Xu 0 siblings, 1 reply; 7+ messages in thread From: Herbert Xu @ 2020-01-17 9:57 UTC (permalink / raw) To: Harald van Dijk; +Cc: DASH shell mailing list On Fri, Jan 17, 2020 at 04:51:24PM +0800, Herbert Xu wrote: > On Mon, Jan 06, 2020 at 09:57:20PM +0000, Harald van Dijk wrote: > > > > One way to fix this is to install an exception handler in evaltree(nr) to > > prevent exceptions bubbling up too far. It can just call exitshell() from > > there. It is not clear to me yet whether this is the best way. > > Thanks for the report. I think what we should do is drop the > relevant state when we enter the subshell. Something like this should fix the redir problem. I'll address the other issue in a subsequent patch. ---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. Reported-by: Harald van Dijk <harald@gigawatt.nl> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au> diff --git a/src/jobs.c b/src/jobs.c index 26a6248..69a511c 100644 --- a/src/jobs.c +++ b/src/jobs.c @@ -859,6 +859,7 @@ static void forkchild(struct job *jp, union node *n, int mode) closescript(); clear_traps(); + clearredir(); #if JOBS /* do job control only in root shell */ diff --git a/src/redir.c b/src/redir.c index 6c81dd0..8242b9c 100644 --- a/src/redir.c +++ b/src/redir.c @@ -66,15 +66,7 @@ # define PIPESIZE PIPE_BUF #endif - -MKINIT -struct redirtab { - struct redirtab *next; - int renamed[10]; -}; - - -MKINIT struct redirtab *redirlist; +struct redirtab *redirlist; /* Bit map of currently closed file descriptors. */ static unsigned closed_redirs; diff --git a/src/redir.h b/src/redir.h index 8e56995..82c8ea8 100644 --- a/src/redir.h +++ b/src/redir.h @@ -41,13 +41,22 @@ #endif #define REDIR_SAVEFD2 03 /* set preverrout */ -struct redirtab; +struct redirtab { + struct redirtab *next; + int renamed[10]; +}; + +extern struct redirtab *redirlist; + 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); +static inline void clearredir(void) +{ + redirlist = NULL; +} -- 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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [v2 PATCH] redir: Clear saved redirections in subshell 2020-01-17 9:57 ` [PATCH] redir: Clear saved redirections in subshell Herbert Xu @ 2020-01-17 15:28 ` Herbert Xu 2020-01-17 23:11 ` Harald van Dijk 2020-01-19 10:21 ` [v3 " Herbert Xu 0 siblings, 2 replies; 7+ messages in thread From: Herbert Xu @ 2020-01-17 15:28 UTC (permalink / raw) To: Harald van Dijk; +Cc: DASH shell mailing list 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> 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); -- 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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [v2 PATCH] redir: Clear saved redirections in subshell 2020-01-17 15:28 ` [v2 PATCH] " Herbert Xu @ 2020-01-17 23:11 ` Harald van Dijk 2020-01-19 10:21 ` [v3 " Herbert Xu 1 sibling, 0 replies; 7+ messages in thread From: Harald van Dijk @ 2020-01-17 23:11 UTC (permalink / raw) To: Herbert Xu; +Cc: DASH shell mailing list 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [v3 PATCH] redir: Clear saved redirections in subshell 2020-01-17 15:28 ` [v2 PATCH] " Herbert Xu 2020-01-17 23:11 ` Harald van Dijk @ 2020-01-19 10:21 ` Herbert Xu 1 sibling, 0 replies; 7+ messages in thread From: Herbert Xu @ 2020-01-19 10:21 UTC (permalink / raw) To: Harald van Dijk; +Cc: DASH shell mailing list On Fri, Jan 17, 2020 at 11:28:25PM +0800, Herbert Xu wrote: > > 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. Slight update to remove the reset_handler prototype. ---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> 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/main.h b/src/main.h index 51f1604..19e4983 100644 --- a/src/main.h +++ b/src/main.h @@ -52,4 +52,3 @@ extern int *dash_errno; void readcmdfile(char *); int dotcmd(int, char **); int exitcmd(int, char **); -void reset_handler(void); 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..1cf2761 100644 --- a/src/redir.h +++ b/src/redir.h @@ -45,7 +45,6 @@ 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); 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); -- 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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: EXIT trap handling in subshells broken [not found] <c1a9c062-0917-d3f3-8826-811b5a4fd802@gigawatt.nl> 2020-01-17 8:51 ` EXIT trap handling in subshells broken Herbert Xu @ 2020-01-17 10:15 ` Herbert Xu 2020-01-17 22:58 ` Harald van Dijk 1 sibling, 1 reply; 7+ messages in thread From: Herbert Xu @ 2020-01-17 10:15 UTC (permalink / raw) To: Harald van Dijk; +Cc: DASH shell mailing list On Mon, Jan 06, 2020 at 09:57:20PM +0000, Harald van Dijk wrote: > > The problem is not limited to redirections: > > f() { (trap "echo \$var" EXIT); } > var=bad > var=ok f > > This prints "bad", when it should print "ok". Here, the local variable is > dropped not in main(), but during the unwind process. This works fine for me with the latest git tree. This is because reset has now been split into exitreset and reset and variables are only reset in reset() which is only called if exitshell is not happening. As if we're in a subshell we will never take the reset() path I don't think we need to do anything here at all. Cheers, -- 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: EXIT trap handling in subshells broken 2020-01-17 10:15 ` EXIT trap handling in subshells broken Herbert Xu @ 2020-01-17 22:58 ` Harald van Dijk 0 siblings, 0 replies; 7+ messages in thread From: Harald van Dijk @ 2020-01-17 22:58 UTC (permalink / raw) To: Herbert Xu; +Cc: DASH shell mailing list On 17/01/2020 10:15, Herbert Xu wrote: > On Mon, Jan 06, 2020 at 09:57:20PM +0000, Harald van Dijk wrote: >> >> The problem is not limited to redirections: >> >> f() { (trap "echo \$var" EXIT); } >> var=bad >> var=ok f >> >> This prints "bad", when it should print "ok". Here, the local variable is >> dropped not in main(), but during the unwind process. > > This works fine for me with the latest git tree. This is because > reset has now been split into exitreset and reset and variables > are only reset in reset() which is only called if exitshell is > not happening. > > As if we're in a subshell we will never take the reset() path > I don't think we need to do anything here at all. Ah, right, forgot about that. That change also altered the behaviour of trap "echo foo=\$foo" EXIT f() { local foo=bar exit } f though, which printed foo= in released versions of dash since at least as far back as 0.5.1, and prints foo= in current versions of bash and zsh. It now prints foo=bar. That is defensible and also what some other shells do, but I'm not sure it's intended. (Combined with some other changes I have, the behaviour stops being defensible, but I suspect that is nothing that affects dash.) Cheers, Harald van Dijk ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-01-19 10:22 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <c1a9c062-0917-d3f3-8826-811b5a4fd802@gigawatt.nl>
2020-01-17 8:51 ` EXIT trap handling in subshells broken Herbert Xu
2020-01-17 9:57 ` [PATCH] redir: Clear saved redirections in subshell Herbert Xu
2020-01-17 15:28 ` [v2 PATCH] " Herbert Xu
2020-01-17 23:11 ` Harald van Dijk
2020-01-19 10:21 ` [v3 " Herbert Xu
2020-01-17 10:15 ` EXIT trap handling in subshells broken Herbert Xu
2020-01-17 22:58 ` Harald van Dijk
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).