Re: [PATCH nft 1/2] evaluate: compact STMT_F_STATEFUL checks

[Pablo Neira Ayuso <pablo@netfilter.org>]

On Mon, Mar 31, 2025 at 05:23:19PM +0200, Florian Westphal wrote:
> We'll gain another F_STATEFUL check in a followup patch,
> so lets condense the pattern into a helper to reduce copypaste.

This series LGTM, thanks Florian.

> Signed-off-by: Florian Westphal <fw@strlen.de>

Reviewed-by: Pablo Neira Ayuso <pablo@netfilter.org>

> ---
>  src/evaluate.c | 26 ++++++++++++++------------
>  1 file changed, 14 insertions(+), 12 deletions(-)
> 
> diff --git a/src/evaluate.c b/src/evaluate.c
> index e4a7b5ceaafa..e9ab829b6bbb 100644
> --- a/src/evaluate.c
> +++ b/src/evaluate.c
> @@ -3453,6 +3453,17 @@ static int stmt_evaluate_payload(struct eval_ctx *ctx, struct stmt *stmt)
>  	return expr_evaluate(ctx, &stmt->payload.val);
>  }
>  
> +static int stmt_evaluate_stateful(struct eval_ctx *ctx, struct stmt *stmt, const char *name)
> +{
> +	if (stmt_evaluate(ctx, stmt) < 0)
> +		return -1;
> +
> +	if (!(stmt->flags & STMT_F_STATEFUL))
> +		return stmt_error(ctx, stmt, "%s statement must be stateful", name);
> +
> +	return 0;
> +}
> +
>  static int stmt_evaluate_meter(struct eval_ctx *ctx, struct stmt *stmt)
>  {
>  	struct expr *key, *setref;
> @@ -3526,11 +3537,8 @@ static int stmt_evaluate_meter(struct eval_ctx *ctx, struct stmt *stmt)
>  
>  	stmt->meter.set = setref;
>  
> -	if (stmt_evaluate(ctx, stmt->meter.stmt) < 0)
> +	if (stmt_evaluate_stateful(ctx, stmt->meter.stmt, "meter") < 0)
>  		return -1;
> -	if (!(stmt->meter.stmt->flags & STMT_F_STATEFUL))
> -		return stmt_binary_error(ctx, stmt->meter.stmt, stmt,
> -					 "meter statement must be stateful");
>  
>  	return 0;
>  }
> @@ -4662,11 +4670,8 @@ static int stmt_evaluate_set(struct eval_ctx *ctx, struct stmt *stmt)
>  		return expr_error(ctx->msgs, stmt->set.key,
>  				  "Key expression comments are not supported");
>  	list_for_each_entry(this, &stmt->set.stmt_list, list) {
> -		if (stmt_evaluate(ctx, this) < 0)
> +		if (stmt_evaluate_stateful(ctx, this, "set") < 0)
>  			return -1;
> -		if (!(this->flags & STMT_F_STATEFUL))
> -			return stmt_error(ctx, this,
> -					  "statement must be stateful");
>  	}
>  
>  	this_set = stmt->set.set->set;
> @@ -4726,11 +4731,8 @@ static int stmt_evaluate_map(struct eval_ctx *ctx, struct stmt *stmt)
>  				  "Data expression timeouts are not supported");
>  
>  	list_for_each_entry(this, &stmt->map.stmt_list, list) {
> -		if (stmt_evaluate(ctx, this) < 0)
> +		if (stmt_evaluate_stateful(ctx, this, "map") < 0)
>  			return -1;
> -		if (!(this->flags & STMT_F_STATEFUL))
> -			return stmt_error(ctx, this,
> -					  "statement must be stateful");
>  	}
>  
>  	return 0;
> -- 
> 2.49.0
> 
>