From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 27991 invoked by alias); 28 Dec 2014 00:32:25 -0000 Mailing-List: contact zsh-workers-help@zsh.org; run by ezmlm Precedence: bulk X-No-Archive: yes List-Id: Zsh Workers List List-Post: List-Help: X-Seq: 34065 Received: (qmail 787 invoked from network); 28 Dec 2014 00:32:23 -0000 X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on f.primenet.com.au X-Spam-Level: X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.2 X-CMAE-Score: 0 X-CMAE-Analysis: v=2.1 cv=CoYIqc8G c=1 sm=1 tr=0 a=FT8er97JFeGWzr5TCOCO5w==:117 a=kj9zAlcOel0A:10 a=q2GGsy2AAAAA:8 a=oR5dmqMzAAAA:8 a=-9mUelKeXuEA:10 a=A92cGCtB03wA:10 a=TZkh7pt8r0sJ1joUhkUA:9 a=ci4xh0Cguo9AW3Bo:21 a=mSAho2arRkM4_GyF:21 a=CjuIK1q_8ugA:10 From: Bart Schaefer Message-id: <141227163220.ZM11821@torch.brasslantern.com> Date: Sat, 27 Dec 2014 16:32:20 -0800 In-reply-to: <141226183516.ZM18384@torch.brasslantern.com> Comments: In reply to Bart Schaefer "Re: Bug report" (Dec 26, 6:35pm) References: <20141226165344.GC1003@basilisk> <141226183516.ZM18384@torch.brasslantern.com> X-Mailer: OpenZMail Classic (0.9.2 24April2005) To: zsh-workers@zsh.org Subject: [PATCH] ERR_EXIT with "for" loops and shell functions (Re: Bug report) MIME-version: 1.0 Content-type: text/plain; charset=us-ascii On Dec 26, 6:35pm, Bart Schaefer wrote: } } This is very tricky because we have to propagate the value of $? from } before the "for" keyword (including before the start of the function) } into the "do" body, but must *not* *use* the value of $? for deciding } whether to exit-on-error, because the nonzero value came from an "if" } test. There is a "noerrexit" flag that is supposed to cover this case, } but it's not set when the very first "sublist" in a function body is } a for-loop (there are likely to be other similar cases). There's a further complication here, which is this: zsh -fc 'false; for x; do :; done; echo $?' 1 A "for" loop which executes zero times is supposed to set $? == 0, or at least it does so in bash. Zsh 5.0.7 passes through $? unchanged when the "for" does not loop. Oddly, the current version from git behaves differently: Src/zsh -fc 'false; for x in; do false; done; echo $?' 0 But: Src/zsh -o null_glob -fc 'false; for x in _*; do false; done; echo $?' 1 I haven't traced through why a missing "in" list resets $? to zero, whereas an empty list created by globbing passes through the outer $?. Here's another difference from bash: bash -c 'false; select x in; do false; done; echo $?' bash: -c: line 0: syntax error near unexpected token `;' bash: -c: line 0: `false; select x in; do false; done; echo $?' whereas zsh accepts a variety of syntax with inconsistent return: zsh -fc 'true; select x; do :; done; echo $?' 1 zsh -fc 'true; select x in; do :; done; echo $?' 0 zsh -fc 'true; select x in $empty; do :; done; echo $?' 1 The first and last of those three also pass syntax in bash and give a loop that executes zero times and sets $? to zero rather than one. The following patch attempts to fix all of this weirdness. I'm a bit concerned that there may still be cases where prefork substitutions are performed when they actually should not be because the errexit [not noerrexit] condition has not been tested soon enough -- it seems as if zsh is checking errexit at the start of the next command rather than immediately following the command that returned nonzero, in at least some circumstances -- but I can't come up with a test case. Possibly this whole thing needs to be rethought. Anyway, the most controversial thing below is probably the change in the return status of "select", which is now always zero unless something in the loop body returns nonzero. If we want to disallow empty-"in" syntax we should do it elsewhere. diff --git a/Src/exec.c b/Src/exec.c index 6a7dbb1..eaf73df 100644 --- a/Src/exec.c +++ b/Src/exec.c @@ -2632,6 +2632,10 @@ execcmd(Estate state, int input, int output, int how, int last1) } } + /* if we get this far, it is OK to pay attention to lastval again */ + if (noerrexit == 2 && !is_shfunc) + noerrexit = 0; + /* Do prefork substitutions */ esprefork = (assign || isset(MAGICEQUALSUBST)) ? PREFORK_TYPESET : 0; if (args && htok) diff --git a/Src/loop.c b/Src/loop.c index 8bb1ec9..7b3bdd2 100644 --- a/Src/loop.c +++ b/Src/loop.c @@ -102,7 +102,10 @@ execfor(Estate state, int do_exec) addlinknode(args, dupstring(*x)); } } - /* lastval = 0; */ + + if (!args || empty(args)) + lastval = 0; + loops++; pushheap(); cmdpush(CS_FOR); @@ -238,10 +241,10 @@ execselect(Estate state, UNUSED(int do_exec)) } if (!args || empty(args)) { state->pc = end; - return 1; + return 0; } loops++; - /* lastval = 0; */ + pushheap(); cmdpush(CS_SELECT); usezle = interact && SHTTY != -1 && isset(USEZLE); @@ -519,14 +522,17 @@ execif(Estate state, int do_exec) s = 1; state->pc = next; } - noerrexit = olderrexit; if (run) { + /* we need to ignore lastval until we reach execcmd() */ + noerrexit = olderrexit ? olderrexit : lastval ? 2 : 0; cmdpush(run == 2 ? CS_ELSE : (s ? CS_ELIFTHEN : CS_IFTHEN)); execlist(state, 1, do_exec); cmdpop(); - } else + } else { + noerrexit = olderrexit; lastval = 0; + } state->pc = end; return lastval; -- Barton E. Schaefer