shell/math: fix ?: to not evaluate not-taken branches

This fixes ash-arith-arith-ternary1/2.tests

function                                             old     new   delta
evaluate_string                                     1271    1432    +161
arith_apply                                          968    1000     +32
arith                                                 22      36     +14
------------------------------------------------------------------------------
(add/remove: 0/0 grow/shrink: 3/0 up/down: 207/0)             Total: 207 bytes

Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com>

shell/ash_test/ash-arith/arith-ternary-assign.right

@@ -0,0 +1 @@
+42:42

shell/ash_test/ash-arith/arith-ternary-assign.tests

@@ -0,0 +1,3 @@
+exec 2>&1
+a='@'
+echo 42:$((a=1?42:3,a))

shell/ash_test/ash-arith/arith-ternary-comma.right

@@ -0,0 +1 @@
+42:42

shell/ash_test/ash-arith/arith-ternary-comma.tests

@@ -0,0 +1,3 @@
+exec 2>&1
+x='@'
+echo 42:$((1?4:x,20*2+2))

shell/ash_test/ash-arith/arith-ternary-preincr.right

@@ -0,0 +1 @@
+42:42

shell/ash_test/ash-arith/arith-ternary-preincr.tests

@@ -0,0 +1,3 @@
+exec 2>&1
+x='@'
+echo 42:$((1?42:++x))

shell/ash_test/ash-arith/arith-ternary3.right

@@ -0,0 +1 @@
+42:42

shell/ash_test/ash-arith/arith-ternary3.tests

@@ -0,0 +1,4 @@
+exec 2>&1
+# "EXPR ?..." should check _evaluated_ EXPR,
+# not its last value
+echo 42:$((1 < 1 ? -1 : 1 > 1 ? 1 : 42))

shell/ash_test/ash-arith/arith-ternary_nested3.right

@@ -0,0 +1,2 @@
+42:42
+a=2:2

shell/ash_test/ash-arith/arith-ternary_nested3.tests

@@ -0,0 +1,6 @@
+exec 2>&1
+x='@'
+a=2
+# After processing nested ?:, outermost ?: should still rememeber to NOT evaluate a*=2
+echo 42:$((1?0?41:42:(a*=2)))
+echo "a=2:$a"

shell/hush_test/hush-arith/arith-ternary-assign.right

@@ -0,0 +1 @@
+42:42

shell/hush_test/hush-arith/arith-ternary-assign.tests

@@ -0,0 +1,3 @@
+exec 2>&1
+a='@'
+echo 42:$((a=1?42:3,a))

shell/hush_test/hush-arith/arith-ternary-comma.right

@@ -0,0 +1 @@
+42:42

shell/hush_test/hush-arith/arith-ternary-comma.tests

@@ -0,0 +1,3 @@
+exec 2>&1
+x='@'
+echo 42:$((1?4:x,20*2+2))

shell/hush_test/hush-arith/arith-ternary-preincr.right

@@ -0,0 +1 @@
+42:42

shell/hush_test/hush-arith/arith-ternary-preincr.tests

@@ -0,0 +1,3 @@
+exec 2>&1
+x='@'
+echo 42:$((1?42:++x))

shell/hush_test/hush-arith/arith-ternary1.right

@@ -0,0 +1,2 @@
+42:42
+a=0

shell/hush_test/hush-arith/arith-ternary1.tests

@@ -0,0 +1,5 @@
+exec 2>&1
+a=0
+# The not-taken branch should not evaluate
+echo 42:$((1 ? 42 : (a+=2)))
+echo "a=$a"

shell/hush_test/hush-arith/arith-ternary2.right

@@ -0,0 +1,3 @@
+6:6
+a=b=+err+
+b=6

shell/hush_test/hush-arith/arith-ternary2.tests

@@ -0,0 +1,7 @@
+exec 2>&1
+a='b=+err+'
+b=5
+# The not-taken branch should not parse variables
+echo 6:$((0 ? a : ++b))
+echo "a=$a"
+echo "b=$b"

shell/hush_test/hush-arith/arith-ternary3.right

@@ -0,0 +1 @@
+42:42

shell/hush_test/hush-arith/arith-ternary3.tests

@@ -0,0 +1,4 @@
+exec 2>&1
+# "EXPR ?..." should check _evaluated_ EXPR,
+# not its last value
+echo 42:$((1 < 1 ? -1 : 1 > 1 ? 1 : 42))

