zsh-workers
 help / color / mirror / code / Atom feed
From: Philippe Altherr <philippe.altherr@gmail.com>
To: Bart Schaefer <schaefer@brasslantern.com>
Cc: zsh-workers@zsh.org
Subject: Re: errexit and (Z)ERR trap regression
Date: Wed, 26 Jun 2024 14:42:46 +0200	[thread overview]
Message-ID: <CAGdYchvP9x83UtFH855_ddE1m88cMJdNmk27+RwzHC9KSJ=ucA@mail.gmail.com> (raw)
In-Reply-To: <CAH+w=7YrF3RMjqY6RJKjH=o8pTziok08aAx4g8gfff9_KYdVuQ@mail.gmail.com>


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

Note that just adding braces is enough to trigger the bug:

     $ zsh -e -c 'true && false; echo NOT REACHED'     # no output; correct
     $ zsh -e -c 'true && {false; echo NOT REACHED}'  # incorrect output
     NOT REACHED

and

     $ zsh -c 'trap "echo Trapped!" ERR; true && false'     # correct output
     Trapped!
     $ zsh -c 'trap "echo Trapped!" ERR; true && {false}'  # no output; bug

I think that the correct fix is the following:

    if (isandor || isnot)
        noerrexit = oldnoerrexit | NOERREXIT_EXIT | NOERREXIT_RETURN;
    else
        noerrexit = oldnoerrexit;

For reminder, here is the current code:

    if (isandor || isnot)
        noerrexit |= NOERREXIT_EXIT | NOERREXIT_RETURN;


In other words, every sub-command in a list should only depend on the
noerrexit that was in effect at the start of the evaluation of the list. It
should in no way depend on the noerrexit that results from the evaluation
of proceeding sub-commands as is currently the case. The current code only
works for lists where the last sub-command is a simple command, thanks to
the "noerrexit = oldnoerrexit;" performed after the last sub-command just
before checking whether an ERREXIT should be triggered.

Here are two other examples fixed by this patch:

    zsh -c 'trap "echo Trapped!" ERR; true && if true; then false; fi'
    zsh -c 'trap "echo Trapped!" ERR; true && {false} always {true}'

Philippe




On Tue, Jun 25, 2024 at 1:03 AM Bart Schaefer <schaefer@brasslantern.com>
wrote:

> On Fri, Jun 21, 2024 at 10:50 PM Lawrence Velázquez <larryv@zsh.org>
> wrote:
> >
> > >    set -e; true && { false; echo one; } || echo two
> > >
> > > Is "false" there considered to be "any command of an AND-OR list" ?
> >
> > This ought to work the same as the subshell version: early exit is
> > suppressed for the commands "true" and "{ false; echo one; }", so
> > the overall output is still "one".
>
> The following seems to fix all six of the mentioned cases from this
> thread, then.
>

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

[-- Attachment #2: noerrexit-list-subcommands.txt --]
[-- Type: text/plain, Size: 1611 bytes --]

diff --git a/Src/exec.c b/Src/exec.c
index e955e85df..916a3567d 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -1485,7 +1485,9 @@ 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 = oldnoerrexit | NOERREXIT_EXIT | NOERREXIT_RETURN;
+	    else
+		noerrexit = oldnoerrexit;
 	    switch (WC_SUBLIST_TYPE(code)) {
 	    case WC_SUBLIST_END:
 		/* End of sublist; just execute, ignoring status. */
diff --git a/Test/C03traps.ztst b/Test/C03traps.ztst
index de57765a0..9fefdf713 100644
--- a/Test/C03traps.ztst
+++ b/Test/C03traps.ztst
@@ -995,6 +995,25 @@ F:Must be tested with a top-level script rather than source or function
 ?loop 0
 ?loop 1
 
+  ( set -e; true && {false; echo NOT REACHED} )
+  ( trap "print Trapped!" ERR; true && {false} )
+  ( trap "print Trapped!" ERR; true && if true; then false; fi )
+  ( trap "print Trapped!" ERR; true && {false} always {true} )
+  ( true && (set -e; false; echo NOT REACHED) )
+  ( true && (trap "print Trapped!" ERR; false) )
+  ( true && { set -e; false; echo NOT REACHED } )
+  ( true && { trap "print Trapped!" ERR; false } )
+  ( set -e; true && (false; echo one) || echo two )
+  ( set -e; true && { false; echo one; } || echo two )
+0:ERR_EXIT is triggered by last command in an AND-OR list
+>Trapped!
+>Trapped!
+>Trapped!
+>Trapped!
+>Trapped!
+>one
+>one
+
   if zmodload zsh/system 2>/dev/null; then
   (
     trap 'echo TERM; exit 2' TERM

  reply	other threads:[~2024-06-26 12:43 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-06-20 17:12 Martijn Dekker
2024-06-21 18:46 ` Bart Schaefer
2024-06-21 20:02   ` Lawrence Velázquez
2024-06-21 20:36     ` Bart Schaefer
2024-06-22  5:49       ` Lawrence Velázquez
2024-06-24 23:02         ` Bart Schaefer
2024-06-26 12:42           ` Philippe Altherr [this message]
2024-06-26 21:43             ` Bart Schaefer
2024-06-27  2:01               ` Philippe Altherr
2024-06-27 16:43                 ` Bart Schaefer
2024-06-27 17:09                   ` Philippe Altherr

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAGdYchvP9x83UtFH855_ddE1m88cMJdNmk27+RwzHC9KSJ=ucA@mail.gmail.com' \
    --to=philippe.altherr@gmail.com \
    --cc=schaefer@brasslantern.com \
    --cc=zsh-workers@zsh.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).