Re: [nft PATCH 4/7] netlink: decode payload statment

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

On Wed, Jul 27, 2016 at 02:43:13AM +0200, Florian Westphal wrote:
> This allows nft to display payload set operations if the
> header isn't byte aligned or has non-byte divisible sizes.
> 
> Signed-off-by: Florian Westphal <fw@strlen.de>
> ---
>  src/netlink_delinearize.c | 165 ++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 160 insertions(+), 5 deletions(-)
> 
> diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c
> index ae87280..5e28111 100644
> --- a/src/netlink_delinearize.c
> +++ b/src/netlink_delinearize.c
> @@ -1747,6 +1747,165 @@ static void stmt_expr_postprocess(struct rule_pp_ctx *ctx)
>  		expr_postprocess_range(ctx, op);
>  }
>  
> +static void stmt_payload_binop_pp(struct rule_pp_ctx *ctx, struct expr *binop)
> +{
> +	struct expr *payload = binop->left;
> +	struct expr *mask = binop->right;
> +	unsigned int shift;
> +
> +	assert(payload->ops->type == EXPR_PAYLOAD);
> +	if (payload_expr_trim(payload, mask, &ctx->pctx, &shift)) {
> +		__binop_adjust(binop, mask, shift);
> +		payload_expr_complete(payload, &ctx->pctx);
> +		expr_set_type(mask, payload->dtype,
> +			      payload->byteorder);
> +	}
> +}
> +
> +static void stmt_payload_postprocess(struct rule_pp_ctx *ctx)
> +{
> +	struct stmt *stmt = ctx->stmt;
> +	struct expr *expr, *binop, *payload, *value, *mask;
> +	mpz_t bitmask;
> +
> +	expr_postprocess(ctx, &stmt->payload.expr);
> +
> +	expr_set_type(stmt->payload.val,
> +		      stmt->payload.expr->dtype,
> +		      stmt->payload.expr->byteorder);
> +
> +	if (payload_is_known(stmt->payload.expr))
> +		goto out;

Could you wrap the whole code below into a function, something like:

        stmt_payload_binop_postprocess(...)

(feel free to select a better name that looks consistent to what we
have around), so this looks like:

static void stmt_payload_postprocess(struct rule_pp_ctx *ctx)
{
        [...]

	expr_postprocess(ctx, &stmt->payload.expr);

	expr_set_type(stmt->payload.val,
		      stmt->payload.expr->dtype,
		      stmt->payload.expr->byteorder);

	if (!payload_is_known(stmt->payload.expr))
                stmt_payload_binop_postprocess(...);

        expr_postprocess(...);
}

So we reduce the use of gotos a bit.

> +	/*
> +	 * expr_postprocess() failed to decode the payload information.

Thus, this comment becomes a description for this new
stmt_payload_binop_postprocess() function.