Re: PATCH: Re: `foo=foo; (( foo ))' => infinite recursion

Re: PATCH: Re: `foo=foo; (( foo ))' => infinite recursion

[Sven Wischnowsky]

Bart Schaefer wrote:

> On Mar 14,  9:05am, Sven Wischnowsky wrote:
> } Subject: PATCH: Re: `foo=foo; (( foo ))' => infinite recursion
> }
> } 
> } Bart Schaefer wrote:
> } 
> } > On Mar 13, 10:34am, Sven Wischnowsky wrote:
> } > } Subject: Re: `foo=foo; (( foo ))' => infinite recursion
> } > }
> } > } What really irritated me was that getnumvalue() called matheval().
> } > 
> } > Maybe just a recursion counter with a reasonably large limit?
> } 
> } Ok.
> 
> Something is still too aggressive:
> 
> zagzig% foo='x > 0 ? (--x, y += foo) : 1'
> zagzig% x=3
> zagzig% echo $[foo]
> zsh: math recursion limit exceeded
> 
> `foo' should have been evaluated at most three times, no?
> 
> Maybe there's no real reason to fix this ...

Ugh. Evaluating `.. ? .. : ..' or `.. || ..' or `.. && ..' set noeval
for the unused part but it still called getnparam() and the like.

Bye
 Sven

diff -ru ../z.old/Src/init.c Src/init.c
--- ../z.old/Src/init.c	Tue Mar 14 16:50:21 2000
+++ Src/init.c	Wed Mar 15 10:04:53 2000
@@ -568,6 +568,9 @@
 
     init_eprog();
 
+    zero_mnumber.type = MN_INTEGER;
+    zero_mnumber.u.l = 0;
+
     getkeyptr = NULL;
 
     lineno = 1;
diff -ru ../z.old/Src/math.c Src/math.c
--- ../z.old/Src/math.c	Tue Mar 14 16:50:22 2000
+++ Src/math.c	Wed Mar 15 10:04:07 2000
@@ -37,6 +37,11 @@
 /**/
 int noeval;
  
+/* integer zero */
+
+/**/
+mnumber zero_mnumber;
+
 /* last input base we used */
 
 /**/
@@ -1018,13 +1023,13 @@
 	    push(yyval, NULL);
 	    break;
 	case ID:
-	    push(getnparam(yylval), yylval);
+	    push((noeval ? zero_mnumber : getnparam(yylval)), yylval);
 	    break;
 	case CID:
-	    push(getcvar(yylval), yylval);
+	    push((noeval ? zero_mnumber : getcvar(yylval)), yylval);
 	    break;
 	case FUNC:
-	    push(callmathfunc(yylval), yylval);
+	    push((noeval ? zero_mnumber : callmathfunc(yylval)), yylval);
 	    break;
 	case M_INPAR:
 	    mathparse(TOPPREC);

--
Sven Wischnowsky                         wischnow@informatik.hu-berlin.de

Re: PATCH: Re: `foo=foo; (( foo ))' => infinite recursion

[Bart Schaefer]

On Mar 14,  9:05am, Sven Wischnowsky wrote:
} Subject: PATCH: Re: `foo=foo; (( foo ))' => infinite recursion
}
} 
} Bart Schaefer wrote:
} 
} > On Mar 13, 10:34am, Sven Wischnowsky wrote:
} > } Subject: Re: `foo=foo; (( foo ))' => infinite recursion
} > }
} > } What really irritated me was that getnumvalue() called matheval().
} > 
} > Maybe just a recursion counter with a reasonably large limit?
} 
} Ok.

Something is still too aggressive:

zagzig% foo='x > 0 ? (--x, y += foo) : 1'
zagzig% x=3
zagzig% echo $[foo]
zsh: math recursion limit exceeded

`foo' should have been evaluated at most three times, no?

Maybe there's no real reason to fix this ...

-- 
Bart Schaefer                                 Brass Lantern Enterprises
http://www.well.com/user/barts              http://www.brasslantern.com

PATCH: Re: `foo=foo; (( foo ))' => infinite recursion

[Sven Wischnowsky]

Bart Schaefer wrote:

