Re: [PATCH] More ERR_EXIT (was Re: Tests RE behavior of ERR_EXIT)

[Philippe Altherr <philippe.altherr@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 5202 bytes --]

>
> You shouldn't even be bothering with 5.8.1, it's been wrong all along;
> it blindly never errexits at the end of an if/then/fi.


I think that this isn't necessarily wrong. My understanding of the code so
far is that the decision to trigger an ERR_EXIT is pushed down the
evaluation stack. In "if cmd1; then cmd2; else cmd3; fi", only the
evaluations of the (word codes representing the) commands "cmd1", "cmd2",
or "cmd3" can ever trigger an ERR_EXIT. The evaluation of the (word codes
representing the) if/then/else itself never triggers an ERR_EXIT. In other
words only (the word codes representing) "basic commands", like function
calls or UNIX commands, can ever trigger an ERR_EXIT. This strategy has the
benefit that ERR_EXIT will be triggered exactly at the point where the
fatal non-zero exit status was produced.

00 if
01   cmd1
02  then
03   cmd2
04. else
05.   cmd3
06 fi

In the example above, with the strategy I described, and with the knowledge
that the if condition never triggers an ERR_EXIT, it's guaranteed that an
ERR_EXIT will only ever be thrown at line 03 or 05. If the triggering of
the ERR_EXIT was sometimes delayed and delegated to the if/then/else, then
ERR_EXIT could also be triggered at line 02 or 04, or worse at line 01 or
06, which wouldn't let you know whether the non-zero status originated from
"cmd2" or from "cmd3". The delayed/delegated triggering looks undesirable
because it gives you less information on the origin of the error. My
understanding is that it's also never needed.

The behavior of ERR_EXIT is controlled by the variables "noerrexit" and
"local_noerrexit". My understanding of these variables is the following:

- noerrexit: This variable is set to indicate that the triggering of
ERR_EXIT must be disabled in the evaluation of any word code from the point
where it's set until it's reset. For example it's set here
<https://github.com/zsh-users/zsh/blob/b1533066ca7d50c88b37ce72093c12cf19807818/Src/loop.c#L550>
in execif before the evaluation of the condition and reset here
<https://github.com/zsh-users/zsh/blob/b1533066ca7d50c88b37ce72093c12cf19807818/Src/loop.c#L576>,
here
<https://github.com/zsh-users/zsh/blob/b1533066ca7d50c88b37ce72093c12cf19807818/Src/loop.c#L578>,
here
<https://github.com/zsh-users/zsh/blob/b1533066ca7d50c88b37ce72093c12cf19807818/Src/loop.c#L580>,
and here
<https://github.com/zsh-users/zsh/blob/b1533066ca7d50c88b37ce72093c12cf19807818/Src/loop.c#L585>
after the evaluation of the condition. I don't really understand why the
reseting is so complicated. It's much more straightforward in execwhile (
here
<https://github.com/zsh-users/zsh/blob/b1533066ca7d50c88b37ce72093c12cf19807818/Src/loop.c#L440>
).

- local_noerrexit: This variable is set to indicate that the triggering of
ERR_EXIT must be disabled in the remainder of the evaluation of the current
word code. For example it's set at the end of each compound command, like
here
<https://github.com/zsh-users/zsh/blob/b1533066ca7d50c88b37ce72093c12cf19807818/Src/loop.c#L481>.
This used to be a plain "this_noerrexit = 1", which I don't think was wrong.

I think my patches so far have uncovered a different bug that was
> already present but was masked by the foregoing, which is, that
> noerrexit is unwound in cases where it should not be.  I think this is
> happening at lines 1530-1531 of exec.c, right under the comment about
> "hairy code near the end of execif()".  That's an area I didn't touch,
> but I'm pretty sure it's restoring noerrexit to its state before
> entering the "if" (oldnoerrexit) when it should be preserving the
> state from the "&&" conditional.  In 5.8.1 this gets reversed again
> via this_noerrexit.


I must admit that I don't understand the NOERREXIT_UNTIL_EXEC logic here
<https://github.com/zsh-users/zsh/blob/b1533066ca7d50c88b37ce72093c12cf19807818/Src/exec.c#L1529>,
nor the complicated resetting logic of noerrexit at the end of execif. I
was about to say that this doesn't seem to be the source of the problem
because if, while, and for statements all behave the same in Zsh.

function fooIf1()    { init; cond=true; if    $cond; then cond=false; false
>        ; fi  ; }
> function fooIf2()    { init; cond=true; if    $cond; then cond=false;
> false && true; fi  ; }



function fooWhile1() { init; cond=true; while $cond; do   cond=false; false
>        ; done; }
> function fooWhile2() { init; cond=true; while $cond; do   cond=false;
> false && true; done; }



function fooFor1()   { init; cond=true; for v in x ; do   cond=false; false
>        ; done; }
> function fooFor2()   { init; cond=true; for v in x ; do   cond=false;
> false && true; done; }


In the examples above fooIf1, fooWhile1, and fooFor1 all work correctly but
fooIf2, fooWhile2, and fooFor2 fail to trigger ERR_EXIT in Zsh 5.8 and
trigger it too early (in foo instead of in bar) in Zsh 5.9.

However, if I comment out the NOERREXIT_UNTIL_EXEC logic in exec.c (or
remove the negation), then fooIf2 and surprisingly also fooFor2 work
correctly in Zsh 5.9 but not fooWhile2!?! fooWhile2 still triggers too
early.

So it looks like this may indeed be the start of the answer. But I'm still
scratching my head on why that is.

Philippe

[-- Attachment #2: Type: text/html, Size: 7053 bytes --]