Skip to contentSkip to conversation

dash arithmetic expression bug (1 << 1 + 1 | 1)

DISCUSSION Herbert Xu 1 participant 2 messages Mar 9, 2010 – Mar 10, 2010 2 messages not in the archive

lore
Share & tools
lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

Herbert Xu

author
#1 lore raw
Quoted 8 lines · Jilles Tjoelker

Noncommutative operators break more easily, for example Input:

    echo $((3 - 3 * 3 - 3))
Expected result:
    -9
Actual result:
    -3

This should fix it:

commit 9655c1ac5646bde1007ecba7c6271d3aa98f294b

Author: Herbert Xu <[email protected]>
Date:   Tue Mar 9 12:52:30 2010 +0800

[ARITH] Fix binary operator parsing

Jilles Tjoelker reported that binary operator parsing doesn't respect operator precedence correctly in the case where a lower- precedence operator is followed by a higher-precedence operator, and then by a lower-precedence operator.

This patch fixes this by stopping when we encounter a binary oeprator with a precedence lower than one that we have already encountered.

Signed-off-by Herbert Xu <[email protected]>
ChangeLog +4 -0
index 3e68917..8dfd747 100644
@@ -1,3 +1,7 @@
+2010-03-09  Herbert Xu <[email protected]>
+
+	* Fix binary operator parsing.
+
 2009-11-26  Herbert Xu <[email protected]>
 
 	* Fix off-by-one recordregion in readcmd.
src/arith_yacc.c +20 -10
index f4857fe..74b95f8 100644
@@ -74,6 +74,8 @@ static const char prec[ARITH_BINOP_MAX - ARITH_BINOP_MIN] = {
 	ARITH_PRECEDENCE(ARITH_BOR, 7),
 };
 
+#define ARITH_MAX_PREC 8
+
 static void yyerror(const char *s) __attribute__ ((noreturn));
 static void yyerror(const char *s)
 {

@@ -81,9 +83,14 @@ static void yyerror(const char *s)
 	/* NOTREACHED */
 }
 
+static inline int arith_prec(int op)
+{
+	return prec[op - ARITH_BINOP_MIN];
+}
+
 static inline int higher_prec(int op1, int op2)
 {
-	return prec[op1 - ARITH_BINOP_MIN] < prec[op2 - ARITH_BINOP_MIN];
+	return arith_prec(op1) < arith_prec(op2);
 }
 
 static intmax_t do_binop(int op, intmax_t a, intmax_t b)

@@ -174,7 +181,7 @@ again:
 	}
 }
 
-static intmax_t binop2(intmax_t a, int op, int noeval)
+static intmax_t binop2(intmax_t a, int op, int prec, int noeval)
 {
 	for (;;) {
 		union yystype val;

@@ -188,15 +195,18 @@ static intmax_t binop2(intmax_t a, int op, int noeval)
 		b = primary(token, &val, yylex(), noeval);
 
 		op2 = last_token;
-		if (op2 < ARITH_BINOP_MIN || op2 >= ARITH_BINOP_MAX)
-			return noeval ? b : do_binop(op, a, b);
-
-		if (higher_prec(op2, op)) {
-			b = binop2(b, op2, noeval);
-			return noeval ? b : do_binop(op, a, b);
+		if (op2 >= ARITH_BINOP_MIN && op2 < ARITH_BINOP_MAX &&
+		    higher_prec(op2, op)) {
+			b = binop2(b, op2, arith_prec(op), noeval);
+			op2 = last_token;
 		}
 
-		a = do_binop(op, a, b);
+		a = noeval ? b : do_binop(op, a, b);
+
+		if (op2 < ARITH_BINOP_MIN || op2 >= ARITH_BINOP_MAX ||
+		    arith_prec(op2) >= prec)
+			return a;
+
 		op = op2;
 	}
 }

@@ -209,7 +219,7 @@ static intmax_t binop(int token, union yystype *val, int op, int noeval)
 	if (op < ARITH_BINOP_MIN || op >= ARITH_BINOP_MAX)
 		return a;
 
-	return binop2(a, op, noeval);
+	return binop2(a, op, ARITH_MAX_PREC, noeval);
 }
 
 static intmax_t and(int token, union yystype *val, int op, int noeval)
Quoted 2 lines

Another change I'm making to the arith code is making || return 0 or 1 only, matching C, POSIX and other shells.

Please send a patch.

Thanks,

Signature · 4 lines
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

A message that is not in the archive (sent off-list or not yet received): [email protected]

author
#3 lore raw
Quoted 4 lines · Jilles Tjoelker

Apart from the compliance issue, it is also bad to expose implementation details like the exact meaning of 'noeval' to scripts such that they may come to depend on them.

Patch applied. Thanks a lot!

Signature · 4 lines
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt