---
20220818231006.zyhyj%stef...@sdaoden.eu:
I just fixed two other bugs regarding $: ternary resolution which
affect strange cases regarding precedence in whiteouts
$((0?I1=10:(1?I3:I2=12)))
vs
$((0?I1=10:(0?I3:I2=12)))
as well as a missing variable-expansion prevention in a whiteout.
(Let me allow to keep saying whiteout, please.)
It bugs me. "Three line fix", so to say.
Locally i added a couple of more tests, and i am now really out of
ideas what other tests i could throw at the code.
Sorry again for now waiting until i had an iteration.
[.]I will send the small bugfix patch onto v2 tomorrow,[.]
It is a bit larger than it could be because it adds a dedicated
error constant, as well as renaming another one.
shell/math.c | 5 +++--
shell/shexp-arith.h | 53 ++++++++++++++++++++++++++++++---------------
2 files changed, 38 insertions(+), 20 deletions(-)
diff --git a/shell/math.c b/shell/math.c
index a20596089d..8388fdecad 100644
--- a/shell/math.c
+++ b/shell/math.c
@@ -214,7 +214,7 @@ static arith_t strto_arith_t(const char *nptr, char
**endptr)
#define CONCAT(S1,S2) su__CONCAT_1(S1, S2)
# define su__CONCAT_1(S1,S2) su__CONCAT_2(S1, S2)
# define su__CONCAT_2(S1,S2) S1 ## S2
-#define DBG(X)
+#define DBGX(X)
#define FALLTHRU
#define N_(X) X
#define NIL NULL
@@ -299,8 +299,9 @@ arith(arith_state_t *math_state, const char *expr)
a_X(ASSIGN_NO_VAR, "assignment without variable");
a_X(DIV_BY_ZERO, "division by zero");
a_X(EXP_INVALID, "invalid exponent");
- a_X(NO_OPERAND, "syntax error, expected operand");
+ a_X(NO_OP, "syntax error, expected operand");
a_X(COND_NO_COLON, "syntax error, incomplete ?: condition");
+ a_X(COND_PREC_INVALID, "?: condition, invalid precedence (1:v2:v3=3)");
a_X(NAME_LOOP, "recursive variable name reference");
a_X(OP_INVALID, "unknown operator");
}
diff --git a/shell/shexp-arith.h b/shell/shexp-arith.h
index f48ada1b46..fa44d4234d 100644
--- a/shell/shexp-arith.h
+++ b/shell/shexp-arith.h
@@ -47,8 +47,9 @@ enum a_shexp_arith_error{
a_SHEXP_ARITH_ERR_ASSIGN_NO_VAR, /* Assignment without variable */
a_SHEXP_ARITH_ERR_DIV_BY_ZERO,
a_SHEXP_ARITH_ERR_EXP_INVALID, /* Invalid exponent */
- a_SHEXP_ARITH_ERR_NO_OPERAND, /* Expected an argument here */
+ a_SHEXP_ARITH_ERR_NO_OP, /* Expected an argument here */
a_SHEXP_ARITH_ERR_COND_NO_COLON, /* Incomplete ?: condition */
+ a_SHEXP_ARITH_ERR_COND_PREC_INVALID, /* 1 ? VAR1 : VAR2 = 3 */
a_SHEXP_ARITH_ERR_NAME_LOOP, /* Variable self-reference loop */
a_SHEXP_ARITH_ERR_OP_INVALID /* Unknown operator */
};
@@ -269,10 +270,10 @@ a_shexp_arith_eval(
#endif
ASSERT_NYD_EXEC(resp != NIL,
- self.sac_error = a_SHEXP_ARITH_ERR_NO_OPERAND);
- DBG( *resp = 0; )
+ self.sac_error = a_SHEXP_ARITH_ERR_NO_OP);
+ DBGX( *resp = 0; )
ASSERT_NYD_EXEC(exp_len == 0 || exp_buf != NIL,
- self.sac_error = a_SHEXP_ARITH_ERR_NO_OPERAND);
+ self.sac_error = a_SHEXP_ARITH_ERR_NO_OP);
a_SHEXP_ARITH_IFS( self.sac_ifs_ws = ok_vlook(ifs_ws); )
self.sac_stack = &sas_stack;
@@ -425,6 +426,7 @@ a_shexp__arith_eval(struct a_shexp_arith_ctx *self,
++sasp->sas_nums_top;
lop = a_SHEXP_ARITH_OP_NUM;
+
a_SHEXP_ARITH_L((" + _arith_eval VAR <%s>\n",
sasp->sas_nums_top[-1].sav_var));
continue;
@@ -533,7 +535,7 @@ junapre:
prec < a_SHEXP_ARITH_PREC_UNARY) ||
prec >= a_SHEXP_ARITH_PREC_SKY){
if(lop != a_SHEXP_ARITH_OP_NUM){
- self->sac_error = a_SHEXP_ARITH_ERR_NO_OPERAND;
+ self->sac_error = a_SHEXP_ARITH_ERR_NO_OP;
goto jleave;
}
@@ -594,9 +596,11 @@ junapre:
ASSERT(sasp->sas_nums_top > sasp->sas_nums);
--sasp->sas_nums_top;
- if((op & a_SHEXP_ARITH_OP_MASK) != a_SHEXP_ARITH_OP_ASSIGN &&
- sasp->sas_nums_top->sav_var != NIL){
- if(!a_shexp__arith_val_eval(self, sasp->sas_nums_top))
+ if(sasp->sas_nums_top->sav_var != NIL){
+ if(!(op & a_SHEXP_ARITH_OP_FLAG_WHITE_MASK) &&
+ (op & a_SHEXP_ARITH_OP_MASK) !=
+ a_SHEXP_ARITH_OP_ASSIGN &&
+ !a_shexp__arith_val_eval(self, sasp->sas_nums_top))
goto jleave;
sasp->sas_nums_top->sav_var = NIL;
}
@@ -669,17 +673,29 @@ junapre:
}
/* Is this a right-associative operation? */
else{
- boole doit=FAL0;
+ boole doit;
+ doit = FAL0;
if(lprec < prec){
doit = TRU1;
a_SHEXP_ARITH_L((" + _arith_eval DELAY PRECEDENCE\n"));
}else if(lprec == prec && prec == a_SHEXP_ARITH_PREC_ASSIGN){
doit = TRU1;
a_SHEXP_ARITH_L((" + _arith_eval DELAY RIGHT ASSOC\n"));
- }else if(lop == a_SHEXP_ARITH_OP_COND){
- doit = TRU1;
- a_SHEXP_ARITH_L((" + _arith_eval DELAY CONDITION\n"));
+ }else if(lprec == a_SHEXP_ARITH_PREC_COND){
+ if(lop == a_SHEXP_ARITH_OP_COND){
+ doit = TRU1;
+ a_SHEXP_ARITH_L((" + _arith_eval DELAY CONDITION\n"));
+ }
+ /* Without massive rewrite this is the location to detect
+ * in-whiteout precedence bugs as in
+ * $((0?I1=10:(1?I3:I2=12)))
+ * which would be parsed like (1?I3:I2)=12 without error
+ * (different to 0?I3:I2=12) otherwise */
+ else if(op != a_SHEXP_ARITH_OP_COMMA){
+ self->sac_error = a_SHEXP_ARITH_ERR_COND_PREC_INVALID;
+ goto jleave;
+ }
}
if(doit){
@@ -887,7 +903,7 @@ a_shexp__arith_op_apply(struct a_shexp_arith_ctx *self){
/* At least one argument is always needed */
if((nums_top = sasp->sas_nums_top) == sasp->sas_nums){
- self->sac_error = a_SHEXP_ARITH_ERR_NO_OPERAND;
+ self->sac_error = a_SHEXP_ARITH_ERR_NO_OP;
goto jleave;
}
--nums_top;
@@ -922,7 +938,9 @@ a_shexp__arith_op_apply(struct a_shexp_arith_ctx *self){
}
goto jquick;
}else if(op == a_SHEXP_ARITH_OP_COND_COLON){
+ ASSERT(sasp->sas_ops_top > sasp->sas_ops);
ASSERT(nums_top > sasp->sas_nums);
+
if(!ign){
/* Move the ternary value over to LHV where we find it as a result,
* and ensure LHV's name is forgotten so not to evaluate it (for
@@ -930,18 +948,17 @@ a_shexp__arith_op_apply(struct a_shexp_arith_ctx *self){
* outer group, because it still exists on number stack) */
nums_top[-1].sav_val = nums_top[0].sav_val;
nums_top[-1].sav_var = NIL;
- }else{
+ }else
val = nums_top[-1].sav_val;
- /*nums_top[0].sav_var = NIL;*/
- }
+
sasp->sas_nums_top = nums_top;
- ASSERT(sasp->sas_ops_top > sasp->sas_ops);
if((sasp->sas_ops_top[-1] & a_SHEXP_ARITH_OP_MASK
) == a_SHEXP_ARITH_OP_COND_COLON){
--sasp->sas_ops_top;
if(!a_shexp__arith_op_apply_colons(self))
goto jleave;
+ ASSERT(sasp->sas_nums_top > sasp->sas_nums);
if(!ign)
sasp->sas_nums_top[-1].sav_val = val;
}
@@ -950,7 +967,7 @@ a_shexp__arith_op_apply(struct a_shexp_arith_ctx *self){
s64 rval;
if(nums_top == sasp->sas_nums){
- self->sac_error = a_SHEXP_ARITH_ERR_NO_OPERAND;
+ self->sac_error = a_SHEXP_ARITH_ERR_NO_OP;
goto jleave;
}
sasp->sas_nums_top = nums_top--;
--
2.37.2
--steffen
|
|Der Kragenbaer, The moon bear,
|der holt sich munter he cheerfully and one by one
|einen nach dem anderen runter wa.ks himself off
|(By Robert Gernhardt)
_______________________________________________
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
