about summary refs log tree commit diff
diff options
context:
space:
mode:
authorPeter Stephenson <pws@users.sourceforge.net>2008-08-07 16:25:14 +0000
committerPeter Stephenson <pws@users.sourceforge.net>2008-08-07 16:25:14 +0000
commit2706eac45492b0fcdfc3cf104ac947e65d09ee25 (patch)
treeff7f6e2a05231a8444589e54c95313114ebcae07
parentc4f33330f6797d0c199abf522f79c3ccac88ed55 (diff)
downloadzsh-2706eac45492b0fcdfc3cf104ac947e65d09ee25.tar.gz
zsh-2706eac45492b0fcdfc3cf104ac947e65d09ee25.tar.xz
zsh-2706eac45492b0fcdfc3cf104ac947e65d09ee25.zip
25415: Make DEBUG_BEFORE_CMD the default.
Reuse ERR_EXIT in DEBUG traps.
Clean up trapreturn code.
-rw-r--r--ChangeLog10
-rw-r--r--Doc/Zsh/builtins.yo9
-rw-r--r--Doc/Zsh/func.yo8
-rw-r--r--Doc/Zsh/options.yo9
-rw-r--r--README6
-rw-r--r--Src/builtin.c6
-rw-r--r--Src/exec.c58
-rw-r--r--Src/init.c18
-rw-r--r--Src/options.c2
-rw-r--r--Src/signals.c44
-rw-r--r--Src/zsh.h22
-rw-r--r--Test/A05execution.ztst2
-rw-r--r--Test/C03traps.ztst13
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  <pws@csr.com>
+
+	* 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  <pws@csr.com>
 
 	* 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 <D>)
 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