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.
> 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!
>> break;
>> case NFT_GOTO:
>> - nft_print(octx, "goto %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, "goto %s", chain);
>> + } else {
>> + nft_print(octx, "goto ");
>> + expr_print(expr->chain, octx);
>
> Same thing here.
>
> Apart from those nitpicks, this looks good :)
>
