Re: [PATCH 1/2 nft] jump: Introduce chain_expr in jump and goto statements

[Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx>]

On Tue, May 14, 2019 at 11:13:39PM +0200, Fernando Fernandez Mancera wrote:
[...]
> diff --git a/src/expression.c b/src/expression.c
> index eece12e..55a4ad7 100644
> --- a/src/expression.c
> +++ b/src/expression.c
> @@ -207,17 +207,18 @@ static bool verdict_expr_cmp(const struct expr *e1, const struct expr *e2)
>  
>  	if ((e1->verdict == NFT_JUMP ||
>  	     e1->verdict == NFT_GOTO) &&
> -	    strcmp(e1->chain, e2->chain))
> -		return false;
> +	     (expr_basetype(e1) == expr_basetype(e2) &&
> +	     !mpz_cmp(e1->value, e2->value)))

Maybe replace these two new lines above by:

              expr_cmp(e1->chain, e2->chain)

> +		return true;
>  
> -	return true;
> +	return false;
>  }
>  
>  static void verdict_expr_clone(struct expr *new, const struct expr *expr)
>  {
>  	new->verdict = expr->verdict;
>  	if (expr->chain != NULL)
> -		new->chain = xstrdup(expr->chain);
> +		mpz_init_set(new->chain->value, expr->chain->value);
>  }
>  
>  static void verdict_expr_destroy(struct expr *expr)

Memleak here in verdict_expr_destroy(), you need to call:

        expr_free(expr->chain);

instead of:

        xfree(expr->chain)

Please, run valgrind with --leak-check=full to make sure there are no
leaks either from adding a new rule nor when listing the ruleset.

Thanks!