[PATCH] Use or-assignments to set noerrexit bits

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

[-- Attachment #1.1: Type: text/plain, Size: 2000 bytes --]

Currently execif
<https://github.com/zsh-users/zsh/blob/41b402d36d0aeac594cf424a9e46b5edb20c815d/Src/loop.c#L550>
uses an or-assignment to set the NOERREXIT_EXIT and NOERREXIT_RETURN bits.
Other places, like for example execwhile
<https://github.com/zsh-users/zsh/blob/41b402d36d0aeac594cf424a9e46b5edb20c815d/Src/loop.c#L431>,
use a plain assignment. This looks wrong because if any other noerrexit
bits are present, they would inadvertently be lost.

Currently there is only one other noerrexit bit that is ever
set: NOERREXIT_SIGNAL. It is set in init.c here
<https://github.com/zsh-users/zsh/blob/master/Src/init.c#L1161> and here
<https://github.com/zsh-users/zsh/blob/master/Src/init.c#L1299>. In both
cases it seems to only affect the execution of initialization scripts.

The fourth noerrexit bit NOERREXIT_UNTIL_EXEC is never used. For more
details on this, see the discussion of my other patch that removes this bit.

Thus, in regular code, it's currently impossible to observe a difference
between or-assignments and plain assignments. However, it's certainly
possible to build examples with well placed sleep statements inside if and
while conditions in initialization scripts such that a SIGINT triggers
different behaviors depending on whether it's caught in the middle of an if
condition or in the middle of a while condition, which sounds wrong.
Furthermore, if new noerrexit bits are ever added, the current plain
assignments would most likely become problematic.

For these reasons, I think it's best to systematically use or-assignments
to set noerrexit bits (and and-assigments, like in "noerrexit &=
~NOERREXIT_RETURN" to remove them, which is already the case today).
Restoring bits to their prior state, like in "noerrexit = olderrexit",
should keep using plain assignments.

This patch is built on top of my
patch patch-07-remove-noerrexit-until-exec.txt but I don't think that it
depends on it nor on any of my previous patches (but I haven't tried to
confirm it).

Philippe

[-- Attachment #1.2: Type: text/html, Size: 2298 bytes --]

[-- Attachment #2: patch-08-use-or-assignments-to-set-noerrexit-bits --]
[-- Type: application/octet-stream, Size: 1701 bytes --]

diff --git a/Src/exec.c b/Src/exec.c
index 1dd569019..1810fca5e 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -1415,7 +1415,7 @@ execlist(Estate state, int dont_change_job, int exiting)
 	    int oerrexit_opt = opts[ERREXIT];
 	    Param pm;
 	    opts[ERREXIT] = 0;
-	    noerrexit = NOERREXIT_EXIT | NOERREXIT_RETURN;
+	    noerrexit |= NOERREXIT_EXIT | NOERREXIT_RETURN;
 	    if (ltype & Z_SIMPLE) /* skip the line number */
 		pc2++;
 	    pm = assignsparam("ZSH_DEBUG_CMD",
@@ -1472,7 +1472,7 @@ execlist(Estate state, int dont_change_job, int exiting)
 	    next = state->pc + WC_SUBLIST_SKIP(code);
 	    /* suppress errexit for commands before && and || and after ! */
 	    if (isandor || isnot)
-		noerrexit = NOERREXIT_EXIT | NOERREXIT_RETURN;
+		noerrexit |= NOERREXIT_EXIT | NOERREXIT_RETURN;
 	    switch (WC_SUBLIST_TYPE(code)) {
 	    case WC_SUBLIST_END:
 		/* End of sublist; just execute, ignoring status. */
@@ -1568,7 +1568,7 @@ sublist_done:
 	     */
 	    int oerrexit_opt = opts[ERREXIT];
 	    opts[ERREXIT] = 0;
-	    noerrexit = NOERREXIT_EXIT | NOERREXIT_RETURN;
+	    noerrexit |= NOERREXIT_EXIT | NOERREXIT_RETURN;
 	    exiting = donetrap;
 	    ret = lastval;
 	    dotrap(SIGDEBUG);
diff --git a/Src/loop.c b/Src/loop.c
index 61543ed73..88c55dd1a 100644
--- a/Src/loop.c
+++ b/Src/loop.c
@@ -428,7 +428,7 @@ execwhile(Estate state, UNUSED(int do_exec))
     } else {
         for (;;) {
             state->pc = loop;
-            noerrexit = NOERREXIT_EXIT | NOERREXIT_RETURN;
+            noerrexit |= NOERREXIT_EXIT | NOERREXIT_RETURN;
 
 	    /* In case the test condition is a functional no-op,
 	     * make sure signal handlers recognize ^C to end the loop. */