Re: TRAPZERR() in chpwd()

Re: TRAPZERR() in chpwd()

[Sven Wischnowsky]

Peter Stephenson wrote:

> Sven wrote
> > It made things like `false && echo a; echo b' fail. The `echo b'
> > wasn't executed.
> >
> > The simplest way to solve this is by saving and
> > restoring donetrap around the execution of the pipelines.
> > 
> > 
> > I haven't committed the patch... does anyone see a problem with it?
> 
> Yes, the `donetrap' test which makes sure that the trap is only executed
> once for each occurrence of a non-zero return status isn't working.  (I
> think this is pretty much the only reason for having donetrap as a static
> variable, so saving it and restoring it essentially disables its staticness
> altogether, although there may well be some subtlety I'm missing.)

At least the things I triend my patch with work.

> % trap 'print ERRR-or!' ZERR
> % f() { false; }
> % f
> ERRR-or!
> ERRR-or!
> 
> How about a less drastic version of my previous patch?  I think the point
> is that we just need the `donetrap' effect but not the goto effect

Yep.

Bye
 Sven


--
Sven Wischnowsky                         wischnow@informatik.hu-berlin.de

Re: TRAPZERR() in chpwd()

[Peter Stephenson]

Sven wrote
> It made things like `false && echo a; echo b' fail. The `echo b'
> wasn't executed.
>
> The simplest way to solve this is by saving and
> restoring donetrap around the execution of the pipelines.
> 
> 
> I haven't committed the patch... does anyone see a problem with it?

Yes, the `donetrap' test which makes sure that the trap is only executed
once for each occurrence of a non-zero return status isn't working.  (I
think this is pretty much the only reason for having donetrap as a static
variable, so saving it and restoring it essentially disables its staticness
altogether, although there may well be some subtlety I'm missing.)

% trap 'print ERRR-or!' ZERR
% f() { false; }
% f
ERRR-or!
ERRR-or!

How about a less drastic version of my previous patch?  I think the point
is that we just need the `donetrap' effect but not the goto effect (which I
should have realised before).

We also need some more TRAPZERR stuff in Test/08traps.ztst for this kind of
thing.

Index: Src/exec.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/exec.c,v
retrieving revision 1.5
diff -u -r1.5 exec.c
--- Src/exec.c	2000/04/10 09:18:27	1.5
+++ Src/exec.c	2000/05/02 11:58:56
@@ -842,7 +842,8 @@
 			 * for this sublist.                                   */
 			donetrap = 1;
 			goto sublist_done;
-		    }
+		    } else if (WC_SUBLIST_TYPE(code) == WC_SUBLIST_END)
+			donetrap = 1;
 		}
 		cmdpush(CS_CMDAND);
 		break;
@@ -867,7 +868,8 @@
 			 * for this sublist.                                   */
 			donetrap = 1;
 			goto sublist_done;
-		    }
+		    } else if (WC_SUBLIST_TYPE(code) == WC_SUBLIST_END)
+			donetrap = 1;
 		}
 		cmdpush(CS_CMDOR);
 		break;

-- 
Peter Stephenson <pws@cambridgesiliconradio.com>
Cambridge Silicon Radio, Unit 300, Science Park, Milton Road,
Cambridge, CB4 0XL, UK                          Tel: +44 (0)1223 392070

Re: TRAPZERR() in chpwd()

[Sven Wischnowsky]

Peter Stephenson wrote:

> Paul Ackersviller wrote:
> > I believe the change must've come after 3.1.6, since it exhibits the same
> > behaviour as 3.0.7 in my experience.  It's only 3.1.7-pre-1 and (some?)
> > development versions leading up to it.
> 
> Right, I think it's the wordcode stuff.  I'm pretty sure I spent some time
> (possibly more than once...) getting this right a fair while ago.
> 
> Sven, can you confirm that this is the right fix before I commit it?  Or if
> not, what is, because this can't be too far away from the right answer.
> It seems to work with all the basic tests I've tried.

It made things like `false && echo a; echo b' fail. The `echo b'
wasn't executed.

Hm, why not just setting dontrap unconditionally in the WC_SUBLIST_AND 
branch? Seems to work fine.

