From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 24743 invoked by alias); 16 Feb 2016 12:46:27 -0000 Mailing-List: contact zsh-workers-help@zsh.org; run by ezmlm Precedence: bulk X-No-Archive: yes List-Id: Zsh Workers List List-Post: List-Help: X-Seq: 37999 Received: (qmail 19708 invoked from network); 16 Feb 2016 12:46:25 -0000 X-Spam-Checker-Version: SpamAssassin 3.4.1 (2015-04-28) on f.primenet.com.au X-Spam-Level: X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00 autolearn=ham autolearn_force=no version=3.4.1 X-AuditID: cbfec7f5-f79b16d000005389-f6-56c31a1cf1a7 Date: Tue, 16 Feb 2016 12:46:17 +0000 From: Peter Stephenson To: zsh-workers@zsh.org Subject: Re: [BUG] Sticky-sh POSIX_TRAPS are function-local Message-id: <20160216124617.7b75746a@pwslap01u.europe.root.pri> In-reply-to: <20160216095744.52cb8389@pwslap01u.europe.root.pri> References: <56C15DF1.8080405@inlv.org> <20160216095744.52cb8389@pwslap01u.europe.root.pri> Organization: Samsung Cambridge Solution Centre X-Mailer: Claws Mail 3.7.9 (GTK+ 2.22.0; i386-redhat-linux-gnu) MIME-version: 1.0 Content-type: text/plain; charset=US-ASCII Content-transfer-encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFjrPLMWRmVeSWpSXmKPExsVy+t/xK7oyUofDDG4uN7A42PyQyYHRY9XB D0wBjFFcNimpOZllqUX6dglcGTPuXmUsmGtYsbPrBEsD4wHVLkZODgkBE4lj0xexQthiEhfu rWfrYuTiEBJYyigx73sbE4Qzg0li5Y4pzCBVQgLnGCXOTlCHSJxllDh2/z5QFQcHi4CqxI4v ziA1bAKGElM3zWYEsUUExCXOrj3PAmILC1hLnNmygAnE5hWwl/i8aiWYzSngIPHr/BEWiPmx Eiv3zwaz+QX0Ja7+/cQEcZ29xMwrZxghegUlfky+B1bDLKAlsXlbEyuELS+xec1bqDvVJW7c 3c0+gVF4FpKWWUhaZiFpWcDIvIpRNLU0uaA4KT3XSK84Mbe4NC9dLzk/dxMjJJi/7mBceszq EKMAB6MSD+8G10NhQqyJZcWVuYcYJTiYlUR4/70CCvGmJFZWpRblxxeV5qQWH2KU5mBREued uet9iJBAemJJanZqakFqEUyWiYNTqoHRwy/E68hq+9bVP1rYxM5NWrsi0SWGv+HrjMOmTXnb gttMO+vnn7XkVlm6V9yZPf9tn1eG1WOxWCXFdWmyE9cxHGXivmxs/GSJy4vuB5Ktwm7KZ08v lrnQr34168xqR+W8658YPrR5SjY86RJi9o462T2J94nxg8u/mFaebFnIs/ZT57SJJrVKLMUZ iYZazEXFiQDfxiNeYgIAAA== This makes the POSIX behaviour of the EXIT trap sticky based on the setting of POSIX_TRAPS at the point where the EXIT trap was set. It's hard to see how this can not be what the script writer intended. In other words, it doesn't care about the stickiness of the emulator surrounding it. I've pointed out knock-on effects in the README; I suspect anyone relying on those side effects was on a hiding to nothing anyway. diff --git a/README b/README index 8b8ab57..d5343db 100644 --- a/README +++ b/README @@ -65,6 +65,20 @@ remainder of the value discarded. This could lead to different behaviour if the argument contains non-numeric characters, or if the argument has leading zeroes and the OCTAL_ZEROES option is set. +3) For some time the shell has had a POSIX_TRAPS option which determines +whether the EXIT trap has POSIX behaviour (the trap is only run at shell +exit) or traditional zsh behaviour (the trap is run once and discarded +when the enclosing fuction or shell exits, whichever happens first). +The use of this option has now been made "sticky" on the EXIT trap --- +in other words, the setting of the option at the point where the trap is +set now determines whether the trap has POSIX or traditional zsh +behaviour. This means that changing the option after the trap was set +no longer has any effect. + +Other aspects of EXIT trap handling have not changed --- there is still +only one EXIT trap at any point in a programme, so it is not generally +useful to combine POSIX and non-POSIX behaviour in the same script. + Incompatibilities between 5.0.8 and 5.2 --------------------------------------- diff --git a/Src/signals.c b/Src/signals.c index aa0b5aa..32452ae 100644 --- a/Src/signals.c +++ b/Src/signals.c @@ -55,6 +55,15 @@ mod_export Eprog siglists[VSIGCOUNT]; /**/ mod_export int nsigtrapped; +/* + * Flag that exit trap has been set in POSIX mode. + * The setter's expectation is therefore that it is run + * on programme exit, not function exit. + */ + +/**/ +static int exit_trap_posix; + /* Variables used by signal queueing */ /**/ @@ -755,7 +764,7 @@ killjb(Job jn, int sig) * at once, so just use a linked list. */ struct savetrap { - int sig, flags, local; + int sig, flags, local, posix; void *list; }; @@ -774,6 +783,7 @@ dosavetrap(int sig, int level) st = (struct savetrap *)zalloc(sizeof(*st)); st->sig = sig; st->local = level; + st->posix = (sig == SIGEXIT) ? exit_trap_posix : 0; if ((st->flags = sigtrapped[sig]) & ZSIG_FUNC) { /* * Get the old function: this assumes we haven't added @@ -873,6 +883,10 @@ settrap(int sig, Eprog l, int flags) * works just the same. */ sigtrapped[sig] |= (locallevel << ZSIG_SHIFT) | flags; + if (sig == SIGEXIT) { + /* Make POSIX behaviour of EXIT trap sticky */ + exit_trap_posix = isset(POSIXTRAPS); + } unqueue_signals(); return 0; } @@ -908,7 +922,7 @@ removetrap(int sig) * already one at the current locallevel we just overwrite it. */ if (!dontsavetrap && - (sig == SIGEXIT ? !isset(POSIXTRAPS) : isset(LOCALTRAPS)) && + (sig == SIGEXIT ? !exit_trap_posix : isset(LOCALTRAPS)) && locallevel && (!trapped || locallevel > (sigtrapped[sig] >> ZSIG_SHIFT))) dosavetrap(sig, locallevel); @@ -935,6 +949,8 @@ removetrap(int sig) #endif sig != SIGCHLD) signal_default(sig); + if (sig == SIGEXIT) + exit_trap_posix = 0; /* * At this point we free the appropriate structs. If we don't @@ -978,7 +994,7 @@ starttrapscope(void) * so give it the next higher one. dosavetrap() is called * automatically where necessary. */ - if (sigtrapped[SIGEXIT] && !isset(POSIXTRAPS)) { + if (sigtrapped[SIGEXIT] && !exit_trap_posix) { locallevel++; unsettrap(SIGEXIT); locallevel--; @@ -1005,7 +1021,7 @@ endtrapscope(void) * Don't do this inside another trap. */ if (!intrap && - !isset(POSIXTRAPS) && (exittr = sigtrapped[SIGEXIT])) { + !exit_trap_posix && (exittr = sigtrapped[SIGEXIT])) { if (exittr & ZSIG_FUNC) { exitfn = removehashnode(shfunctab, "TRAPEXIT"); } else { @@ -1031,7 +1047,9 @@ endtrapscope(void) if (st->flags & ZSIG_FUNC) settrap(sig, NULL, ZSIG_FUNC); else - settrap(sig, (Eprog) st->list, 0); + settrap(sig, (Eprog) st->list, 0); + if (sig == SIGEXIT) + exit_trap_posix = st->posix; dontsavetrap--; /* * counting of nsigtrapped should presumably be handled @@ -1042,16 +1060,26 @@ endtrapscope(void) if ((sigtrapped[sig] = st->flags) & ZSIG_FUNC) shfunctab->addnode(shfunctab, ((Shfunc)st->list)->node.nam, (Shfunc) st->list); - } else if (sigtrapped[sig]) - unsettrap(sig); + } else if (sigtrapped[sig]) { + /* + * Don't restore the old state if someone has set a + * POSIX-style exit trap --- allow this to propagate. + */ + if (sig != SIGEXIT || !exit_trap_posix) + unsettrap(sig); + } zfree(st, sizeof(*st)); } } if (exittr) { - if (!isset(POSIXTRAPS)) - dotrapargs(SIGEXIT, &exittr, exitfn); + /* + * We already made sure this wasn't set as a POSIX exit trap. + * We respect the user's intention when the trap in question + * was set. + */ + dotrapargs(SIGEXIT, &exittr, exitfn); if (exittr & ZSIG_FUNC) shfunctab->freenode((HashNode)exitfn); else diff --git a/Test/C03traps.ztst b/Test/C03traps.ztst index 4b2843a..d8183a4 100644 --- a/Test/C03traps.ztst +++ b/Test/C03traps.ztst @@ -399,6 +399,26 @@ >} >No, really exited + (cd ..; $ZTST_exe -fc 'unsetopt posixtraps; + echo start program + emulate sh -c '\''testfn() { + echo start function + set -o | grep posixtraps + trap "echo EXIT TRAP TRIGGERED" EXIT + echo end function + }'\'' + testfn + echo program continuing + echo end of program') +0:POSIX_TRAPS effect on EXIT trap is sticky +>start program +>start function +>noposixtraps off +>end function +>program continuing +>end of program +>EXIT TRAP TRIGGERED + (set -e printf "a\nb\n" | while read line do