* Arith expansion accepts extra closing parenthesis @ 2015-06-01 16:32 Martijn Dekker 2015-06-02 8:56 ` Peter Stephenson 2015-06-02 17:58 ` Bart Schaefer 0 siblings, 2 replies; 5+ messages in thread From: Martijn Dekker @ 2015-06-01 16:32 UTC (permalink / raw) To: zsh-workers I just found some more arithmetic parsing strangeness. zsh 5.0.8 and 4.3.11 both happily accept and ignore one (1) extra closing parenthesis in variable expansion within arithmetic expansion (even in POSIX mode). % X='1)' % echo $(($X)) 1 % echo $((X)) 1 (Expected output: error message in both cases) Yet, 'echo $((1)))' errors out as expected, as do 'echo $(($X))' and 'echo $((X))' for X='1))', X='1)))', X='(1', etc. - Martijn ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Arith expansion accepts extra closing parenthesis 2015-06-01 16:32 Arith expansion accepts extra closing parenthesis Martijn Dekker @ 2015-06-02 8:56 ` Peter Stephenson 2015-06-02 9:14 ` Peter Stephenson 2015-06-02 17:58 ` Bart Schaefer 1 sibling, 1 reply; 5+ messages in thread From: Peter Stephenson @ 2015-06-02 8:56 UTC (permalink / raw) To: zsh-workers On Mon, 1 Jun 2015 18:32:14 +0200 Martijn Dekker <martijn@inlv.org> wrote: > I just found some more arithmetic parsing strangeness. zsh 5.0.8 and > 4.3.11 both happily accept and ignore one (1) extra closing parenthesis > in variable expansion within arithmetic expansion (even in POSIX mode). > > % X='1)' > % echo $(($X)) > 1 > % echo $((X)) > 1 I think this is a special case, down to the way we handle parenthesesed expressions --- we treat it as if it's a top-level expression but expect a closing parenthesis next. So checking for a stray closing parenthesis on return from the top level of parsing ought to be a robust fix. pws diff --git a/Src/math.c b/Src/math.c index 97a97b3..e20a90c 100644 --- a/Src/math.c +++ b/Src/math.c @@ -407,6 +407,13 @@ mathevall(char *s, enum prec_type prec_tp, char **ep) stack[0].val.type = MN_INTEGER; stack[0].val.u.l = 0; mathparse(prec_tp == MPREC_TOP ? TOPPREC : ARGPREC); + /* + * Internally, we parse the contents of parentheses at top + * precedence... so we can return a parenthesis here if + * there are too many at the end. + */ + if (mtok == M_OUTPAR && !errflag) + zerr("unexpected ')'"); *ep = ptr; DPUTS(!errflag && sp > 0, "BUG: math: wallabies roaming too freely in outback"); diff --git a/Test/C01arith.ztst b/Test/C01arith.ztst index d284e08..2d35ea6 100644 --- a/Test/C01arith.ztst +++ b/Test/C01arith.ztst @@ -395,3 +395,17 @@ >6 >7 >120 + + foo="(1)" + print $((foo)) + print $(($foo)) + print $(((2))) + foo="3)" + (print $((foo))) 2>&1 + (print $(($foo))) 2>&1 +1: Good and bad trailing parentheses +>1 +>1 +>2 +>(eval):6: unexpected ')' +>(eval):7: unexpected ')' ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Arith expansion accepts extra closing parenthesis 2015-06-02 8:56 ` Peter Stephenson @ 2015-06-02 9:14 ` Peter Stephenson 0 siblings, 0 replies; 5+ messages in thread From: Peter Stephenson @ 2015-06-02 9:14 UTC (permalink / raw) To: zsh-workers Some of the errors from arithmetic evaluation are a bit cryptic in that it's not obvious the failure reported is actually to do with arithmetic at all: "unexpected ')'" could refer to lots of uses of ')'. This improves the previous patch so that more messages indicate where the error has come from. I've left alone those that don't seem as cryptic. I'm glad I've never been "out of integers". That sounds bad. pws diff --git a/Src/math.c b/Src/math.c index 97a97b3..977e923 100644 --- a/Src/math.c +++ b/Src/math.c @@ -407,6 +407,13 @@ mathevall(char *s, enum prec_type prec_tp, char **ep) stack[0].val.type = MN_INTEGER; stack[0].val.u.l = 0; mathparse(prec_tp == MPREC_TOP ? TOPPREC : ARGPREC); + /* + * Internally, we parse the contents of parentheses at top + * precedence... so we can return a parenthesis here if + * there are too many at the end. + */ + if (mtok == M_OUTPAR && !errflag) + zerr("bad math expression: unexpected ')'"); *ep = ptr; DPUTS(!errflag && sp > 0, "BUG: math: wallabies roaming too freely in outback"); @@ -791,7 +798,7 @@ zzlex(void) ptr++; if (!*ptr) { - zerr("character missing after ##"); + zerr("bad math expression: character missing after ##"); return EOI; } ptr = getkeystring(ptr, NULL, GETKEYS_MATH, &v); @@ -914,7 +921,7 @@ setmathvar(struct mathvalue *mvp, mnumber v) mvp->pval = NULL; } if (!mvp->lval) { - zerr("lvalue required"); + zerr("bad math expression: lvalue required"); v.type = MN_INTEGER; v.u.l = 0; return v; @@ -1256,7 +1263,7 @@ op(int what) /* Error if (-num ** b) and b is not an integer */ double tst = (double)(zlong)b.u.d; if (tst != b.u.d) { - zerr("imaginary power"); + zerr("bad math expression: imaginary power"); return; } } @@ -1338,7 +1345,7 @@ op(int what) push(((a.type & MN_FLOAT) ? a.u.d : a.u.l) ? b : c, NULL, 0); break; case COLON: - zerr("':' without '?'"); + zerr("bad math expression: ':' without '?'"); break; case PREPLUS: if (spval->type & MN_FLOAT) @@ -1355,7 +1362,7 @@ op(int what) setmathvar(stack + sp, *spval); break; default: - zerr("out of integers"); + zerr("bad math expression: out of integers"); return; } } @@ -1525,7 +1532,7 @@ mathparse(int pc) mathparse(TOPPREC); if (mtok != M_OUTPAR) { if (!errflag) - zerr("')' expected"); + zerr("bad math expression: ')' expected"); return; } break; @@ -1543,7 +1550,7 @@ mathparse(int pc) noeval--; if (mtok != COLON) { if (!errflag) - zerr("':' expected"); + zerr("bad math expression: ':' expected"); return; } if (q) diff --git a/Test/C01arith.ztst b/Test/C01arith.ztst index d284e08..2d35ea6 100644 --- a/Test/C01arith.ztst +++ b/Test/C01arith.ztst @@ -395,3 +395,17 @@ >6 >7 >120 + + foo="(1)" + print $((foo)) + print $(($foo)) + print $(((2))) + foo="3)" + (print $((foo))) 2>&1 + (print $(($foo))) 2>&1 +1: Good and bad trailing parentheses +>1 +>1 +>2 +>(eval):6: unexpected ')' +>(eval):7: unexpected ')' ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Arith expansion accepts extra closing parenthesis 2015-06-01 16:32 Arith expansion accepts extra closing parenthesis Martijn Dekker 2015-06-02 8:56 ` Peter Stephenson @ 2015-06-02 17:58 ` Bart Schaefer 2015-06-02 18:10 ` Bart Schaefer 1 sibling, 1 reply; 5+ messages in thread From: Bart Schaefer @ 2015-06-02 17:58 UTC (permalink / raw) To: zsh-workers On Jun 1, 6:32pm, Martijn Dekker wrote: } Subject: Arith expansion accepts extra closing parenthesis } } I just found some more arithmetic parsing strangeness. zsh 5.0.8 and } 4.3.11 both happily accept and ignore one (1) extra closing parenthesis } } % X='1)' } % echo $((X)) } 1 Easier way to see this: % let "1)" % It appears mathparse() around lines 1566-1568 of math.c consumes one token of lookahead with zzlex() and inadvertently discards it when it turns out to be a close-paren. This is partly because of the pseuedo- precedence assigned to M_OUTPAR "for convenience". I'm not sure what the correct fix is; maybe checkunary() should throw an error on M_OUTPAR? ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Arith expansion accepts extra closing parenthesis 2015-06-02 17:58 ` Bart Schaefer @ 2015-06-02 18:10 ` Bart Schaefer 0 siblings, 0 replies; 5+ messages in thread From: Bart Schaefer @ 2015-06-02 18:10 UTC (permalink / raw) To: zsh-workers Oops, sorry for the delayed reaction email on this -- I was AFK all day yesterday and didn't notice that there had been other replies. ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2015-06-02 18:10 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-06-01 16:32 Arith expansion accepts extra closing parenthesis Martijn Dekker 2015-06-02 8:56 ` Peter Stephenson 2015-06-02 9:14 ` Peter Stephenson 2015-06-02 17:58 ` Bart Schaefer 2015-06-02 18:10 ` Bart Schaefer
Code repositories for project(s) associated with this public inbox https://git.vuxu.org/mirror/zsh/ This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).