Re: PATCH: skip command from debug trap

["Rocky Bernstein" <rocky.bernstein@xxxxxxxxx>]

First and foremost, thanks for the change.

When I first read this I was eager to try it out. So I rushed to apply
the patch and put in a "skip" command. When this didn't work I then
reread - there is more unfinished work?

I'm not fussy about the number used. I don't understand why 255 is any
more special than say 4366 as a return from the TRAP instruction, but
255 is okay.

And if one  wants to use a negative number I don't see why that can't
be accommodated for either. For example if -1 is used, then -2 could
mean skip one return level and -3 two levels. Okay, maybe not such a
good idea if this is to be useful and easy for someone to remember.
However if the number where a high negative number, it probably can't
get confused with a return level because you will probably run out of
stack first. Recent ksh barfs on more than 2048 return levels (RELEASE
file 08-06-17 entry).

As for using PS4 to call a trace function, sure it's possible to do
things this way, but I not see what the merit is. Especially when
there is a more straightforward and conventional way to do things,
i.e. a command call such as trap.


On Tue, Aug 5, 2008 at 10:27 AM, Peter Stephenson <pws@xxxxxxx> wrote:
> I believe this is the fairly simple code needed to skip a command from a
> debug trap.
>
> The only difficult bit is the way of triggering it, since it's long
> established that any non-zero number returned from a trap causes the
> enclosing function to return.  I was inclined to pick a negative number,
> but Rocky wants that for something else.  So how about 255, which is
> fairly special?
>
> If we're going to do this kind of stuff there seems little point in
> trying to retain the old default behaviour of debug traps, where it
> won't work.
>
> Index: README
> ===================================================================
> RCS file: /cvsroot/zsh/zsh/README,v
> retrieving revision 1.53
> diff -u -r1.53 README
> --- README      1 Jun 2008 18:35:50 -0000       1.53
> +++ README      5 Aug 2008 14:17:32 -0000
> @@ -56,6 +56,12 @@
>  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
> Index: Doc/Zsh/builtins.yo
> ===================================================================
> RCS file: /cvsroot/zsh/zsh/Doc/Zsh/builtins.yo,v
> retrieving revision 1.108
> diff -u -r1.108 builtins.yo
> --- Doc/Zsh/builtins.yo 10 Jun 2008 08:50:36 -0000      1.108
> +++ Doc/Zsh/builtins.yo 5 Aug 2008 14:17:33 -0000
> @@ -1301,8 +1301,13 @@
>  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,
> +executing a tt(return) with the value of 255 within the trap causes
> +the command referred to to be skipped.
> +
>  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.
> Index: Doc/Zsh/func.yo
> ===================================================================
> RCS file: /cvsroot/zsh/zsh/Doc/Zsh/func.yo,v
> retrieving revision 1.20
> diff -u -r1.20 func.yo
> --- Doc/Zsh/func.yo     30 Jul 2008 19:46:20 -0000      1.20
> +++ Doc/Zsh/func.yo     5 Aug 2008 14:17:33 -0000
> @@ -308,8 +308,10 @@
>  )
>  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, returning the value 255 from the trap function causes
> +execution of the command to be skipped.
>  )
>  findex(TRAPEXIT)
>  item(tt(TRAPEXIT))(
> Index: Doc/Zsh/options.yo
> ===================================================================
> RCS file: /cvsroot/zsh/zsh/Doc/Zsh/options.yo,v
> retrieving revision 1.61
> diff -u -r1.61 options.yo
> --- Doc/Zsh/options.yo  12 Jun 2008 13:45:05 -0000      1.61
> +++ Doc/Zsh/options.yo  5 Aug 2008 14:17:33 -0000
> @@ -1046,7 +1046,7 @@
>  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))(
> Index: Src/exec.c
> ===================================================================
> RCS file: /cvsroot/zsh/zsh/Src/exec.c,v
> retrieving revision 1.136
> diff -u -r1.136 exec.c
> --- Src/exec.c  1 Aug 2008 13:53:44 -0000       1.136
> +++ Src/exec.c  5 Aug 2008 14:17:33 -0000
> @@ -1061,6 +1061,9 @@
>        }
>
>        if (sigtrapped[SIGDEBUG] && isset(DEBUGBEFORECMD)) {
> +           int otrapskip = trapskip;
> +           trapskip = 0;
> +
>            exiting = donetrap;
>            ret = lastval;
>            dotrap(SIGDEBUG);
> @@ -1071,7 +1074,8 @@
>             * Only execute the trap once per sublist, even
>             * if the DEBUGBEFORECMD option changes.
>             */
> -           donedebug = 1;
> +           donedebug = trapskip ? 2 : 1;
> +           trapskip = otrapskip;
>        } else
>            donedebug = 0;
>
> @@ -1087,6 +1091,16 @@
>
>        /* 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);
> +               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)
> Index: Src/options.c
> ===================================================================
> RCS file: /cvsroot/zsh/zsh/Src/options.c,v
> retrieving revision 1.43
> diff -u -r1.43 options.c
> --- Src/options.c       31 Jul 2008 08:44:21 -0000      1.43
> +++ Src/options.c       5 Aug 2008 14:17:33 -0000
> @@ -112,7 +112,7 @@
>  {{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},
> Index: Src/signals.c
> ===================================================================
> RCS file: /cvsroot/zsh/zsh/Src/signals.c,v
> retrieving revision 1.48
> diff -u -r1.48 signals.c
> --- Src/signals.c       1 Aug 2008 13:53:45 -0000       1.48
> +++ Src/signals.c       5 Aug 2008 14:17:34 -0000
> @@ -1076,6 +1076,11 @@
>  /**/
>  int trapisfunc;
>
> +/* Signal to list code that we should skip the next statement. */
> +
> +/**/
> +int trapskip;
> +
>  /**/
>  void
>  dotrapargs(int sig, int *sigtr, void *sigfn)
> @@ -1188,7 +1193,9 @@
>     execrestore();
>     lexrestore();
>
> -    if (trapret > 0) {
> +    trapskip = (trapret == 255 && sig == SIGDEBUG);
> +
> +    if (trapret > 0 && !trapskip) {
>        if (isfunc) {
>            breaks = loops;
>            errflag = 1;
>
>
>
> --
> Peter Stephenson <pws@xxxxxxx>                  Software Engineer
> CSR PLC, Churchill House, Cambridge Business Park, Cowley Road
> Cambridge, CB4 0WZ, UK                          Tel: +44 (0)1223 692070
>