Re: set -e bug

[Peter Stephenson <p.stephenson@xxxxxxxxxxx>]

On Thu, 13 Dec 2012 00:54:23 +0100
Vincent Lefevre <vincent@xxxxxxxxxx> wrote:
> I think I've found a "set -e" bug, which I can reproduce in zsh 4.3.17
> (Debian/unstable) and zsh 5.0.0 (Ubuntu). Consider the following zsh
> script:
> 
> ----------------------------------------
> #!/usr/bin/env zsh
> 
> set -e
> echo $ZSH_VERSION
> printf "a\nb\n" | while read line
> do
>   [[ $line = a* ]] || continue
>   #echo "$line"
>   ((ctr++))
>   [[ $line = foo ]]
> done
> echo "ctr = $ctr"
> ----------------------------------------
> 
> I get:
> 
> 4.3.17
> ctr = 1
> 
> with an exit status 0, even though [[ $line = foo ]] is false.

This is still there (it's not the same as the problem that we "exited"
on errors only to the top level of a script).

The logic to do with donetrap and noerrexit in execlist() has me stumped
at that moment --- it's not clear what the combined effect is supposed
to be, we seem to reset things before we use them, and some effects are
propagating up from nested calls --- but I'm reasonably confident that
resetting donetrap earlier is the right thing to do: it's done for all
normal executions of commands in a list, it should also be done when we
optimise the list to a single simple command, which is logically
equivalent to an end of list.

Indeed, this picks up a bug in another test where the wrong name of a
variable was used (and the variable must be set somewhere else, though I
didn't confirm this).  Somehow the return status of the test wasn't
being propagated so we never noticed.

Index: Src/exec.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/exec.c,v
retrieving revision 1.214
diff -p -u -r1.214 exec.c
--- Src/exec.c	11 Oct 2012 20:14:03 -0000	1.214
+++ Src/exec.c	13 Dec 2012 10:14:06 -0000
@@ -1207,6 +1207,9 @@ execlist(Estate state, int dont_change_j
 	} else
 	    donedebug = intrap ? 1 : 0;
 
+	/* Reset donetrap:  this ensures that a trap is only *
+	 * called once for each sublist that fails.          */
+	donetrap = 0;
 	if (ltype & Z_SIMPLE) {
 	    next = state->pc + WC_LIST_SKIP(code);
 	    if (donedebug != 2)
@@ -1214,9 +1217,6 @@ execlist(Estate state, int dont_change_j
 	    state->pc = next;
 	    goto sublist_done;
 	}
-	/* Reset donetrap:  this ensures that a trap is only *
-	 * called once for each sublist that fails.          */
-	donetrap = 0;
 
 	/* Loop through code followed by &&, ||, or end of sublist. */
 	code = *state->pc++;
Index: Test/A06assign.ztst
===================================================================
RCS file: /cvsroot/zsh/zsh/Test/A06assign.ztst,v
retrieving revision 1.8
diff -p -u -r1.8 A06assign.ztst
--- Test/A06assign.ztst	21 Dec 2010 16:54:31 -0000	1.8
+++ Test/A06assign.ztst	13 Dec 2012 10:14:06 -0000
@@ -184,8 +184,8 @@
 
  typeset -A hash
  hash=(one 1)
- h+=string
- [[ $h[@] == string ]]
+ hash+=string
+ [[ $hash[@] == string ]]
 0:add scalar to association
 
 # tests of var+=(array)
Index: Test/C03traps.ztst
===================================================================
RCS file: /cvsroot/zsh/zsh/Test/C03traps.ztst,v
retrieving revision 1.20
diff -p -u -r1.20 C03traps.ztst
--- Test/C03traps.ztst	12 Sep 2010 18:56:41 -0000	1.20
+++ Test/C03traps.ztst	13 Dec 2012 10:14:06 -0000
@@ -388,6 +388,17 @@
 >}
 >No, really exited
 
+   (set -e
+    printf "a\nb\n" | while read line
+    do
+       [[ $line = a* ]] || continue
+       ((ctr++))
+       [[ $line = foo ]]
+    done
+    echo "ctr = $ctr"
+   )
+1:ERREXIT in loop with simple commands
+
 %clean
 
   rm -f TRAPEXIT