From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 19721 invoked by alias); 10 Mar 2013 20:01:43 -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: 31140 Received: (qmail 26283 invoked from network); 10 Mar 2013 20:01:41 -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.6 required=5.0 tests=BAYES_00,DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED,FREEMAIL_FROM,NML_ADSP_CUSTOM_MED,RCVD_IN_DNSWL_LOW, T_DKIM_INVALID autolearn=no version=3.3.2 Received-SPF: pass (ns1.primenet.com.au: SPF record at _netblocks.google.com designates 209.85.223.180 as permitted sender) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:x-received:in-reply-to:references:date:message-id :subject:from:to:cc:content-type; bh=s/4DdVRYJGQkGzQmPcS+V9DTv3SrcljIo4uL6ba1dxo=; b=qjOPepxZvlCqVqlDvPN8QEkPwEGhzl+RfoEiK9qPY/yk7comx9LqkrMFa/XZ6jFtDT kZrAROn4A9kcKnju1afQKoH6usy/tZgY1A47c8U7dBk5yIjTlcxn5l1LIBokNBlgiihZ 8ArHi+aXCLwh+G85EORK7W+vYZn+MPtC7hswdKK/CGJIZnbmNcWVskKZkTCSnX2rF3yT uLp8hB3NeNLaG4RVgkOVONNsQEq/ZCUPgDP0tBysxw6fFCUOM2WloxO2HFkfErjtwSgc 4feXNANCVMETebO2TkpKQ7c7CFO/SA5rKbQDGrwjy9Is/8ZAhe/DZBz2za+Qsk0rXFiI kj3Q== MIME-Version: 1.0 X-Received: by 10.50.186.165 with SMTP id fl5mr5048117igc.81.1362945696087; Sun, 10 Mar 2013 13:01:36 -0700 (PDT) In-Reply-To: <20130310184244.41e1d554@pws-pc.ntlworld.com> References: <1362917197-2576-1-git-send-email-mikachu@gmail.com> <20130310184244.41e1d554@pws-pc.ntlworld.com> Date: Sun, 10 Mar 2013 21:01:36 +0100 Message-ID: Subject: Re: PATCH: Don't crash when math recursion limit is exceeded From: Mikael Magnusson To: Peter Stephenson Cc: zsh-workers@zsh.org Content-Type: text/plain; charset=UTF-8 On 10 March 2013 19:42, Peter Stephenson wrote: > On Sun, 10 Mar 2013 13:06:37 +0100 > Mikael Magnusson wrote: >> This used to never trigger for me when compiling in debug mode, but I >> finally had better luck today and tracked it down. >> >> *ep seems to already be NULL, but I couldn't quite figure out where it >> was set to NULL so I figured it can't hurt to make it explicitly NULL >> at that point. I suppose setting *ep = ""; would also work (without the >> second hunk)? > > Another way of doing it would probably be to set "*ep = s", which is > consistent with what *ep is supposed to be doing. > > If you set it to NULL it's probably safest to change the value returned > by the other calls of mathevall() around line 960 pre-patch, which also > outputs the "bad math expression" error. I don't see why they shouldn't > be fully consistent; zerr() already tests for errflag, so they shouldn't > need to test for that, but either neither or both should. Ah, I didn't even think to check for other users... I don't have a good understanding of how zerr and errflag is meant to work (the definition has a comment "error/break flag"), but it seems like setting *ep=s is safer, yeah. (I'm a bit confused why this doesn't cause the 'illegal character' warning to print, but this is probably related to not knowing about zerr/errflag.) Somewhat relatedly, bash prints the name of the token that caused the recursion, and we could do this quite easily like this, I think; @@ -362,9 +362,9 @@ if (mlevel >= MAX_MLEVEL) { xyyval.type = MN_INTEGER; xyyval.u.l = 0; + *ep = s; - zerr("math recursion limit exceeded"); + zerr("math recursion limit exceeded: %s", *ep); return xyyval; } % bash -c 'foo=foo;((5 + foo + 7))' bash: ((: foo: expression recursion level exceeded (error token is "foo") % Src/zsh -c 'foo=foo;((5 + foo + 7))' zsh:1: math recursion limit exceeded: foo But maybe I'm getting carried away. -- Mikael Magnusson