shell/hush_test/hush-arith/arith-ternary_nested3.right

@@ -0,0 +1,2 @@
+42:42
+a=2:2

shell/hush_test/hush-arith/arith-ternary_nested3.tests

@@ -0,0 +1,6 @@
+exec 2>&1
+x='@'
+a=2
+# After processing nested ?:, outermost ?: should still rememeber to NOT evaluate a*=2
+echo 42:$((1?0?41:42:(a*=2)))
+echo "a=2:$a"

shell/math.c

@@ -356,6 +356,11 @@ arith_apply(arith_state_t *math_state, operator op, var_or_num_t *numstack, var_
 		NUMPTR = top_of_stack; /* this decrements NUMPTR */
 		top_of_stack--; /* now points to left side */
 
+		if (math_state->evaluation_disabled) {
+			dbg("binary op %02x skipped", op);
+			goto ret_NULL;
+		}
+
 		right_side_val = rez;
 		rez = top_of_stack->val;
 		if (op == TOK_BOR || op == TOK_OR_ASSIGN)
@@ -428,6 +433,11 @@ arith_apply(arith_state_t *math_state, operator op, var_or_num_t *numstack, var_
 		}
 	}
 
+	if (math_state->evaluation_disabled) {
+		dbg("unary op %02x skipped", op);
+		goto ret_NULL;
+	}
+
 	if (is_assign_op(op)) {
 		char buf[sizeof(arith_t)*3 + 2];
 
@@ -446,6 +456,7 @@ arith_apply(arith_state_t *math_state, operator op, var_or_num_t *numstack, var_
 	}
 
 	top_of_stack->val = rez;
+ ret_NULL:
 	/* Erase var name, it is just a number now */
 	top_of_stack->var_name = NULL;
 	return NULL;
@@ -594,8 +605,10 @@ static arith_t strto_arith_t(const char *nptr, char **endptr)
 static arith_t
 evaluate_string(arith_state_t *math_state, const char *expr)
 {
+#define EVAL_DISABLED ((unsigned long long)math_state->evaluation_disabled)
+#define TOP_BIT_ULL ((unsigned long long)LLONG_MAX + 1)
 	operator lasttok;
-	const char *errmsg;
+	const char *errmsg = NULL;
 	const char *start_expr = expr = skip_whitespace(expr);
 	unsigned expr_len = strlen(expr) + 2;
 	/* Stack of integers/names */
@@ -614,7 +627,6 @@ evaluate_string(arith_state_t *math_state, const char *expr)
 	/* Start with a left paren */
 	dbg("(%d) op:TOK_LPAREN", (int)(opstackptr - opstack));
 	*opstackptr++ = lasttok = TOK_LPAREN;
-	errmsg = NULL;
 
 	while (1) {
 		const char *p;
@@ -653,19 +665,26 @@ evaluate_string(arith_state_t *math_state, const char *expr)
 		p = endofname(expr);
 		if (p != expr) {
 			/* Name */
-			size_t var_name_size = (p - expr) + 1;  /* +1 for NUL */
+			if (!math_state->evaluation_disabled) {
-			numstackptr->var_name = alloca(var_name_size);
+				size_t var_name_size = (p - expr) + 1;  /* +1 for NUL */
-			safe_strncpy(numstackptr->var_name, expr, var_name_size);
+				numstackptr->var_name = alloca(var_name_size);
-			dbg("[%d] var:'%s'", (int)(numstackptr - numstack), numstackptr->var_name);
+				safe_strncpy(numstackptr->var_name, expr, var_name_size);
-			expr = skip_whitespace(p);
+				dbg("[%d] var:'%s'", (int)(numstackptr - numstack), numstackptr->var_name);
-			/* If it is not followed by "=" operator... */
+				expr = skip_whitespace(p);
-			if (expr[0] != '=' /* not "=..." */
+				/* If it is not followed by "=" operator... */
-			 || expr[1] == '=' /* or "==..." */
+				if (expr[0] != '=' /* not "=..." */
-			) {
+				 || expr[1] == '=' /* or "==..." */
-				/* Evaluate variable to value */
+				) {
-				errmsg = arith_lookup_val(math_state, numstackptr);
+					/* Evaluate variable to value */
-				if (errmsg)
+					errmsg = arith_lookup_val(math_state, numstackptr);
-					goto err_with_custom_msg;
+					if (errmsg)
+						goto err_with_custom_msg;
+				}
+			} else {
+				dbg("[%d] var:IGNORED", (int)(numstackptr - numstack));
+				numstackptr->var_name = NULL;
+				numstackptr->val = 0; //paranoia, probably not needed
+				expr = p;
 			}
  push_num:
 			numstackptr++;
@@ -814,10 +833,11 @@ evaluate_string(arith_state_t *math_state, const char *expr)
 			 *         pop prev_op
 			 *         if can't evaluate prev_op (it is lower precedence than op):
 			 *             push prev_op back
-			 *             goto P
+			 *             goto C
 			 *         evaluate prev_op on top of numstack
-			 * P: push op
+			 *     C:if op is "?": check result, set disable flag if needed
-			 * N: loop to parse the rest of string
+			 * push op
+			 * N:loop to parse the rest of string
 			 */
 			while (opstackptr != opstack) {
 				operator prev_op = *--opstackptr;
@@ -840,7 +860,7 @@ evaluate_string(arith_state_t *math_state, const char *expr)
 					) {
 						/* ...x~y@. push @ on opstack */
 						opstackptr++; /* undo removal of ~ op */
-						goto push_op;
+						goto check_cond;
 					}
 					/* else: ...x~y@. Evaluate x~y, replace it on stack with result. Then repeat */
 				}
@@ -863,21 +883,41 @@ dbg("    numstack:%d val:%lld '%s'", (int)(numstackptr - numstack), numstackptr[
 						errmsg = "malformed ?: operator";
 						goto err_with_custom_msg;
 					}
+					/* Example: a=1?2:3,a. We just executed ":".
+					 * Prevent assignment from being still disabled.
+					 */
+					math_state->evaluation_disabled >>= 1;
+					dbg("':' executed: evaluation_disabled=%llx (restored)", EVAL_DISABLED);
 				}
 			} /* while (opstack not empty) */
 			if (op == TOK_RPAREN) /* unpaired RPAREN? */
 				goto err;
-		}
+ check_cond:
+			if (op == TOK_CONDITIONAL) {
+				/* We know the value of EXPR in "EXPR ? ..."
+				 * Should we stop evaluating now? */
+				if (math_state->evaluation_disabled & TOP_BIT_ULL)
+					goto err; /* >63 levels of ?: nesting not supported */
+				math_state->evaluation_disabled <<= 1;
+				if (numstackptr[-1].val == 0)
+					math_state->evaluation_disabled |= 1;
+				dbg("'?' entered: evaluation_disabled=%llx", EVAL_DISABLED);
+			}
+		} /* if */
 		/* else: LPAREN or UNARY: push it on opstack */
- push_op:
+
 		/* Push this operator to opstack */
 		dbg("(%d) op:%02x insert_op:%02x", (int)(opstackptr - opstack), op, insert_op);
 		*opstackptr++ = lasttok = op;
- next: ;
+ next:
 		if (insert_op != 0xff) {
 			op = insert_op;
 			insert_op = 0xff;
 			dbg("inserting %02x", op);
+			if (op == TOK_CONDITIONAL_SEP) {
+				math_state->evaluation_disabled ^= 1;
+				dbg("':' entered: evaluation_disabled=%llx (negated)", EVAL_DISABLED);
+			}
 			goto tok_found1;
 		}
 	} /* while (1) */
@@ -896,6 +936,7 @@ arith(arith_state_t *math_state, const char *expr)
 {
 	math_state->errmsg = NULL;
 	math_state->list_of_recursed_names = NULL;
+	math_state->evaluation_disabled = 0;
 	return evaluate_string(math_state, expr);
 }
 

shell/math.h

@@ -73,13 +73,12 @@ typedef long arith_t;
 
 typedef const char* FAST_FUNC (*arith_var_lookup_t)(const char *name);
 typedef void        FAST_FUNC (*arith_var_set_t)(const char *name, const char *val);
-//typedef const char* FAST_FUNC (*arith_var_endofname_t)(const char *name);
 
 typedef struct arith_state_t {
 	const char           *errmsg;
 	arith_var_lookup_t    lookupvar;
 	arith_var_set_t       setvar;
-//	arith_var_endofname_t endofname;
+	uint64_t              evaluation_disabled;
 	void                 *list_of_recursed_names;
 } arith_state_t;