From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 18575 invoked from network); 11 Jun 2007 15:43:01 -0000 X-Spam-Checker-Version: SpamAssassin 3.2.0 (2007-05-01) on f.primenet.com.au X-Spam-Level: X-Spam-Status: No, score=-2.6 required=5.0 tests=AWL,BAYES_00 autolearn=no version=3.2.0 Received: from news.dotsrc.org (HELO a.mx.sunsite.dk) (130.225.247.88) by ns1.primenet.com.au with SMTP; 11 Jun 2007 15:43:01 -0000 Received-SPF: none (ns1.primenet.com.au: domain at sunsite.dk does not designate permitted sender hosts) Received: (qmail 52950 invoked from network); 11 Jun 2007 15:42:55 -0000 Received: from sunsite.dk (130.225.247.90) by a.mx.sunsite.dk with SMTP; 11 Jun 2007 15:42:55 -0000 Received: (qmail 10642 invoked by alias); 11 Jun 2007 15:42:52 -0000 Mailing-List: contact zsh-workers-help@sunsite.dk; run by ezmlm Precedence: bulk X-No-Archive: yes X-Seq: 23547 Received: (qmail 10633 invoked from network); 11 Jun 2007 15:42:51 -0000 Received: from news.dotsrc.org (HELO a.mx.sunsite.dk) (130.225.247.88) by sunsite.dk with SMTP; 11 Jun 2007 15:42:51 -0000 Received: (qmail 52635 invoked from network); 11 Jun 2007 15:42:51 -0000 Received: from cluster-c.mailcontrol.com (168.143.177.190) by a.mx.sunsite.dk with SMTP; 11 Jun 2007 15:42:48 -0000 Received: from cameurexb01.EUROPE.ROOT.PRI ([62.189.241.200]) by rly16c.srv.mailcontrol.com (MailControl) with ESMTP id l5BFgbpJ016726 for ; Mon, 11 Jun 2007 16:42:37 +0100 Received: from news01.csr.com ([10.103.143.38]) by cameurexb01.EUROPE.ROOT.PRI with Microsoft SMTPSVC(6.0.3790.1830); Mon, 11 Jun 2007 16:42:36 +0100 Received: from news01.csr.com (localhost.localdomain [127.0.0.1]) by news01.csr.com (8.13.8/8.13.4) with ESMTP id l5BFgaBM014410 for ; Mon, 11 Jun 2007 16:42:36 +0100 Received: from csr.com (pws@localhost) by news01.csr.com (8.13.8/8.13.8/Submit) with ESMTP id l5BFgaFe014407 for ; Mon, 11 Jun 2007 16:42:36 +0100 Message-Id: <200706111542.l5BFgaFe014407@news01.csr.com> X-Authentication-Warning: news01.csr.com: pws owned process doing -bs To: zsh-workers@sunsite.dk (Zsh hackers list) Subject: PATCH: error status from math evaluation Date: Mon, 11 Jun 2007 16:42:36 +0100 From: Peter Stephenson X-OriginalArrivalTime: 11 Jun 2007 15:42:36.0569 (UTC) FILETIME=[22895090:01C7AC3F] Content-Type: text/plain MIME-Version: 1.0 X-Scanned-By: MailControl A-07-07-05 (www.mailcontrol.com) on 10.67.0.126 Playing with parameters, I discovered that an error that would normally be fatal (i.e., that sets the internal variable errflag) in (( ... )) not only resets the error flag, it returns status zero. For example % readonly readonly % (( readonly = 1 )) && print Success zsh: read-only variable: readonly Success That combination surely can't be right. Resetting the error flag (after detecting the error) is probably OK; that just means the script or function etc. doesn't bomb out at that point. I wouldn't want to change that now since it has quite wide implications for execution and error handling. The status zero comes from the fact that we explicitly set the returned integer to the value of errflag if that's non-zero, and then we use that value to test for the status; the non-zero value produces zero return status. This is rather screwy; I can only imagine that someone thought "well, the value is probably wrong so I can't return that... let's return errflag instead." But the value isn't useful and the caller hasn't been given a way of detecting this has happened. Suppose we return status 2 on an error? That's consistent with other bits of the shell and gives something to test for. I don't see any reason to maintain compatibility---the current code is just too obscure to be useful. I've set the returned value from mathevall() to zero instead of errflag, which seems to make more sense. At the moment I can't see a way the old behaviour could ever be useful. The value is irrelevant for (( ... )) because of the previous fix, and it's irrelevant for $(( ... )) because we don't reset the error flag in that case: % print $(( readonly = 1 )); echo Got here zsh: read-only variable: readonly (no further output because the command sequence was aborted.) This is inconsistent but arguable---in this form you haven't got the chance to handle a bad return status so an exception is all that's left. In summary, the only visible change is status 2 instead of 0 on an error from (( ... )). Index: Doc/Zsh/builtins.yo =================================================================== RCS file: /cvsroot/zsh/zsh/Doc/Zsh/builtins.yo,v retrieving revision 1.94 diff -u -r1.94 builtins.yo --- Doc/Zsh/builtins.yo 28 May 2007 22:57:40 -0000 1.94 +++ Doc/Zsh/builtins.yo 11 Jun 2007 15:24:36 -0000 @@ -708,7 +708,8 @@ ifzman(the section `Arithmetic Evaluation' in zmanref(zshmisc))\ ifnzman(noderef(Arithmetic Evaluation)) for a description of arithmetic expressions. The exit status is 0 if the -value of the last expression is nonzero, and 1 otherwise. +value of the last expression is nonzero, 1 if it is zero, and 2 if +an error occurred. ) findex(limit) cindex(resource limits) Index: Doc/Zsh/arith.yo =================================================================== RCS file: /cvsroot/zsh/zsh/Doc/Zsh/arith.yo,v retrieving revision 1.10 diff -u -r1.10 arith.yo --- Doc/Zsh/arith.yo 30 Jun 2006 09:41:35 -0000 1.10 +++ Doc/Zsh/arith.yo 11 Jun 2007 15:24:36 -0000 @@ -22,7 +22,7 @@ arithmetic expansion performed as for an argument of tt(let). More precisely, `tt(LPAR()LPAR())var(...)tt(RPAR()RPAR())' is equivalent to `tt(let ")var(...)tt(")'. The return status is 0 if the arithmetic value -of the expression is non-zero, and 1 otherwise. +of the expression is non-zero, 1 if it is zero, and 2 if an error occurred. For example, the following statement Index: Src/exec.c =================================================================== RCS file: /cvsroot/zsh/zsh/Src/exec.c,v retrieving revision 1.116 diff -u -r1.116 exec.c --- Src/exec.c 3 Jun 2007 17:44:20 -0000 1.116 +++ Src/exec.c 11 Jun 2007 15:24:37 -0000 @@ -3662,7 +3662,10 @@ fprintf(xtrerr, " ))\n"); fflush(xtrerr); } - errflag = 0; + if (errflag) { + errflag = 0; + return 2; + } /* should test for fabs(val.u.d) < epsilon? */ return (val.type == MN_INTEGER) ? val.u.l == 0 : val.u.d == 0.0; } Index: Src/math.c =================================================================== RCS file: /cvsroot/zsh/zsh/Src/math.c,v retrieving revision 1.29 diff -u -r1.29 math.c --- Src/math.c 12 Feb 2007 16:43:42 -0000 1.29 +++ Src/math.c 11 Jun 2007 15:24:37 -0000 @@ -1061,8 +1061,19 @@ "BUG: math: wallabies roaming too freely in outback"); if (errflag) { + /* + * This used to set the return value to errflag. + * I don't understand how that could be useful; the + * caller doesn't know that's what's happened and + * may not get a value at all. + * Worse, we reset errflag in execarith() and setting + * this explicitly non-zero means a (( ... )) returns + * status 0 if there's an error. That surely can't + * be right. execarith() now detects an error and returns + * status 2. + */ ret.type = MN_INTEGER; - ret.u.l = errflag; + ret.u.l = 0; } else { if (stack[0].val.type == MN_UNSET) ret = getnparam(stack[0].lval); -- Peter Stephenson Software Engineer CSR PLC, Churchill House, Cambridge Business Park, Cowley Road Cambridge, CB4 0WZ, UK Tel: +44 (0)1223 692070 To access the latest news from CSR copy this link into a web browser: http://www.csr.com/email_sig.php To get further information regarding CSR, please visit our Investor Relations page at http://ir.csr.com/csr/about/overview