3 files changed, 65 insertions, 28 deletions

diff --git a/shell/ash_test/ash-arith/arith-precedence1.right b/shell/ash_test/ash-arith/arith-precedence1.right
index 7f407b5d2..3f9320a13 100644
--- a/shell/ash_test/ash-arith/arith-precedence1.right
+++ b/shell/ash_test/ash-arith/arith-precedence1.right
@@ -1 +1,4 @@
 4:4
+4:4
+4:4
+4:4
diff --git a/shell/ash_test/ash-arith/arith-precedence1.tests b/shell/ash_test/ash-arith/arith-precedence1.tests
index 964ae4ee2..bfef05292 100755
--- a/shell/ash_test/ash-arith/arith-precedence1.tests
+++ b/shell/ash_test/ash-arith/arith-precedence1.tests
@@ -1,2 +1,15 @@
 exec 2>&1
+# bash documentation says that precedence order is:
+#  ...
+#  expr ? expr1 : expr2
+#  = *= /= %= += -= <<= >>= &= ^= |=
+#  exprA , exprB
+# but in practice, the rules for expr1 and expr2 are different:
+# assignments and commas in expr1 have higher precedence than :?,
+# but in expr2 they haven't:
+# "v ? 1,2 : 3,4" is parsed as "(v ? (1,2) : 3),4"
+# "v ? a=2 : b=4" is parsed as "(v ? (a=1) : b)=4" (thus, this is a syntax error)
+echo 4:$((0 ? 1,2 : 3,4))
+echo 4:$((1 ? 1,2 : 3,4))
 echo 4:"$((0 ? 1,2 : 3,4))"
+echo 4:"$((1 ? 1,2 : 3,4))"
diff --git a/shell/math.c b/shell/math.c
index 57eb56952..d5f3ce361 100644
--- a/shell/math.c
+++ b/shell/math.c
@@ -116,6 +116,12 @@
 #include "libbb.h"
 #include "math.h"
 
+#if 1
+# define dbg(...) ((void)0)
+#else
+# define dbg(...) bb_error_msg(__VA_ARGS__)
+#endif
+
 typedef unsigned char operator;
 
 /* An operator's token id is a bit of a bitfield. The lower 5 bits are the
@@ -151,6 +157,17 @@ typedef unsigned char operator;
 #define fix_assignment_prec(prec) do { if (prec == 3) prec = 2; } while (0)
 
 /* Ternary conditional operator is right associative too */
+// FIXME:
+// bash documentation says that precedence order is:
+//  ...
+//  expr ? expr1 : expr2
+//  = *= /= %= += -= <<= >>= &= ^= |=
+//  exprA , exprB
+// but in practice, the rules for expr1 and expr2 are different:
+// assignments and commas in expr1 have higher precedence than ?:,
+// but in expr2 they haven't:
+// "v ? 1,2 : 3,4" is parsed as "(v ? (1,2) : 3),4"
+// "v ? a=2 : b=4" is parsed as "(v ? (a=1) : b)=4" (thus, this is a syntax error)
 #define TOK_CONDITIONAL         tok_decl(4,0)
 #define TOK_CONDITIONAL_SEP     tok_decl(4,1)
 
@@ -246,7 +263,7 @@ typedef struct {
 } var_or_num_t;
 
 #define VALID_NAME(name) ((name) && (name) != SECOND_VAL_VALID)
-#define NOT_NAME(name) (!(name) || (name) == SECOND_VAL_VALID)
+#define NOT_NAME(name)   (!(name) || (name) == SECOND_VAL_VALID)
 
 
 typedef struct remembered_name {
@@ -624,11 +641,12 @@ evaluate_string(arith_state_t *math_state, const char *expr)
         var_or_num_t *const numstack = alloca((expr_len / 2) * sizeof(numstack[0]));
         var_or_num_t *numstackptr = numstack;
         /* Stack of operator tokens */
-        operator *const stack = alloca(expr_len * sizeof(stack[0]));
-        operator *stackptr = stack;
+        operator *const opstack = alloca(expr_len * sizeof(opstack[0]));
+        operator *opstackptr = opstack;
 
         /* Start with a left paren */
-        *stackptr++ = lasttok = TOK_LPAREN;
+        dbg("(%d) op:TOK_LPAREN", (int)(opstackptr - opstack));
+        *opstackptr++ = lasttok = TOK_LPAREN;
         errmsg = NULL;
 
         while (1) {
@@ -655,11 +673,11 @@ evaluate_string(arith_state_t *math_state, const char *expr)
                                  * and let the loop process it */
                                 expr = ptr_to_rparen;
 //bb_error_msg("expr=')'");
-                                continue;
+                                goto tok_find;
                         }
                         /* At this point, we're done with the expression */
                         if (numstackptr != numstack + 1) {
-                                /* ...but if there isn't, it's bad */
+                                /* if there is not exactly one result, it's bad */
                                 goto err;
                         }
                         goto ret;
@@ -671,9 +689,9 @@ evaluate_string(arith_state_t *math_state, const char *expr)
                         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);