> On Mar 13, 10:34am, Sven Wischnowsky wrote:
> } Subject: Re: `foo=foo; (( foo ))' => infinite recursion
> }
> } What really irritated me was that getnumvalue() called matheval().
> } That meant that in cases like the one above $foo could contain any
> } mathematical expression and that would get evaluated.
> 
> Urgh.  This is ugly, but appears to have been done for a long time, so
> I'm a bit leery of changing it.  (( foo == $foo )) is probably meant to
> be true, which won't be the case any more with your patch.
> 
> Maybe just a recursion counter with a reasonably large limit?

Ok. The patch is relative to my last one for this. If you haven't
applied that, just ignore anything but the first two hunks.

Bye
 Sven

diff -ru ../z.old/Src/math.c Src/math.c
--- ../z.old/Src/math.c	Mon Mar 13 17:49:30 2000
+++ Src/math.c	Tue Mar 14 09:03:54 2000
@@ -47,6 +47,8 @@
 static mnumber yyval;
 static char *yylval;
 
+#define MAX_MLEVEL 256
+
 static int mlevel = 0;
 
 /* != 0 means recognize unary plus, minus, etc. */
@@ -861,6 +863,14 @@
     struct mathvalue *xstack = 0, nstack[STACKSZ];
     mnumber ret;
 
+    if (mlevel >= MAX_MLEVEL) {
+	xyyval.type = MN_INTEGER;
+	xyyval.u.l = 0;
+
+	zerr("math recursion limit exceeded", NULL, 0);
+
+	return xyyval;
+    }
     if (mlevel++) {
 	xlastbase = lastbase;
 	xnoeval = noeval;
@@ -948,78 +958,6 @@
 	(*ss)--;
     mtok = xmtok;
     return (x.type & MN_FLOAT) ? (zlong)x.u.d : x.u.l;
-}
-
-/**/
-mod_export mnumber
-mathnumber(char *s)
-{
-    mnumber ret;
-
-    ret.type = MN_INTEGER;
-
-    while (*s) {
-	switch (*s++) {
-	case '[':
-	    {
-		int base = zstrtol(s, &s, 10);
-
-		if (*s == ']')
-		    s++;
-		ret.u.l = zstrtol(s, &s, base);
-		return ret;
-	    }
-	case ' ':
-	case '\t':
-	case '\n':
-	    break;
-	case '0':
-	    if (*s == 'x' || *s == 'X') {
-		/* Should we set lastbase here? */
-		ret.u.l = zstrtol(++s, &s, 16);
-		return ret;
-	    }
-	/* Fall through! */
-	default:
-	    if (idigit(*--s) || *s == '.') {
-		char *nptr;
-#ifdef USE_LOCALE
-		char *prev_locale;
-#endif
-		for (nptr = s; idigit(*nptr); nptr++);
-
-		if (*nptr == '.' || *nptr == 'e' || *nptr == 'E') {
-		    /* it's a float */
-		    ret.type = MN_FLOAT;
-#ifdef USE_LOCALE
-		    prev_locale = setlocale(LC_NUMERIC, NULL);
-		    setlocale(LC_NUMERIC, "POSIX");
-#endif
-		    ret.u.d = strtod(s, &nptr);
-#ifdef USE_LOCALE
-		    setlocale(LC_NUMERIC, prev_locale);
-#endif
-		    if (s == nptr || *nptr == '.')
-			goto end;
-		    s = nptr;
-		} else {
-		    /* it's an integer */
-		    ret.u.l = zstrtol(s, &s, 10);
-
-		    if (*s == '#')
-			ret.u.l = zstrtol(++s, &s, ret.u.l);
-		}
-		return ret;
-	    }
-	    goto end;
-	}
-    }
- end:
-
-    ret.type = MN_INTEGER;
-    ret.u.l = 0;
-
-    return ret;
 }
 
 /*
diff -ru ../z.old/Src/params.c Src/params.c
--- ../z.old/Src/params.c	Mon Mar 13 17:49:31 2000
+++ Src/params.c	Tue Mar 14 09:03:55 2000
@@ -1420,7 +1420,7 @@
 	mn.type = MN_FLOAT;
 	mn.u.d = v->pm->gets.ffn(v->pm);
     } else
-	return mathnumber(getstrvalue(v));
+	return matheval(getstrvalue(v));
     return mn;
 }
 

--
Sven Wischnowsky                         wischnow@informatik.hu-berlin.de