From 2706eac45492b0fcdfc3cf104ac947e65d09ee25 Mon Sep 17 00:00:00 2001 From: Peter Stephenson Date: Thu, 7 Aug 2008 16:25:14 +0000 Subject: 25415: Make DEBUG_BEFORE_CMD the default. Reuse ERR_EXIT in DEBUG traps. Clean up trapreturn code. --- ChangeLog | 10 +++++++++ Doc/Zsh/builtins.yo | 9 +++++++- Doc/Zsh/func.yo | 8 +++++-- Doc/Zsh/options.yo | 9 +++++++- README | 6 ++++++ Src/builtin.c | 6 ++++-- Src/exec.c | 58 ++++++++++++++++++++++++++++++++++++++++---------- Src/init.c | 18 +++++++++++----- Src/options.c | 2 +- Src/signals.c | 44 +++++++++++++------------------------- Src/zsh.h | 22 ++++++++++++++++++- Test/A05execution.ztst | 2 ++ Test/C03traps.ztst | 13 +++++++++++ 13 files changed, 154 insertions(+), 53 deletions(-) diff --git a/ChangeLog b/ChangeLog index 71929d01f..44c6f64c3 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,13 @@ +2008-08-07 Peter Stephenson + + * 25415: README, Doc/Zsh/builtins.yo, Doc/Zsh/func.yo, + Doc/Zsh/options.yo, Src/builtin.c, Src/exec.c, Src/init.c, + Src/options.c, Src/signals.c, Src/zsh.h, Test/A05execution.ztst, + Test/C03traps.ztst: Make DEBUG_BEFORE_CMD the default; + make ERR_EXIT ineffective in DEBUG traps but allow it to + be set to skip the next command (actually sublist); tidy + up code associated with trapreturn. + 2008-08-06 Peter Stephenson * 25405: Src/exec.c: return value was not set from anonymous diff --git a/Doc/Zsh/builtins.yo b/Doc/Zsh/builtins.yo index bcd03be98..b14bc58fb 100644 --- a/Doc/Zsh/builtins.yo +++ b/Doc/Zsh/builtins.yo @@ -1304,8 +1304,15 @@ If var(sig) is tt(ZERR) then var(arg) will be executed after each command with a nonzero exit status. tt(ERR) is an alias for tt(ZERR) on systems that have no tt(SIGERR) signal (this is the usual case). + If var(sig) is tt(DEBUG) then var(arg) will be executed -after each command. +before each command if the option tt(DEBUG_BEFORE_CMD) is set +(as it is by default), else after each command. In the former +case it is possible to skip the next command; see +the description of the tt(ERR_EXIT) option in +ifzman(zmanref(zshoptions))\ +ifnzman(noderef(Description of Options)). + If var(sig) is tt(0) or tt(EXIT) and the tt(trap) statement is executed inside the body of a function, then the command var(arg) is executed after the function completes. diff --git a/Doc/Zsh/func.yo b/Doc/Zsh/func.yo index 83ab8bc5d..36b135275 100644 --- a/Doc/Zsh/func.yo +++ b/Doc/Zsh/func.yo @@ -308,8 +308,12 @@ executed inside other traps. ) findex(TRAPDEBUG) item(tt(TRAPDEBUG))( -Executed after each command. If the option tt(DEBUG_BEFORE_CMD) -is set, executed before each command instead. +If the option tt(DEBUG_BEFORE_CMD) is set (as it is by default), executed +before each command; otherwise executed after each command. In the former +case it is possible to skip the next command; see the description of the +tt(ERR_EXIT) option in +ifzman(zmanref(zshoptions))\ +ifnzman(noderef(Description of Options)). ) findex(TRAPEXIT) item(tt(TRAPEXIT))( diff --git a/Doc/Zsh/options.yo b/Doc/Zsh/options.yo index eec951d4f..a0d1f9c0d 100644 --- a/Doc/Zsh/options.yo +++ b/Doc/Zsh/options.yo @@ -1046,7 +1046,7 @@ ifnzman(Arithmetic Evaluation)\ ifzman(the section ARITHMETIC EVALUATION in zmanref(zshmisc)) has an explicit list. ) -pindex(DEBUG_BEFORE_CMD) +pindex(DEBUG_BEFORE_CMD ) cindex(traps, DEBUG, before or after command) cindex(DEBUG trap, before or after command) item(tt(DEBUG_BEFORE_CMD))( @@ -1060,6 +1060,13 @@ item(tt(ERR_EXIT) (tt(-e), ksh: tt(-e)))( If a command has a non-zero exit status, execute the tt(ZERR) trap, if set, and exit. This is disabled while running initialization scripts. + +The behaviour is also disabled inside tt(DEBUG) traps. In this +case the option is handled specially: it is unset on entry to +the trap. If the option tt(DEBUG_BEFORE_CMD) is set, +as it is by default, and the option tt(ERR_EXIT) is found to have been set +on exit, then the command for which the tt(DEBUG) trap is being executed is +skipped. The option is restored after the trap exits. ) pindex(ERR_RETURN) cindex(function return, on error) diff --git a/README b/README index 5375eb3a5..20627a9ca 100644 --- a/README +++ b/README @@ -56,6 +56,12 @@ behaviour.) Now it is treated identically to "$@". The same change applies to expressions with forced splitting such as ${=1+"$@"}, but otherwise the case where SH_WORD_SPLIT is not set is unaffected. +Debug traps (`trap ... DEBUG' or the function TRAPDEBUG) now run by default +before the command to which they refer instead of after. This is almost +always the right behaviour for the intended purpose of debugging and is +consistent with recent versions of other shells. The option +DEBUG_BEFORE_CMD can be unset to revert to the previous behaviour. + In previous versions of the shell it was possible to use index 0 in an array or string subscript to refer to the same element as index 1 if the option KSH_ARRAYS was not in effect. This was a limited approximation to diff --git a/Src/builtin.c b/Src/builtin.c index e1378d901..ee44b3776 100644 --- a/Src/builtin.c +++ b/Src/builtin.c @@ -4462,8 +4462,10 @@ bin_break(char *name, char **argv, UNUSED(Options ops), int func) retflag = 1; breaks = loops; lastval = num; - if (trapreturn == -2) - trapreturn = lastval; + if (trap_state == TRAP_STATE_PRIMED && trap_return == -2) { + trap_state = TRAP_STATE_FORCE_RETURN; + trap_return = lastval; + } return lastval; } zexit(num, 0); /* else treat return as logout/exit */ diff --git a/Src/exec.c b/Src/exec.c index 5b360815f..6ca5b2f2b 100644 --- a/Src/exec.c +++ b/Src/exec.c @@ -65,16 +65,23 @@ int nohistsave; mod_export int errflag; /* - * Status of return from a trap. + * State of trap return value. Value is from enum trap_state. + */ + +/**/ +int trap_state; + +/* + * Value associated with return from a trap. * This is only active if we are inside a trap, else its value * is irrelevant. It is initialised to -1 for a function trap and * -2 for a non-function trap and if negative is decremented as * we go deeper into functions and incremented as we come back up. * The value is used to decide if an explicit "return" should cause * a return from the caller of the trap; it does this by setting - * trapreturn to a status (i.e. a non-negative value). + * trap_return to a status (i.e. a non-negative value). * - * In summary, trapreturn is + * In summary, trap_return is * - zero unless we are in a trap * - negative in a trap unless it has triggered. Code uses this * to detect an active trap. @@ -83,7 +90,7 @@ mod_export int errflag; */ /**/ -int trapreturn; +int trap_return; /* != 0 if this is a subshell */ @@ -1061,6 +1068,10 @@ execlist(Estate state, int dont_change_job, int exiting) } if (sigtrapped[SIGDEBUG] && isset(DEBUGBEFORECMD)) { + int oerrexit_opt = opts[ERREXIT]; + opts[ERREXIT] = 0; + noerrexit = 1; + exiting = donetrap; ret = lastval; dotrap(SIGDEBUG); @@ -1071,7 +1082,8 @@ execlist(Estate state, int dont_change_job, int exiting) * Only execute the trap once per sublist, even * if the DEBUGBEFORECMD option changes. */ - donedebug = 1; + donedebug = isset(ERREXIT) ? 2 : 1; + opts[ERREXIT] = oerrexit_opt; } else donedebug = 0; @@ -1087,6 +1099,18 @@ execlist(Estate state, int dont_change_job, int exiting) /* Loop through code followed by &&, ||, or end of sublist. */ code = *state->pc++; + if (donedebug == 2) { + /* Skip sublist. */ + while (wc_code(code) == WC_SUBLIST) { + state->pc = state->pc + WC_SUBLIST_SKIP(code); + if (WC_SUBLIST_TYPE(code) == WC_SUBLIST_END) + break; + code = *state->pc++; + } + donetrap = 1; + /* yucky but consistent... */ + goto sublist_done; + } while (wc_code(code) == WC_SUBLIST) { next = state->pc + WC_SUBLIST_SKIP(code); if (!oldnoerrexit) @@ -1177,12 +1201,20 @@ sublist_done: noerrexit = oldnoerrexit; if (sigtrapped[SIGDEBUG] && !isset(DEBUGBEFORECMD) && !donedebug) { + /* + * Save and restore ERREXIT for consistency with + * DEBUGBEFORECMD, even though it's not used. + */ + int oerrexit_opt = opts[ERREXIT]; + opts[ERREXIT] = 0; + noerrexit = 1; exiting = donetrap; ret = lastval; dotrap(SIGDEBUG); lastval = ret; donetrap = exiting; noerrexit = oldnoerrexit; + opts[ERREXIT] = oerrexit_opt; } cmdsp = csp; @@ -4145,8 +4177,8 @@ doshfunc(char *name, Eprog prog, LinkList doshargs, int flags, int noreturnval) oargv0 = NULL; obreaks = breaks;; - if (trapreturn < 0) - trapreturn--; + if (trap_state == TRAP_STATE_PRIMED) + trap_return--; oldlastval = lastval; oldnumpipestats = numpipestats; if (noreturnval) { @@ -4264,8 +4296,8 @@ doshfunc(char *name, Eprog prog, LinkList doshargs, int flags, int noreturnval) endtrapscope(); - if (trapreturn < -1) - trapreturn++; + if (trap_state == TRAP_STATE_PRIMED) + trap_return++; ret = lastval; if (noreturnval) { lastval = oldlastval; @@ -4528,7 +4560,9 @@ execsave(void) es->badcshglob = badcshglob; es->cmdoutpid = cmdoutpid; es->cmdoutval = cmdoutval; - es->trapreturn = trapreturn; + es->trap_return = trap_return; + es->trap_state = trap_state; + es->trapisfunc = trapisfunc; es->noerrs = noerrs; es->subsh_close = subsh_close; es->underscore = ztrdup(underscore); @@ -4555,7 +4589,9 @@ execrestore(void) badcshglob = exstack->badcshglob; cmdoutpid = exstack->cmdoutpid; cmdoutval = exstack->cmdoutval; - trapreturn = exstack->trapreturn; + trap_return = exstack->trap_return; + trap_state = exstack->trap_state; + trapisfunc = exstack->trapisfunc; noerrs = exstack->noerrs; subsh_close = exstack->subsh_close; setunderscore(exstack->underscore); diff --git a/Src/init.c b/Src/init.c index f2dc99af7..d8a0dbc57 100644 --- a/Src/init.c +++ b/Src/init.c @@ -191,10 +191,6 @@ loop(int toplevel, int justonce) exit(lastval); if (((!interact || sourcelevel) && errflag) || retflag) break; - if (intrap && trapreturn >= 0) { - lastval = trapreturn; - trapreturn = 0; - } if (isset(SINGLECOMMAND) && toplevel) { if (sigtrapped[SIGEXIT]) dotrap(SIGEXIT); @@ -880,7 +876,8 @@ setupvals(void) lastmailcheck = time(NULL); locallevel = sourcelevel = 0; sfcontext = SFC_NONE; - trapreturn = 0; + trap_return = 0; + trap_state = TRAP_STATE_INACTIVE; noerrexit = -1; nohistsave = 1; dirstack = znewlinklist(); @@ -1060,6 +1057,7 @@ source(char *s) char *old_scriptname = scriptname, *us; unsigned char *ocs; int ocsp; + int otrap_return = trap_return, otrap_state = trap_state; if (!s || (!(prog = try_source_file((us = unmeta(s)))) && @@ -1090,6 +1088,13 @@ source(char *s) dosetopt(SHINSTDIN, 0, 1); scriptname = s; + /* + * The special return behaviour of traps shouldn't + * trigger in files sourced from traps; the return + * is just a return from the file. + */ + trap_state = TRAP_STATE_INACTIVE; + sourcelevel++; if (prog) { pushheap(); @@ -1100,6 +1105,9 @@ source(char *s) loop(0, 0); /* loop through the file to be sourced */ sourcelevel--; + trap_state = otrap_state; + trap_return = otrap_return; + /* restore the current shell state */ if (prog) freeeprog(prog); diff --git a/Src/options.c b/Src/options.c index bb99ba1e5..d6aa3c850 100644 --- a/Src/options.c +++ b/Src/options.c @@ -112,7 +112,7 @@ static struct optname optns[] = { {{NULL, "cshjunkiequotes", OPT_EMULATE|OPT_CSH}, CSHJUNKIEQUOTES}, {{NULL, "cshnullcmd", OPT_EMULATE|OPT_CSH}, CSHNULLCMD}, {{NULL, "cshnullglob", OPT_EMULATE|OPT_CSH}, CSHNULLGLOB}, -{{NULL, "debugbeforecmd", OPT_EMULATE}, DEBUGBEFORECMD}, +{{NULL, "debugbeforecmd", OPT_ALL}, DEBUGBEFORECMD}, {{NULL, "emacs", 0}, EMACSMODE}, {{NULL, "equals", OPT_EMULATE|OPT_ZSH}, EQUALS}, {{NULL, "errexit", OPT_EMULATE}, ERREXIT}, diff --git a/Src/signals.c b/Src/signals.c index 61fff64a2..d978c3dec 100644 --- a/Src/signals.c +++ b/Src/signals.c @@ -1082,12 +1082,10 @@ dotrapargs(int sig, int *sigtr, void *sigfn) { LinkList args; char *name, num[4]; - int trapret = 0; int obreaks = breaks; int oretflag = retflag; - int otrapreturn = trapreturn; int isfunc; - int traperr; + int traperr, new_trap_state, new_trap_return; /* if signal is being ignored or the trap function * * is NULL, then return * @@ -1123,6 +1121,7 @@ dotrapargs(int sig, int *sigtr, void *sigfn) *sigtr |= ZSIG_IGNORED; lexsave(); + /* execsave will save the old trap_return and trap_state */ execsave(); breaks = retflag = 0; runhookdef(BEFORETRAPHOOK, NULL); @@ -1148,7 +1147,8 @@ dotrapargs(int sig, int *sigtr, void *sigfn) sprintf(num, "%d", sig); zaddlinknode(args, num); - trapreturn = -1; /* incremented by doshfunc */ + trap_return = -1; /* incremented by doshfunc */ + trap_state = TRAP_STATE_PRIMED; trapisfunc = isfunc = 1; sfcontext = SFC_SIGNAL; @@ -1156,46 +1156,32 @@ dotrapargs(int sig, int *sigtr, void *sigfn) sfcontext = osc; freelinklist(args, (FreeFunc) NULL); zsfree(name); - } else { - trapreturn = -2; /* not incremented, used at current level */ + trap_return = -2; /* not incremented, used at current level */ + trap_state = TRAP_STATE_PRIMED; trapisfunc = isfunc = 0; execode(sigfn, 1, 0); } runhookdef(AFTERTRAPHOOK, NULL); - if (trapreturn > 0 && isfunc) { - /* - * Context was its own function. We propagate the return - * value specially. Return value zero means don't do - * anything special, so don't handle it. - */ - trapret = trapreturn; - } else if (trapreturn >= 0 && !isfunc) { - /* - * Context was an inline trap. If an explicit return value - * was used, we need to set `lastval'. Otherwise we use the - * value restored by execrestore. In this case, all return - * values indicate an explicit return from the current function, - * so always handle specially. trapreturn is always restored by - * execrestore. - */ - trapret = trapreturn + 1; - } traperr = errflag; - trapreturn = otrapreturn; + + /* Grab values before they are restored */ + new_trap_state = trap_state; + new_trap_return = trap_return; + execrestore(); lexrestore(); - if (trapret > 0) { + if (new_trap_state == TRAP_STATE_FORCE_RETURN && + /* zero return from function isn't special */ + !(isfunc && new_trap_return == 0)) { if (isfunc) { breaks = loops; errflag = 1; - lastval = trapret; - } else { - lastval = trapret-1; } + lastval = new_trap_return; /* return triggered */ retflag = 1; } else { diff --git a/Src/zsh.h b/Src/zsh.h index d245a416a..49c08c7ac 100644 --- a/Src/zsh.h +++ b/Src/zsh.h @@ -921,7 +921,9 @@ struct execstack { int badcshglob; pid_t cmdoutpid; int cmdoutval; - int trapreturn; + int trap_return; + int trap_state; + int trapisfunc; int noerrs; int subsh_close; char *underscore; @@ -2225,6 +2227,24 @@ struct heap { #define ZSIG_ALIAS (1<<3) /* Trap is stored under an alias */ #define ZSIG_SHIFT 4 +/* + * State of traps, stored in trap_state. + */ +enum trap_state { + /* Traps are not active; trap_return is not useful. */ + TRAP_STATE_INACTIVE, + /* + * Traps are set but haven't triggered; trap_return gives + * minus function depth. + */ + TRAP_STATE_PRIMED, + /* + * Trap has triggered to force a return; trap_return givens + * return value. + */ + TRAP_STATE_FORCE_RETURN +}; + /***********/ /* Sorting */ /***********/ diff --git a/Test/A05execution.ztst b/Test/A05execution.ztst index e731d1109..8dee85c28 100644 --- a/Test/A05execution.ztst +++ b/Test/A05execution.ztst @@ -124,6 +124,7 @@ 0:TRAPEXIT >Exit + unsetopt DEBUG_BEFORE_CMD unfunction fn print 'TRAPDEBUG() { print Line $LINENO @@ -138,6 +139,7 @@ >Line 1 >Line 1 + unsetopt DEBUG_BEFORE_CMD unfunction fn print 'trap '\''print Line $LINENO'\'' DEBUG : diff --git a/Test/C03traps.ztst b/Test/C03traps.ztst index 79a30d773..b663e296f 100644 --- a/Test/C03traps.ztst +++ b/Test/C03traps.ztst @@ -402,6 +402,19 @@ 0: trapreturn handling bug is properly fixed ?./zsh-trapreturn-bug2:5: no such file or directory: ./fdasfsdafd + fn() { + setopt localtraps localoptions debugbeforecmd + trap '(( LINENO == 4 )) && setopt errexit' DEBUG + print $LINENO three + print $LINENO four + print $LINENO five + [[ -o errexit ]] && print "Hey, ERREXIT is set!" + } + fn +1:Skip line from DEBUG trap +>3 three +>5 five + %clean rm -f TRAPEXIT -- cgit 1.4.1