Re: bug: ZERR traps and functions.

[Peter Stephenson <p.w.stephenson@ntlworld.com>]

On Wed, 10 Jan 2007 17:41:13 -0500
"Gene Carter" <gananda@gmail.com> wrote:
> I believe I've found a bug in how ZSH handles functions and error trapping.
> 
> with zsh 4.3.2 on AIX 4.3.3...
> 
> If you set error trapping and a function returns non-zero, the shell exits,
> but the code specified in the trap doesn't get executed.

Right, this occurs with an explicit "return" returning non-zero from a
function nested within the code where ERR_EXIT is set, not with an
instruction that merely causes an implicit return because of its exit
status.

Then I noticed that without ERR_EXIT set the rules for returning the
status from a return within the trap weren't being obeyed, either.
Specifically, the special case of a non-zero return status from TRAPZERR
should trigger a return from an enclosing function with the same status,
but while it did trigger a return, the status was lost.  (See
description of TRAPNAL in "Special Functions" in zshmisc.)  The other
cases (trap '...' ZERR, or returning 0 from TRAPZERR) worked OK.

I hate traps.

Index: Src/signals.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/signals.c,v
retrieving revision 1.42
diff -u -r1.42 signals.c
--- Src/signals.c	19 Dec 2006 10:35:55 -0000	1.42
+++ Src/signals.c	12 Jan 2007 22:56:34 -0000
@@ -1071,6 +1071,7 @@
     char *name, num[4];
     int trapret = 0;
     int obreaks = breaks;
+    int oretflag = retflag;
     int isfunc;
     int traperr;
 
@@ -1109,7 +1110,7 @@
 
     lexsave();
     execsave();
-    breaks = 0;
+    breaks = retflag = 0;
     runhookdef(BEFORETRAPHOOK, NULL);
     if (*sigtr & ZSIG_FUNC) {
 	int osc = sfcontext;
@@ -1176,15 +1177,20 @@
 	if (isfunc) {
 	    breaks = loops;
 	    errflag = 1;
+	    lastval = trapret;
 	} else {
 	    lastval = trapret-1;
 	}
+	/* return triggered */
+	retflag = 1;
     } else {
 	if (traperr && emulation != EMULATE_SH)
 	    lastval = 1;
 	if (try_tryflag)
 	    errflag = traperr;
 	breaks += obreaks;
+	/* return not triggered: restore old flag */
+	retflag = oretflag;
 	if (breaks > loops)
 	    breaks = loops;
     }
Index: Test/C03traps.ztst
===================================================================
RCS file: /cvsroot/zsh/zsh/Test/C03traps.ztst,v
retrieving revision 1.9
diff -u -r1.9 C03traps.ztst
--- Test/C03traps.ztst	3 Mar 2006 22:09:58 -0000	1.9
+++ Test/C03traps.ztst	12 Jan 2007 22:56:34 -0000
@@ -287,6 +287,69 @@
 >child exiting
 >wait #2 finished, gotsig=0, status=33
 
+  fn1() {
+    setopt errexit
+    trap 'echo error1' ZERR
+    false
+    print Shouldn\'t get here 1a
+  }
+  fn2() {
+    setopt errexit
+    trap 'echo error2' ZERR
+    return 1
+    print Shouldn\'t get here 2a
+  }
+  fn3() {
+    setopt errexit
+    TRAPZERR() { echo error3; }
+    false
+    print Shouldn\'t get here 3a
+  }
+  fn4() {
+    setopt errexit
+    TRAPZERR() { echo error4; }
+    return 1
+    print Shouldn\'t get here 4a
+  }
+  (fn1; print Shouldn\'t get here 1b)
+  (fn2; print Shouldn\'t get here 2b)
+  (fn3; print Shouldn\'t get here 3b)
+  (fn4; print Shouldn\'t get here 4b)
+1: Combination of ERR_EXIT and ZERR trap
+>error1
+>error2
+>error3
+>error4
+
+   fn1() { TRAPZERR() { print trap; return 42; }; false; print Broken; }
+   (fn1)
+   print Working $?
+0: Force return of containing function from TRAPZERR.
+>trap
+>Working 42
+
+   fn2() { trap 'print trap; return 42' ZERR; false; print Broken }
+   (fn2)
+   print Working $?
+0: Return with non-zero status triggered from within trap '...' ZERR.
+>trap
+>Working 42
+
+   fn3() { TRAPZERR() { print trap; return 0; }; false; print OK this time; }
+   (fn3)
+   print Working $?
+0: Normal return from TRAPZERR.
+>trap
+>OK this time
+>Working 0
+
+   fn4() { trap 'print trap; return 0' ZERR; false; print Broken; }
+   (fn4)
+   print Working $?
+0: Return with zero status triggered from within trap '...' ZERR.
+>trap
+>Working 0
+
 %clean
 
   rm -f TRAPEXIT

-- 
Peter Stephenson <p.w.stephenson@ntlworld.com>
Web page now at http://homepage.ntlworld.com/p.w.stephenson/