-//bb_error_msg("var:'%s'", numstackptr->var);
+                        dbg("[%d] var:'%s'", (int)(numstackptr - numstack), numstackptr->var_name);
                         expr = p;
- num:
+ push_num:
                         numstackptr++;
                         lasttok = TOK_NUM;
                         continue;
@@ -684,6 +702,7 @@ evaluate_string(arith_state_t *math_state, const char *expr)
                         numstackptr->var_name = NULL;
                         errno = 0;
                         numstackptr->val = strto_arith_t(expr, (char**) &expr);
+                        dbg("[%d] val:%lld", (int)(numstackptr - numstack), numstackptr->val);
                         /* A number can't be followed by another number, or a variable name.
                          * We'd catch this later anyway, but this would require numstack[]
                          * to be twice as deep to handle strings where _every_ char is
@@ -691,10 +710,9 @@ evaluate_string(arith_state_t *math_state, const char *expr)
                          */
                         if (isalnum(*expr) || *expr == '_')
                                 goto err;
-//bb_error_msg("val:%lld", numstackptr->val);
                         if (errno)
                                 numstackptr->val = 0; /* bash compat */
-                        goto num;
+                        goto push_num;
                 }
 
                 /* Should be an operator */
@@ -715,14 +733,13 @@ evaluate_string(arith_state_t *math_state, const char *expr)
                                 char next = skip_whitespace(expr + 2)[0];
                                 if (!(isalpha(next) || next == '_')) {
                                         /* not a ++VAR */
-                                        //bb_error_msg("special %c%c", expr[0], expr[0]);
                                         op = (expr[0] == '+' ? TOK_ADD : TOK_SUB);
                                         expr++;
                                         goto tok_found1;
                                 }
                         }
                 }
-
+ tok_find:
                 p = op_tokens;
                 while (1) {
                         /* Compare expr to current op_tokens[] element */
@@ -799,13 +816,10 @@ evaluate_string(arith_state_t *math_state, const char *expr)
                         /* The algorithm employed here is simple: while we don't
                          * hit an open paren nor the bottom of the stack, pop
                          * tokens and apply them */
-                        while (stackptr != stack) {
-                                operator prev_op = *--stackptr;
+                        while (opstackptr != opstack) {
+                                operator prev_op = *--opstackptr;
                                 if (op == TOK_RPAREN) {
-//bb_error_msg("op == TOK_RPAREN");
                                         if (prev_op == TOK_LPAREN) {
-//bb_error_msg("prev_op == TOK_LPAREN");
-//bb_error_msg("  %p %p numstackptr[-1].var_name:'%s'", numstack, numstackptr-1, numstackptr[-1].var_name);
                                                 if (VALID_NAME(numstackptr[-1].var_name)) {
                                                         /* Expression is (var), lookup now */
                                                         errmsg = arith_lookup_val(math_state, &numstackptr[-1]);
@@ -819,31 +833,38 @@ evaluate_string(arith_state_t *math_state, const char *expr)
                                                 lasttok = TOK_NUM;
                                                 goto next;
                                         }
-//bb_error_msg("prev_op != TOK_LPAREN");
+                                        /* Not (y), but ...x~y): fall through to evaluate x~y */
                                 } else {
                                         operator prev_prec = PREC(prev_op);
-//bb_error_msg("op != TOK_RPAREN");
                                         fix_assignment_prec(prec);
                                         fix_assignment_prec(prev_prec);
                                         if (prev_prec < prec
                                          || (prev_prec == prec && is_right_associative(prec))
                                         ) {
-                                                stackptr++;
-                                                break;
+                                                /* ...x~y@: push @ on opstack */
+                                                opstackptr++; /* undo removal of ~ op */
+                                                goto push_op;
                                         }
+                                        /* ...x~y@: evaluate x~y, replace it on stack with result. Then repeat */
                                 }
-//bb_error_msg("arith_apply(prev_op:%02x)", prev_op);
+                                dbg("arith_apply(prev_op:%02x, numstack:%d)", prev_op, (int)(numstackptr - numstack));
                                 errmsg = arith_apply(math_state, prev_op, numstack, &numstackptr);
                                 if (errmsg)
                                         goto err_with_custom_msg;
-                        }
-                        if (op == TOK_RPAREN)
+dbg("    numstack:%d val:%lld %lld %p", (int)(numstackptr - numstack),
+        numstackptr[-1].val,
+        numstackptr[-1].var_name == SECOND_VAL_VALID ? numstackptr[-1].second_val : 0,
+        numstackptr[-1].var_name
+);
+                        } /* while (opstack not empty) */
+                        if (op == TOK_RPAREN) /* unpaired RPAREN? */
                                 goto err;
                 }
-
-                /* Push this operator to the stack and remember it */
-//bb_error_msg("push op:%02x", op);
-                *stackptr++ = lasttok = op;
+                /* else: LPAREN or UNARY: push it on opstack */
+ push_op:
+                /* Push this operator to opstack */
+                dbg("(%d) op:%02x", (int)(opstackptr - opstack), op);
+                *opstackptr++ = lasttok = op;
  next: ;
         } /* while (1) */