On Tue, Jul 16, 2019 at 11:08:12AM +0200, Fernando Fernandez Mancera wrote:
> Signed-off-by: Fernando Fernandez Mancera <ffmancera@xxxxxxxxxx>
> ---
> include/rule.h | 8 ++++----
> src/evaluate.c | 29 +++++++++++++++++++----------
> src/parser_bison.y | 25 +++++++++++++++++--------
> src/rule.c | 4 ++--
> 4 files changed, 42 insertions(+), 24 deletions(-)
>
> diff --git a/include/rule.h b/include/rule.h
> index aefb24d..4d7cec8 100644
> --- a/include/rule.h
> +++ b/include/rule.h
> @@ -173,13 +173,13 @@ enum chain_flags {
> * struct prio_spec - extendend priority specification for mixed
> * textual/numerical parsing.
> *
> - * @str: name of the standard priority value
> - * @num: Numerical value. This MUST contain the parsed value of str after
> + * @prio_expr: expr of the standard priority value
> + * @num: Numerical value. This MUST contain the parsed value of prio_expr after
> * evaluation.
> */
> struct prio_spec {
> - const char *str;
> - int num;
> + struct expr *prio_expr;
Use:
struct expr *expr;
instead.
> + int num;
You could just store this in the expression, no need for this num
field.
> struct location loc;
> };
>
> diff --git a/src/evaluate.c b/src/evaluate.c
> index 8086f75..cee65cd 100644
> --- a/src/evaluate.c
> +++ b/src/evaluate.c
> @@ -3181,15 +3181,24 @@ static int set_evaluate(struct eval_ctx *ctx, struct set *set)
> return 0;
> }
>
> -static bool evaluate_priority(struct prio_spec *prio, int family, int hook)
> +static bool evaluate_priority(struct eval_ctx *ctx, struct prio_spec *prio,
> + int family, int hook)
> {
> int priority;
> + char prio_str[NFT_NAME_MAXLEN];
>
> /* A numeric value has been used to specify priority. */
> - if (prio->str == NULL)
> + if (prio->prio_expr == NULL)
prio_expr == NULL never happens.
> return true;
>
> - priority = std_prio_lookup(prio->str, family, hook);
> + if (expr_evaluate(ctx, &prio->prio_expr) < 0)
> + return false;
> + if (prio->prio_expr->etype == EXPR_VALUE)
You should bail out here with expr_error() is this is not an EXPR_VALUE.
> + mpz_export_data(prio_str, prio->prio_expr->value,
> + BYTEORDER_HOST_ENDIAN,
> + NFT_NAME_MAXLEN);
Remove the branch above, expr_evalute() already deals with
transforming the symbol to value expression.
> +
> + priority = std_prio_lookup(prio_str, family, hook);
> if (priority == NF_IP_PRI_LAST)
> return false;
> prio->num += priority;
> @@ -3211,10 +3220,10 @@ static int flowtable_evaluate(struct eval_ctx *ctx, struct flowtable *ft)
> if (ft->hooknum == NF_INET_NUMHOOKS)
> return chain_error(ctx, ft, "invalid hook %s", ft->hookstr);
>
> - if (!evaluate_priority(&ft->priority, NFPROTO_NETDEV, ft->hooknum))
> + if (!evaluate_priority(ctx, &ft->priority, NFPROTO_NETDEV, ft->hooknum))
> return __stmt_binary_error(ctx, &ft->priority.loc, NULL,
Better use expr_error() here?
> - "'%s' is invalid priority.",
> - ft->priority.str);
> + "invalid priority expression %s.",
> + expr_name(ft->priority.prio_expr));
>
> if (!ft->dev_expr)
> return chain_error(ctx, ft, "Unbound flowtable not allowed (must specify devices)");
> @@ -3410,11 +3419,11 @@ static int chain_evaluate(struct eval_ctx *ctx, struct chain *chain)
> return chain_error(ctx, chain, "invalid hook %s",
> chain->hookstr);
>
> - if (!evaluate_priority(&chain->priority, chain->handle.family,
> - chain->hooknum))
> + if (!evaluate_priority(ctx, &chain->priority,
> + chain->handle.family, chain->hooknum))
> return __stmt_binary_error(ctx, &chain->priority.loc, NULL,
> - "'%s' is invalid priority in this context.",
> - chain->priority.str);
> + "invalid priority expression %s in this context.",
> + expr_name(chain->priority.prio_expr));
> }
>
> list_for_each_entry(rule, &chain->rules, list) {
> diff --git a/src/parser_bison.y b/src/parser_bison.y
> index a4905f2..c6a43cf 100644
> --- a/src/parser_bison.y
> +++ b/src/parser_bison.y
> @@ -626,8 +626,8 @@ int nft_lex(void *, void *, void *);
> %type <stmt> meter_stmt meter_stmt_alloc flow_stmt_legacy_alloc
> %destructor { stmt_free($$); } meter_stmt meter_stmt_alloc flow_stmt_legacy_alloc
>
> -%type <expr> symbol_expr verdict_expr integer_expr variable_expr chain_expr
> -%destructor { expr_free($$); } symbol_expr verdict_expr integer_expr variable_expr chain_expr
> +%type <expr> symbol_expr verdict_expr integer_expr variable_expr chain_expr prio_expr
> +%destructor { expr_free($$); } symbol_expr verdict_expr integer_expr variable_expr chain_expr prio_expr
> %type <expr> primary_expr shift_expr and_expr
> %destructor { expr_free($$); } primary_expr shift_expr and_expr
> %type <expr> exclusive_or_expr inclusive_or_expr
> @@ -1926,30 +1926,39 @@ extended_prio_name : OUT
> | STRING
> ;
>
> +prio_expr : extended_prio_name
> + {
> + $$ = constant_expr_alloc(&@$, &string_type,
> + BYTEORDER_HOST_ENDIAN,
> + NFT_NAME_MAXLEN *
this should be strlen($1) * BITS_PER_BYTE
> + BITS_PER_BYTE, $1);
> + }
> + ;
