From bed7c81ea24e9e9ba2a897e233de2abefe611e8b Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Thu, 16 Sep 2010 11:50:46 +0200 Subject: [PATCH] shell/math: deconvolute and explain ?: handling. Give better error message function old new delta arith_apply 1271 1283 +12 Signed-off-by: Denys Vlasenko --- shell/ash.c | 2 +- shell/ash_test/ash-arith/arith.right | 2 +- shell/hush.c | 4 +- shell/hush_test/hush-arith/arith.right | 2 +- shell/math.c | 243 ++++++++++++++----------- shell/math.h | 20 +- 6 files changed, 144 insertions(+), 129 deletions(-) diff --git a/shell/ash.c b/shell/ash.c index ec887e088..9089adc63 100644 --- a/shell/ash.c +++ b/shell/ash.c @@ -5516,7 +5516,7 @@ cvtnum(arith_t num) int len; expdest = makestrspace(32, expdest); - len = fmtstr(expdest, 32, arith_t_fmt, num); + len = fmtstr(expdest, 32, ARITH_FMT, num); STADJUST(len, expdest); return len; } diff --git a/shell/ash_test/ash-arith/arith.right b/shell/ash_test/ash-arith/arith.right index 7257cc566..9b9ca8e2f 100644 --- a/shell/ash_test/ash-arith/arith.right +++ b/shell/ash_test/ash-arith/arith.right @@ -72,7 +72,7 @@ ghi ./arith.tests: line 191: arithmetic syntax error 16 16 ./arith.tests: line 196: arithmetic syntax error -./arith.tests: line 197: arithmetic syntax error +./arith.tests: line 197: malformed ?: operator ./arith.tests: line 198: arithmetic syntax error 9 9 ./arith.tests: line 205: arithmetic syntax error diff --git a/shell/hush.c b/shell/hush.c index ad30ac1ea..a888332bc 100644 --- a/shell/hush.c +++ b/shell/hush.c @@ -4938,8 +4938,8 @@ static NOINLINE int expand_vars_to_list(o_string *output, int n, char *arg) *p = '\0'; /* replace trailing */ debug_printf_subst("ARITH '%s' first_ch %x\n", arg, first_ch); res = expand_and_evaluate_arith(arg, NULL); - debug_printf_subst("ARITH RES '"arith_t_fmt"'\n", res); - sprintf(arith_buf, arith_t_fmt, res); + debug_printf_subst("ARITH RES '"ARITH_FMT"'\n", res); + sprintf(arith_buf, ARITH_FMT, res); val = arith_buf; break; } diff --git a/shell/hush_test/hush-arith/arith.right b/shell/hush_test/hush-arith/arith.right index fd4ea8e01..8a201fb3b 100644 --- a/shell/hush_test/hush-arith/arith.right +++ b/shell/hush_test/hush-arith/arith.right @@ -81,7 +81,7 @@ ghi hush: arithmetic syntax error 16 16 hush: arithmetic syntax error -hush: arithmetic syntax error +hush: malformed ?: operator hush: arithmetic syntax error 9 9 hush: arithmetic syntax error diff --git a/shell/math.c b/shell/math.c index 871c06c3e..9d3b912a7 100644 --- a/shell/math.c +++ b/shell/math.c @@ -1,5 +1,5 @@ /* - * arithmetic code ripped out of ash shell for code sharing + * Arithmetic code ripped out of ash shell for code sharing. * * This code is derived from software contributed to Berkeley by * Kenneth Almquist. @@ -154,7 +154,7 @@ typedef unsigned char operator; #define fix_assignment_prec(prec) do { if (prec == 3) prec = 2; } while (0) -/* ternary conditional operator is right associative too */ +/* Ternary conditional operator is right associative too */ #define TOK_CONDITIONAL tok_decl(4,0) #define TOK_CONDITIONAL_SEP tok_decl(4,1) @@ -186,10 +186,10 @@ typedef unsigned char operator; #define TOK_DIV tok_decl(14,1) #define TOK_REM tok_decl(14,2) -/* exponent is right associative */ +/* Exponent is right associative */ #define TOK_EXPONENT tok_decl(15,1) -/* unary operators */ +/* Unary operators */ #define UNARYPREC 16 #define TOK_BNOT tok_decl(UNARYPREC,0) #define TOK_NOT tok_decl(UNARYPREC,1) @@ -213,30 +213,37 @@ typedef unsigned char operator; #define TOK_RPAREN tok_decl(SPEC_PREC, 1) static int -tok_have_assign(operator op) +is_assign_op(operator op) { operator prec = PREC(op); - fix_assignment_prec(prec); - return (prec == PREC(TOK_ASSIGN) || - prec == PREC_PRE || prec == PREC_POST); + return prec == PREC(TOK_ASSIGN) + || prec == PREC_PRE + || prec == PREC_POST; } static int is_right_associative(operator prec) { - return (prec == PREC(TOK_ASSIGN) || prec == PREC(TOK_EXPONENT) - || prec == PREC(TOK_CONDITIONAL)); + return prec == PREC(TOK_ASSIGN) + || prec == PREC(TOK_EXPONENT) + || prec == PREC(TOK_CONDITIONAL); } typedef struct { arith_t val; - arith_t contidional_second_val; - char contidional_second_val_initialized; - char *var; /* if NULL then is regular number, - else is variable name */ -} v_n_t; + /* We acquire second_val only when "expr1 : expr2" part + * of ternary ?: op is evaluated. + * We treat ?: as two binary ops: (expr ? (expr1 : expr2)). + * ':' produces a new value which has two parts, val and second_val; + * then '?' selects one of them based on its left side. + */ + arith_t second_val; + char second_val_present; + /* If NULL then it's just a number, else it's a named variable */ + char *var; +} var_or_num_t; typedef struct remembered_name { struct remembered_name *next; @@ -248,7 +255,7 @@ static arith_t FAST_FUNC evaluate_string(arith_state_t *math_state, const char *expr); static const char* -arith_lookup_val(arith_state_t *math_state, v_n_t *t) +arith_lookup_val(arith_state_t *math_state, var_or_num_t *t) { if (t->var) { const char *p = lookupvar(t->var); @@ -290,27 +297,28 @@ arith_lookup_val(arith_state_t *math_state, v_n_t *t) * stack. For an unary operator it will only change the top element, but a * binary operator will pop two arguments and push the result */ static NOINLINE const char* -arith_apply(arith_state_t *math_state, operator op, v_n_t *numstack, v_n_t **numstackptr) +arith_apply(arith_state_t *math_state, operator op, var_or_num_t *numstack, var_or_num_t **numstackptr) { #define NUMPTR (*numstackptr) - v_n_t *numptr_m1; - arith_t numptr_val, rez; + var_or_num_t *top_of_stack; + arith_t rez; const char *err; /* There is no operator that can work without arguments */ if (NUMPTR == numstack) goto err; - numptr_m1 = NUMPTR - 1; - /* Check operand is var with noninteger value */ - err = arith_lookup_val(math_state, numptr_m1); + top_of_stack = NUMPTR - 1; + + /* Resolve name to value, if needed */ + err = arith_lookup_val(math_state, top_of_stack); if (err) return err; - rez = numptr_m1->val; + rez = top_of_stack->val; if (op == TOK_UMINUS) - rez *= -1; + rez = -rez; else if (op == TOK_NOT) rez = !rez; else if (op == TOK_BNOT) @@ -321,112 +329,119 @@ arith_apply(arith_state_t *math_state, operator op, v_n_t *numstack, v_n_t **num rez--; else if (op != TOK_UPLUS) { /* Binary operators */ + arith_t right_side_val; + char bad_second_val; - /* check and binary operators need two arguments */ - if (numptr_m1 == numstack) goto err; - - /* ... and they pop one */ - --NUMPTR; - numptr_val = rez; - if (op == TOK_CONDITIONAL) { - if (!numptr_m1->contidional_second_val_initialized) { - /* protect $((expr1 ? expr2)) without ": expr" */ - goto err; - } - rez = numptr_m1->contidional_second_val; - } else if (numptr_m1->contidional_second_val_initialized) { - /* protect $((expr1 : expr2)) without "expr ? " */ + /* Binary operators need two arguments */ + if (top_of_stack == numstack) goto err; + /* ...and they pop one */ + NUMPTR = top_of_stack; /* this decrements NUMPTR */ + + bad_second_val = top_of_stack->second_val_present; + if (op == TOK_CONDITIONAL) { /* ? operation */ + /* Make next if (...) protect against + * $((expr1 ? expr2)) - that is, missing ": expr" */ + bad_second_val = !bad_second_val; + } + if (bad_second_val) { + /* Protect against $((expr expr1 : expr2)) */ + return "malformed ?: operator"; } - numptr_m1 = NUMPTR - 1; + + top_of_stack--; /* now points to left side */ + if (op != TOK_ASSIGN) { - /* check operand is var with noninteger value for not '=' */ - err = arith_lookup_val(math_state, numptr_m1); + /* Resolve left side value (unless the op is '=') */ + err = arith_lookup_val(math_state, top_of_stack); if (err) return err; } - if (op == TOK_CONDITIONAL) { - numptr_m1->contidional_second_val = rez; + + right_side_val = rez; + rez = top_of_stack->val; + if (op == TOK_CONDITIONAL) /* ? operation */ + rez = (rez ? right_side_val : top_of_stack[1].second_val); + else if (op == TOK_CONDITIONAL_SEP) { /* : operation */ + if (top_of_stack == numstack) { + /* Protect against $((expr : expr)) */ + return "malformed ?: operator"; + } + top_of_stack->second_val_present = op; + top_of_stack->second_val = right_side_val; } - rez = numptr_m1->val; - if (op == TOK_BOR || op == TOK_OR_ASSIGN) - rez |= numptr_val; + else if (op == TOK_BOR || op == TOK_OR_ASSIGN) + rez |= right_side_val; else if (op == TOK_OR) - rez = numptr_val || rez; + rez = right_side_val || rez; else if (op == TOK_BAND || op == TOK_AND_ASSIGN) - rez &= numptr_val; + rez &= right_side_val; else if (op == TOK_BXOR || op == TOK_XOR_ASSIGN) - rez ^= numptr_val; + rez ^= right_side_val; else if (op == TOK_AND) - rez = rez && numptr_val; + rez = rez && right_side_val; else if (op == TOK_EQ) - rez = (rez == numptr_val); + rez = (rez == right_side_val); else if (op == TOK_NE) - rez = (rez != numptr_val); + rez = (rez != right_side_val); else if (op == TOK_GE) - rez = (rez >= numptr_val); + rez = (rez >= right_side_val); else if (op == TOK_RSHIFT || op == TOK_RSHIFT_ASSIGN) - rez >>= numptr_val; + rez >>= right_side_val; else if (op == TOK_LSHIFT || op == TOK_LSHIFT_ASSIGN) - rez <<= numptr_val; + rez <<= right_side_val; else if (op == TOK_GT) - rez = (rez > numptr_val); + rez = (rez > right_side_val); else if (op == TOK_LT) - rez = (rez < numptr_val); + rez = (rez < right_side_val); else if (op == TOK_LE) - rez = (rez <= numptr_val); + rez = (rez <= right_side_val); else if (op == TOK_MUL || op == TOK_MUL_ASSIGN) - rez *= numptr_val; + rez *= right_side_val; else if (op == TOK_ADD || op == TOK_PLUS_ASSIGN) - rez += numptr_val; + rez += right_side_val; else if (op == TOK_SUB || op == TOK_MINUS_ASSIGN) - rez -= numptr_val; + rez -= right_side_val; else if (op == TOK_ASSIGN || op == TOK_COMMA) - rez = numptr_val; - else if (op == TOK_CONDITIONAL_SEP) { - if (numptr_m1 == numstack) { - /* protect $((expr : expr)) without "expr ? " */ - goto err; - } - numptr_m1->contidional_second_val_initialized = op; - numptr_m1->contidional_second_val = numptr_val; - } else if (op == TOK_CONDITIONAL) { - rez = rez ? - numptr_val : numptr_m1->contidional_second_val; - } else if (op == TOK_EXPONENT) { + rez = right_side_val; + else if (op == TOK_EXPONENT) { arith_t c; - if (numptr_val < 0) + if (right_side_val < 0) return "exponent less than 0"; c = 1; - while (--numptr_val >= 0) + while (--right_side_val >= 0) c *= rez; rez = c; - } else if (numptr_val == 0) + } + else if (right_side_val == 0) return "divide by zero"; else if (op == TOK_DIV || op == TOK_DIV_ASSIGN) - rez /= numptr_val; + rez /= right_side_val; else if (op == TOK_REM || op == TOK_REM_ASSIGN) - rez %= numptr_val; + rez %= right_side_val; } - if (tok_have_assign(op)) { + + if (is_assign_op(op)) { char buf[sizeof(arith_t)*3 + 2]; - if (numptr_m1->var == NULL) { + if (top_of_stack->var == NULL) { /* Hmm, 1=2 ? */ +//TODO: actually, bash allows ++7 but for some reason it evals to 7, not 8 goto err; } - /* save to shell variable */ - sprintf(buf, arith_t_fmt, rez); - setvar(numptr_m1->var, buf); - /* after saving, make previous value for v++ or v-- */ + /* Save to shell variable */ + sprintf(buf, ARITH_FMT, rez); + setvar(top_of_stack->var, buf); + /* After saving, make previous value for v++ or v-- */ if (op == TOK_POST_INC) rez--; else if (op == TOK_POST_DEC) rez++; } - numptr_m1->val = rez; - /* erase var name, it is just a number now */ - numptr_m1->var = NULL; + + top_of_stack->val = rez; + /* Erase var name, it is just a number now */ + top_of_stack->var = NULL; return NULL; err: return "arithmetic syntax error"; @@ -499,16 +514,17 @@ evaluate_string(arith_state_t *math_state, const char *expr) const char *start_expr = expr = skip_whitespace(expr); unsigned expr_len = strlen(expr) + 2; /* Stack of integers */ - /* The proof that there can be no more than strlen(startbuf)/2+1 integers - * in any given correct or incorrect expression is left as an exercise to - * the reader. */ - v_n_t *const numstack = alloca((expr_len / 2) * sizeof(numstack[0])); - v_n_t *numstackptr = numstack; + /* The proof that there can be no more than strlen(startbuf)/2+1 + * integers in any given correct or incorrect expression + * is left as an exercise to the reader. */ + 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; - *stackptr++ = lasttok = TOK_LPAREN; /* start off with a left paren */ + /* Start with a left paren */ + *stackptr++ = lasttok = TOK_LPAREN; errmsg = NULL; while (1) { @@ -521,7 +537,7 @@ evaluate_string(arith_state_t *math_state, const char *expr) arithval = *expr; if (arithval == '\0') { if (expr == start_expr) { - /* Null expression. */ + /* Null expression */ numstack->val = 0; goto ret; } @@ -558,7 +574,7 @@ evaluate_string(arith_state_t *math_state, const char *expr) safe_strncpy(numstackptr->var, expr, var_name_size); expr = p; num: - numstackptr->contidional_second_val_initialized = 0; + numstackptr->second_val_present = 0; numstackptr++; lasttok = TOK_NUM; continue; @@ -577,21 +593,32 @@ evaluate_string(arith_state_t *math_state, const char *expr) /* Should be an operator */ p = op_tokens; while (1) { - const char *e = expr; +// TODO: bash allows 7+++v, treats it as 7 + ++v +// we treat it as 7++ + v and reject /* Compare expr to current op_tokens[] element */ - while (*p && *e == *p) - p++, e++; - if (*p == '\0') { /* match: operator is found */ - expr = e; - break; + const char *e = expr; + while (1) { + if (*p == '\0') { + /* Match: operator is found */ + expr = e; + goto tok_found; + } + if (*p != *e) + break; + p++; + e++; } - /* Go to next element of op_tokens[] */ + /* No match, go to next element of op_tokens[] */ while (*p) p++; p += 2; /* skip NUL and TOK_foo bytes */ - if (*p == '\0') /* no next element, operator not found */ + if (*p == '\0') { + /* No next element, operator not found */ + //math_state->syntax_error_at = expr; goto err; + } } + tok_found: op = p[1]; /* fetch TOK_foo value */ /* NB: expr now points past the operator */ @@ -662,21 +689,21 @@ evaluate_string(arith_state_t *math_state, const char *expr) } errmsg = arith_apply(math_state, prev_op, numstack, &numstackptr); if (errmsg) - goto ret; + goto err_with_custom_msg; } - if (op == TOK_RPAREN) { + if (op == TOK_RPAREN) goto err; - } } - /* Push this operator to the stack and remember it. */ + /* Push this operator to the stack and remember it */ *stackptr++ = lasttok = op; next: ; } /* while (1) */ err: - numstack->val = -1; errmsg = "arithmetic syntax error"; + err_with_custom_msg: + numstack->val = -1; ret: math_state->errmsg = errmsg; return numstack->val; diff --git a/shell/math.h b/shell/math.h index 2dcab130d..2d305eb12 100644 --- a/shell/math.h +++ b/shell/math.h @@ -9,19 +9,13 @@ /* The math library has just one function: * - * arith_t arith(arith_state_t *states, const char *expr); + * arith_t arith(arith_state_t *state, const char *expr); * * The expr argument is the math string to parse. All normal expansions must * be done already. i.e. no dollar symbols should be present. * * The state argument is a pointer to a struct of hooks for your shell (see below), - * and a semi-detailed error code. Currently, those values are (for - * compatibility, you should assume all negative values are errors): - * 0 - no errors (yay!) - * -1 - unspecified problem - * -2 - divide by zero - * -3 - exponent less than 0 - * -5 - expression recursion loop detected + * and an error message string (NULL if no error). * * The function returns the answer to the expression. So if you called it * with the expression: @@ -64,12 +58,6 @@ * the regex (in C locale): ^[a-zA-Z_][a-zA-Z_0-9]* */ -/* To make your life easier when dealing with optional 64bit math support, - * rather than assume that the type is "signed long" and you can always - * use "%ld" to scan/print the value, use the arith_t helper defines. See - * below for the exact things that are available. - */ - #ifndef SHELL_MATH_H #define SHELL_MATH_H 1 @@ -77,11 +65,11 @@ PUSH_AND_SET_FUNCTION_VISIBILITY_TO_HIDDEN #if ENABLE_SH_MATH_SUPPORT_64 typedef long long arith_t; -#define arith_t_fmt "%lld" +#define ARITH_FMT "%lld" #define strto_arith_t strtoull #else typedef long arith_t; -#define arith_t_fmt "%ld" +#define ARITH_FMT "%ld" #define strto_arith_t strtoul #endif -- 2.25.1