zsh-workers
 help / color / mirror / code / Atom feed
* [Patch] Fix ERR_RETURN behavior in and/or statements
@ 2022-11-25 17:44 Philippe Altherr
  2022-11-25 17:55 ` Bart Schaefer
  0 siblings, 1 reply; 6+ messages in thread
From: Philippe Altherr @ 2022-11-25 17:44 UTC (permalink / raw)
  To: Zsh hackers list; +Cc: Bart Schaefer, Lawrence Velázquez


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

ATTENTION: This patch depends on the following patches described here
<https://inbox.vuxu.org/zsh-workers/CAGdYchufLwUgpKVU6b5eOiF15p0njFr59H1q9XizypwNjzqKzw@mail.gmail.com/>
:
- patch-01-revert-mistaken-errexit-patches.txt
- patch-03-fix-negation-statement.txt

I have found another issue in execlist, which affects the behavior of
ERR_RETURN in convoluted cases that involve at least two &&/|| commands and
a function call. The following example exhibits the issue.

set -o err_return
> fn() { { false; echo fn1 } && echo fn2 }
> fn; echo done
> fn && echo done


The execution of "fn; echo done" prints "fn1", "fn2", and "done". The
execution of "fn && echo done" should print the same but currently prints
nothing.

The problem is that this test
<https://github.com/zsh-users/zsh/blob/291940bae6cc0471c35c73498e873bc58dae9a95/Src/exec.c#L1429>
in "execlist" is too conservative. Inside "fn", "oldnoerrexit" is equal to
"NOERREXIT_EXIT". Indeed the "&&" in "fn && echo done"  first sets
"noerrexit" to  "NOERREXIT_EXIT | NOERREXIT_RETURN". The call to "fn" then
removes "NOERREXIT_RETURN". Thus, at the beginning of "execlist" in the
call to "fn", "noerrexit" is equal to "NOERREXIT_EXIT" and "oldnoerrexit"
is initialized to the same value. Since "oldnoerrexit" is non-zero, the test
<https://github.com/zsh-users/zsh/blob/291940bae6cc0471c35c73498e873bc58dae9a95/Src/exec.c#L1429>
fails
to readd "NOERREXIT_RETURN" for the evaluation of "{ false; echo fn1 }".
Therefore the evaluation of "false" triggers an early return of "fn" wîth
exit status 1, which in turn triggers an early return of the toplevel and
thus nothing is printed.

The solution is to unconditionally set "noerrexit" to "NOERREXIT_EXIT |
NOERREXIT_RETURN" for any command on the left of "&&" and "||" (and for any
command on the right of "!"). The patch does exactly that (and adds a
regression test).

Philippe

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

[-- Attachment #2: patch-05-fix-and-or-statement.txt --]
[-- Type: text/plain, Size: 1955 bytes --]

diff --git a/Src/exec.c b/Src/exec.c
index 43df8211a..711d8f374 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -1427,14 +1427,12 @@ execlist(Estate state, int dont_change_job, int exiting)
 	    goto sublist_done;
 	}
 	while (wc_code(code) == WC_SUBLIST) {
-	    int isend = (WC_SUBLIST_TYPE(code) == WC_SUBLIST_END);
+	    int isandor = WC_SUBLIST_TYPE(code) != WC_SUBLIST_END;
+	    int isnot = WC_SUBLIST_FLAGS(code) & WC_SUBLIST_NOT;
 	    next = state->pc + WC_SUBLIST_SKIP(code);
-	    if (!oldnoerrexit)
-		noerrexit = isend ? 0 : NOERREXIT_EXIT | NOERREXIT_RETURN;
-	    if (WC_SUBLIST_FLAGS(code) & WC_SUBLIST_NOT) {
-		/* suppress errexit for the commands in ! <list-of-commands> */
+	    /* suppress errexit for commands before && and || and after ! */
+	    if (isandor || isnot)
 		noerrexit = NOERREXIT_EXIT | NOERREXIT_RETURN;
-	    }
 	    switch (WC_SUBLIST_TYPE(code)) {
 	    case WC_SUBLIST_END:
 		/* End of sublist; just execute, ignoring status. */
@@ -1444,7 +1442,7 @@ execlist(Estate state, int dont_change_job, int exiting)
 		    execpline(state, code, ltype, (ltype & Z_END) && exiting);
 		state->pc = next;
 		/* suppress errexit for the command "! ..." */
-		if (WC_SUBLIST_FLAGS(code) & WC_SUBLIST_NOT)
+		if (isnot)
 		  this_noerrexit = 1;
 		goto sublist_done;
 		break;
diff --git a/Test/C03traps.ztst b/Test/C03traps.ztst
index a8880673f..b7132da81 100644
--- a/Test/C03traps.ztst
+++ b/Test/C03traps.ztst
@@ -670,6 +670,22 @@ F:Must be tested with a top-level script rather than source or function
 >before-out
 >before-in
 
+  (set -o err_return
+    fn() {
+      print before-in
+      { false; true } && true
+      print after-in
+    }
+    print before-out
+    fn && true
+    print after-out
+  )
+0:ERR_RETURN not triggered on LHS of "&&" in function on LHS of "&&" (regression test)
+>before-out
+>before-in
+>after-in
+>after-out
+
   mkdir -p zdotdir
   print >zdotdir/.zshenv '
   setopt norcs errreturn

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-12-03  5:12 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-25 17:44 [Patch] Fix ERR_RETURN behavior in and/or statements Philippe Altherr
2022-11-25 17:55 ` Bart Schaefer
2022-11-26  3:47   ` Philippe Altherr
2022-12-03  1:31     ` Philippe Altherr
2022-12-03  1:36       ` Philippe Altherr
2022-12-03  5:11         ` Lawrence Velázquez

Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/zsh/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).