Hi Pablo, thanks for reviewing. Comments below.
On 7/16/19 8:06 PM, Pablo Neira Ayuso wrote:
> On Tue, Jul 16, 2019 at 11:08:12AM +0200, Fernando Fernandez Mancera wrote:
>> Signed-off-by: Fernando Fernandez Mancera <[email protected]>
>> ---
>> 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.
>
I think that would not be possible. Right now, the priority
specification supports combinations of a string and a number. e.g
table inet global {
chain prerouting {
type filter hook prerouting priority filter + 3
policy accept
}
}
or
table inet global {
chain prerouting {
type filter hook prerouting priority filter - 3
policy accept
}
}
I don't think we are going to be able to do that using only a single
"struct expr *".
>> 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.
>
It never happens if we only have a single field in the "struct prio_spec".
Thanks!
