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>

26 files changed, 131 insertions, 24 deletions

diff --git a/shell/ash_test/ash-arith/arith-ternary-assign.right b/shell/ash_test/ash-arith/arith-ternary-assign.right
new file mode 100644
index 000000000..6644d86bf
--- /dev/null
+++ b/shell/ash_test/ash-arith/arith-ternary-assign.right
@@ -0,0 +1 @@
+42:42
diff --git a/shell/ash_test/ash-arith/arith-ternary-assign.tests b/shell/ash_test/ash-arith/arith-ternary-assign.tests
new file mode 100755
index 000000000..fa18fe7b9
--- /dev/null
+++ b/shell/ash_test/ash-arith/arith-ternary-assign.tests
@@ -0,0 +1,3 @@
+exec 2>&1
+a='@'
+echo 42:$((a=1?42:3,a))
diff --git a/shell/ash_test/ash-arith/arith-ternary-comma.right b/shell/ash_test/ash-arith/arith-ternary-comma.right
new file mode 100644
index 000000000..6644d86bf
--- /dev/null
+++ b/shell/ash_test/ash-arith/arith-ternary-comma.right
@@ -0,0 +1 @@
+42:42
diff --git a/shell/ash_test/ash-arith/arith-ternary-comma.tests b/shell/ash_test/ash-arith/arith-ternary-comma.tests
new file mode 100755
index 000000000..5e05b58c4
--- /dev/null
+++ b/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))
diff --git a/shell/ash_test/ash-arith/arith-ternary-preincr.right b/shell/ash_test/ash-arith/arith-ternary-preincr.right
new file mode 100644
index 000000000..6644d86bf
--- /dev/null
+++ b/shell/ash_test/ash-arith/arith-ternary-preincr.right
@@ -0,0 +1 @@
+42:42
diff --git a/shell/ash_test/ash-arith/arith-ternary-preincr.tests b/shell/ash_test/ash-arith/arith-ternary-preincr.tests
new file mode 100755
index 000000000..3985c7079
--- /dev/null
+++ b/shell/ash_test/ash-arith/arith-ternary-preincr.tests
@@ -0,0 +1,3 @@
+exec 2>&1
+x='@'
+echo 42:$((1?42:++x))
diff --git a/shell/ash_test/ash-arith/arith-ternary3.right b/shell/ash_test/ash-arith/arith-ternary3.right
new file mode 100644
index 000000000..6644d86bf
--- /dev/null
+++ b/shell/ash_test/ash-arith/arith-ternary3.right
@@ -0,0 +1 @@
+42:42
diff --git a/shell/ash_test/ash-arith/arith-ternary3.tests b/shell/ash_test/ash-arith/arith-ternary3.tests
new file mode 100755
index 000000000..0bf9f3002
--- /dev/null
+++ b/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))
diff --git a/shell/ash_test/ash-arith/arith-ternary_nested3.right b/shell/ash_test/ash-arith/arith-ternary_nested3.right
new file mode 100644
index 000000000..1a34fde65
--- /dev/null
+++ b/shell/ash_test/ash-arith/arith-ternary_nested3.right
@@ -0,0 +1,2 @@
+42:42
+a=2:2
diff --git a/shell/ash_test/ash-arith/arith-ternary_nested3.tests b/shell/ash_test/ash-arith/arith-ternary_nested3.tests
new file mode 100755
index 000000000..b69dcc6e9
--- /dev/null
+++ b/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"
diff --git a/shell/hush_test/hush-arith/arith-ternary-assign.right b/shell/hush_test/hush-arith/arith-ternary-assign.right
new file mode 100644
index 000000000..6644d86bf
--- /dev/null
+++ b/shell/hush_test/hush-arith/arith-ternary-assign.right
@@ -0,0 +1 @@
+42:42
diff --git a/shell/hush_test/hush-arith/arith-ternary-assign.tests b/shell/hush_test/hush-arith/arith-ternary-assign.tests
new file mode 100755
index 000000000..fa18fe7b9
--- /dev/null
+++ b/shell/hush_test/hush-arith/arith-ternary-assign.tests
@@ -0,0 +1,3 @@
+exec 2>&1
+a='@'
+echo 42:$((a=1?42:3,a))
diff --git a/shell/hush_test/hush-arith/arith-ternary-comma.right b/shell/hush_test/hush-arith/arith-ternary-comma.right
new file mode 100644
index 000000000..6644d86bf
--- /dev/null
+++ b/shell/hush_test/hush-arith/arith-ternary-comma.right
@@ -0,0 +1 @@
+42:42
diff --git a/shell/hush_test/hush-arith/arith-ternary-comma.tests b/shell/hush_test/hush-arith/arith-ternary-comma.tests
new file mode 100755
index 000000000..5e05b58c4
--- /dev/null
+++ b/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))
diff --git a/shell/hush_test/hush-arith/arith-ternary-preincr.right b/shell/hush_test/hush-arith/arith-ternary-preincr.right
new file mode 100644
index 000000000..6644d86bf
--- /dev/null
+++ b/shell/hush_test/hush-arith/arith-ternary-preincr.right
@@ -0,0 +1 @@
+42:42
diff --git a/shell/hush_test/hush-arith/arith-ternary-preincr.tests b/shell/hush_test/hush-arith/arith-ternary-preincr.tests
new file mode 100755
index 000000000..3985c7079
--- /dev/null
+++ b/shell/hush_test/hush-arith/arith-ternary-preincr.tests
@@ -0,0 +1,3 @@
+exec 2>&1
+x='@'
+echo 42:$((1?42:++x))
diff --git a/shell/hush_test/hush-arith/arith-ternary1.right b/shell/hush_test/hush-arith/arith-ternary1.right
new file mode 100644
index 000000000..6b751d7b8
--- /dev/null
+++ b/shell/hush_test/hush-arith/arith-ternary1.right
@@ -0,0 +1,2 @@
+42:42
+a=0
diff --git a/shell/hush_test/hush-arith/arith-ternary1.tests b/shell/hush_test/hush-arith/arith-ternary1.tests
new file mode 100755
index 000000000..3532ce54d
--- /dev/null
+++ b/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"
diff --git a/shell/hush_test/hush-arith/arith-ternary2.right b/shell/hush_test/hush-arith/arith-ternary2.right
new file mode 100644
index 000000000..a549b1b5c
--- /dev/null
+++ b/shell/hush_test/hush-arith/arith-ternary2.right
@@ -0,0 +1,3 @@
+6:6
+a=b=+err+
+b=6
diff --git a/shell/hush_test/hush-arith/arith-ternary2.tests b/shell/hush_test/hush-arith/arith-ternary2.tests
new file mode 100755
index 000000000..cb3163932
--- /dev/null
+++ b/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"
diff --git a/shell/hush_test/hush-arith/arith-ternary3.right b/shell/hush_test/hush-arith/arith-ternary3.right
new file mode 100644
index 000000000..6644d86bf
--- /dev/null
+++ b/shell/hush_test/hush-arith/arith-ternary3.right
@@ -0,0 +1 @@
+42:42
diff --git a/shell/hush_test/hush-arith/arith-ternary3.tests b/shell/hush_test/hush-arith/arith-ternary3.tests
new file mode 100755
index 000000000..0bf9f3002
--- /dev/null
+++ b/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))
diff --git a/shell/hush_test/hush-arith/arith-ternary_nested3.right b/shell/hush_test/hush-arith/arith-ternary_nested3.right
new file mode 100644
index 000000000..1a34fde65
--- /dev/null
+++ b/shell/hush_test/hush-arith/arith-ternary_nested3.right
@@ -0,0 +1,2 @@
+42:42
+a=2:2
diff --git a/shell/hush_test/hush-arith/arith-ternary_nested3.tests b/shell/hush_test/hush-arith/arith-ternary_nested3.tests
new file mode 100755
index 000000000..b69dcc6e9
--- /dev/null
+++ b/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"
diff --git a/shell/math.c b/shell/math.c
index a398bcb98..b1aabef9d 100644
--- a/shell/math.c
+++ b/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 */
-                        numstackptr->var_name = alloca(var_name_size);
-                        safe_strncpy(numstackptr->var_name, expr, var_name_size);
-                        dbg("[%d] var:'%s'", (int)(numstackptr - numstack), numstackptr->var_name);
-                        expr = skip_whitespace(p);
-                        /* If it is not followed by "=" operator... */
-                        if (expr[0] != '=' /* not "=..." */
-                         || expr[1] == '=' /* or "==..." */
-                        ) {
-                                /* Evaluate variable to value */
-                                errmsg = arith_lookup_val(math_state, numstackptr);
-                                if (errmsg)
-                                        goto err_with_custom_msg;
+                        if (!math_state->evaluation_disabled) {
+                                size_t var_name_size = (p - expr) + 1;  /* +1 for NUL */
+                                numstackptr->var_name = alloca(var_name_size);
+                                safe_strncpy(numstackptr->var_name, expr, var_name_size);
+                                dbg("[%d] var:'%s'", (int)(numstackptr - numstack), numstackptr->var_name);
+                                expr = skip_whitespace(p);
+                                /* If it is not followed by "=" operator... */
+                                if (expr[0] != '=' /* not "=..." */
+                                 || expr[1] == '=' /* or "==..." */
+                                ) {
+                                        /* Evaluate variable to value */
+                                        errmsg = arith_lookup_val(math_state, numstackptr);
+                                        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
-                         * N: loop to parse the rest of string
+                         *     C:if op is "?": check result, set disable flag if needed
+                         * 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);
 }
 
diff --git a/shell/math.h b/shell/math.h
index 41ef6e8df..452ddaddd 100644
--- a/shell/math.h
+++ b/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;