No, wait, `{ false && true } || false' doesn't trigger the trap. Hm,
is this an old bug? The simplest way to solve this is by saving and
restoring donetrap around the execution of the pipelines.


I haven't committed the patch... does anyone see a problem with it?

Bye
 Sven

Index: Src/exec.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/exec.c,v
retrieving revision 1.5
diff -u -r1.5 exec.c
--- Src/exec.c	2000/04/10 09:18:27	1.5
+++ Src/exec.c	2000/05/02 11:14:45
@@ -770,7 +770,7 @@
     static int donetrap;
     Wordcode next;
     wordcode code;
-    int ret, cj, csp, ltype;
+    int ret, cj, csp, ltype, odt;
     int old_pline_level, old_list_pipe, oldlineno;
     /*
      * ERREXIT only forces the shell to exit if the last command in a &&
@@ -809,6 +809,7 @@
 	code = *state->pc++;
 	while (wc_code(code) == WC_SUBLIST) {
 	    next = state->pc + WC_SUBLIST_SKIP(code);
+	    odt = donetrap;
 	    if (!oldnoerrexit)
 		noerrexit = (WC_SUBLIST_TYPE(code) != WC_SUBLIST_END);
 	    switch (WC_SUBLIST_TYPE(code)) {
@@ -818,6 +819,7 @@
 		    execsimple(state);
 		else
 		    execpline(state, code, ltype, (ltype & Z_END) && exiting);
+		donetrap = odt;
 		state->pc = next;
 		goto sublist_done;
 		break;
@@ -827,6 +829,7 @@
 		if ((ret = ((WC_SUBLIST_FLAGS(code) & WC_SUBLIST_SIMPLE) ?
 			    execsimple(state) :
 			    execpline(state, code, Z_SYNC, 0)))) {
+		    donetrap = odt;
 		    state->pc = next;
 		    code = *state->pc++;
 		    next = state->pc + WC_SUBLIST_SKIP(code);
@@ -843,7 +846,9 @@
 			donetrap = 1;
 			goto sublist_done;
 		    }
-		}
+		    donetrap = 1;
+		} else
+		    donetrap = odt;
 		cmdpush(CS_CMDAND);
 		break;
 	    case WC_SUBLIST_OR:
@@ -852,6 +857,7 @@
 		if (!(ret = ((WC_SUBLIST_FLAGS(code) & WC_SUBLIST_SIMPLE) ?
 			     execsimple(state) :
 			     execpline(state, code, Z_SYNC, 0)))) {
+		    donetrap = odt;
 		    state->pc = next;
 		    code = *state->pc++;
 		    next = state->pc + WC_SUBLIST_SKIP(code);
@@ -868,7 +874,8 @@
 			donetrap = 1;
 			goto sublist_done;
 		    }
-		}
+		} else
+		    donetrap = odt;
 		cmdpush(CS_CMDOR);
 		break;
 	    }

--
Sven Wischnowsky                         wischnow@informatik.hu-berlin.de

Re: TRAPZERR() in chpwd()

[Peter Stephenson]

Paul Ackersviller wrote:
> I believe the change must've come after 3.1.6, since it exhibits the same
> behaviour as 3.0.7 in my experience.  It's only 3.1.7-pre-1 and (some?)
> development versions leading up to it.

Right, I think it's the wordcode stuff.  I'm pretty sure I spent some time
(possibly more than once...) getting this right a fair while ago.

Sven, can you confirm that this is the right fix before I commit it?  Or if
not, what is, because this can't be too far away from the right answer.
It seems to work with all the basic tests I've tried.

Index: Src/exec.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/exec.c,v
retrieving revision 1.5
diff -u -r1.5 exec.c
--- Src/exec.c	2000/04/10 09:18:27	1.5
+++ Src/exec.c	2000/05/02 09:15:33
@@ -836,7 +836,8 @@
 			code = *state->pc++;
 			next = state->pc + WC_SUBLIST_SKIP(code);
 		    }
-		    if (wc_code(code) != WC_SUBLIST) {
+		    if (wc_code(code) != WC_SUBLIST ||
+			WC_SUBLIST_TYPE(code) == WC_SUBLIST_END) {
 			/* We've skipped to the end of the list, not executing *
 			 * the final pipeline, so don't perform error handling *
 			 * for this sublist.                                   */
@@ -861,7 +862,8 @@
 			code = *state->pc++;
 			next = state->pc + WC_SUBLIST_SKIP(code);
 		    }
-		    if (wc_code(code) != WC_SUBLIST) {
+		    if (wc_code(code) != WC_SUBLIST ||
+			WC_SUBLIST_TYPE(code) == WC_SUBLIST_END) {
 			/* We've skipped to the end of the list, not executing *
 			 * the final pipeline, so don't perform error handling *
 			 * for this sublist.                                   */

-- 
Peter Stephenson <pws@cambridgesiliconradio.com>
Cambridge Silicon Radio, Unit 300, Science Park, Milton Road,
Cambridge, CB4 0XL, UK                          Tel: +44 (0)1223 392070

Re: TRAPZERR() in chpwd()

[Paul Ackersviller]

On Mon, May 01, 2000 at 06:22:16PM +0000, Bart Schaefer wrote:
> zagzig% echo $ZSH_VERSION
> 3.0.6-test-1
> zagzig% TRAPZERR () { echo exit code $?; }
> zagzig% chpwd () { [ -h $PWD ] && echo symlink; true; }
> zagzig% cd /tmp
> zagzig% cd /hosts
> symlink
> zagzig% 
> 
> This may be a fix that went in to 3.0.x and was never propagated to 3.1.x,
> or it may be a bug that was introduced in 3.0.x and fixed in 3.1.x.  I'm
> not sure which at this point.

I believe the change must've come after 3.1.6, since it exhibits the same
behaviour as 3.0.7 in my experience.  It's only 3.1.7-pre-1 and (some?)
development versions leading up to it.

-- 
Paul Ackersviller