Re: PATCH: Don't crash when math recursion limit is exceeded

[Mikael Magnusson <mikachu@xxxxxxxxx>]

On 10 March 2013 19:42, Peter Stephenson <p.w.stephenson@xxxxxxxxxxxx> wrote:
> On Sun, 10 Mar 2013 13:06:37 +0100
> Mikael Magnusson <mikachu@xxxxxxxxx> 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