On Tue, May 21, 2019 at 09:38:16PM +0200, Fernando Fernandez Mancera wrote:
> Hi Pablo,
>
> On 5/21/19 11:28 AM, Pablo Neira Ayuso wrote:
> > On Thu, May 16, 2019 at 10:45:58PM +0200, Fernando Fernandez Mancera wrote:
> >> Now we can introduce expressions as a chain in jump and goto statements.
> >> This
> >> is going to be used to support variables as a chain in the following
> >> patches.
> >
> > Something is wrong with json:
> >
> > json.c: In function ‘verdict_expr_json’:
> > json.c:683:11: warning: assignment from incompatible pointer type
> > [-Wincompatible-pointer-types]
> > chain = expr->chain;
> > ^
> > parser_json.c: In function ‘json_parse_verdict_expr’:
> > parser_json.c:1086:8: warning: passing argument 3 of
> > ‘verdict_expr_alloc’ from incompatible pointer type
> > [-Wincompatible-pointer-types]
> > chain ? xstrdup(chain) : NULL);
> > ^~~~~
> >
> > Most likely --enable-json missing there.
> >
>
> Sorry, I am going to fix that.
Thanks!
> > diff --git a/src/datatype.c b/src/datatype.c
> >> index ac9f2af..10f185b 100644
> >> --- a/src/datatype.c
> >> +++ b/src/datatype.c
> >> @@ -254,6 +254,8 @@ const struct datatype invalid_type = {
> >>
> >> static void verdict_type_print(const struct expr *expr, struct output_ctx
> >> *octx)
> >> {
> >> + char chain[NFT_CHAIN_MAXNAMELEN];
> >> +
> >> switch (expr->verdict) {
> >> case NFT_CONTINUE:
> >> nft_print(octx, "continue");
> >> @@ -262,10 +264,26 @@ static void verdict_type_print(const struct expr
> >> *expr, struct output_ctx *octx)
> >> nft_print(octx, "break");
> >> break;
> >> case NFT_JUMP:
> >> - nft_print(octx, "jump %s", expr->chain);
> >> + if (expr->chain->etype == EXPR_VALUE) {
> >> + mpz_export_data(chain, expr->chain->value,
> >> + BYTEORDER_HOST_ENDIAN,
> >> + NFT_CHAIN_MAXNAMELEN);
> >> + nft_print(octx, "jump %s", chain);
> >> + } else {
> >> + nft_print(octx, "jump ");
> >> + expr_print(expr->chain, octx);
> >> + }
> >
> > I think this should be fine:
> >
> > case NFT_JUMP:
> > nft_print(octx, "jump ");
> > expr_print(expr->chain, octx);
> > break;
> >
> > Any reason to have the 'if (expr->chain->etype == EXPR_VALUE) {'
> > check?
> >
>
> Yes, without this check the list ruleset is slightly different when
> using variables.
>
> table ip foo {
> chain bar {
> type filter hook input priority filter; policy accept;
> jump "ber"
> }
>
> chain ber {
> counter packets 45 bytes 3132
> }
> }
>
> Please, note the quote marks in the jump statement. If we don't want to
> check that, we need to change all the tests that involve jumps (about 12).
Thanks for explaining.
