[PATCH nft,v2 00/11] replace compound_expr_*() by type safe function

[PATCH nft,v2 00/11] replace compound_expr_*() by type safe function

[Pablo Neira Ayuso]

Hi,

This is v2 with minimal changes wrt. previous series.

The aim is to replace (and remove) the existing compound_expr_*()
helper functions which is common to set, list and concat expressions
by expression type safe variants that validate the expression type.

I will hold on with this until nftables 1.1.5 including the fix for
the JSON regression is released.

Pablo Neira Ayuso (11):
  src: add expr_type_catchall() helper and use it
  src: replace compound_expr_add() by type safe set_expr_add()
  src: replace compound_expr_add() by type safe concat_expr_add()
  src: replace compound_expr_add() by type safe list_expr_add()
  segtree: rename set_compound_expr_add() to set_expr_add_splice()
  expression: replace compound_expr_clone() by type safe function
  expression: remove compound_expr_add()
  expression: replace compound_expr_remove() by type safe function
  expression: replace compound_expr_destroy() by type safe funtion
  expression: replace compound_expr_print() by type safe function
  src: replace compound_expr_alloc() by type safe function

 include/expression.h      |  15 +++-
 src/evaluate.c            |   4 +-
 src/expression.c          | 177 ++++++++++++++++++++++++++------------
 src/intervals.c           |  20 ++---
 src/monitor.c             |   2 +-
 src/netlink.c             |   6 +-
 src/netlink_delinearize.c |   8 +-
 src/optimize.c            |  38 ++++----
 src/parser_bison.y        |  40 ++++-----
 src/parser_json.c         |  18 ++--
 src/payload.c             |   6 +-
 src/segtree.c             |  40 ++++-----
 src/trace.c               |  10 +--
 13 files changed, 229 insertions(+), 155 deletions(-)

-- 
2.30.2

[PATCH nft,v2 01/11] src: add expr_type_catchall() helper and use it

[Pablo Neira Ayuso]

Add helper function to check if this is a catchall expression.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 include/expression.h |  3 +++
 src/evaluate.c       |  2 +-
 src/intervals.c      | 12 ++++++------
 src/segtree.c        |  2 +-
 4 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/include/expression.h b/include/expression.h
index e483b7e76f4c..2e0754edaaae 100644
--- a/include/expression.h
+++ b/include/expression.h
@@ -552,6 +552,9 @@ extern struct expr *set_elem_expr_alloc(const struct location *loc,
 
 struct expr *set_elem_catchall_expr_alloc(const struct location *loc);
 
+#define expr_type_catchall(__expr)			\
+	((__expr)->etype == EXPR_SET_ELEM_CATCHALL)
+
 extern void range_expr_value_low(mpz_t rop, const struct expr *expr);
 extern void range_expr_value_high(mpz_t rop, const struct expr *expr);
 void range_expr_swap_values(struct expr *range);
diff --git a/src/evaluate.c b/src/evaluate.c
index a2ca3aaea35c..fc9177d3a91e 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -1928,7 +1928,7 @@ static bool elem_key_compatible(const struct expr *set_key,
 				const struct expr *elem_key)
 {
 	/* Catchall element is always compatible with the set key declaration */
-	if (elem_key->etype == EXPR_SET_ELEM_CATCHALL)
+	if (expr_type_catchall(elem_key))
 		return true;
 
 	return datatype_compatible(set_key->dtype, elem_key->dtype);
diff --git a/src/intervals.c b/src/intervals.c
index 8c8ce8c8a305..d5afffd2120a 100644
--- a/src/intervals.c
+++ b/src/intervals.c
@@ -175,7 +175,7 @@ static void setelem_automerge(struct set_automerge_ctx *ctx)
 	mpz_init(rop);
 
 	list_for_each_entry_safe(i, next, &expr_set(ctx->init)->expressions, list) {
-		if (i->key->etype == EXPR_SET_ELEM_CATCHALL)
+		if (expr_type_catchall(i->key))
 			continue;
 
 		range_expr_value_low(range.low, i);
@@ -410,7 +410,7 @@ static int setelem_delete(struct list_head *msgs, struct set *set,
 	list_for_each_entry_safe(elem, next, &expr_set(elems)->expressions, list) {
 		i = interval_expr_key(elem);
 
-		if (i->key->etype == EXPR_SET_ELEM_CATCHALL) {
+		if (expr_type_catchall(i->key)) {
 			/* Assume max value to simplify handling. */
 			mpz_bitmask(range.low, i->len);
 			mpz_bitmask(range.high, i->len);
@@ -574,7 +574,7 @@ static int setelem_overlap(struct list_head *msgs, struct set *set,
 	list_for_each_entry_safe(elem, next, &expr_set(init)->expressions, list) {
 		i = interval_expr_key(elem);
 
-		if (i->key->etype == EXPR_SET_ELEM_CATCHALL)
+		if (expr_type_catchall(i->key))
 			continue;
 
 		range_expr_value_low(range.low, i);
@@ -686,7 +686,7 @@ int set_to_intervals(const struct set *set, struct expr *init, bool add)
 	list_for_each_entry_safe(i, n, &expr_set(init)->expressions, list) {
 		elem = interval_expr_key(i);
 
-		if (elem->key->etype == EXPR_SET_ELEM_CATCHALL)
+		if (expr_type_catchall(elem->key))
 			continue;
 
 		if (prev)
@@ -770,12 +770,12 @@ int setelem_to_interval(const struct set *set, struct expr *elem,
 	bool adjacent = false;
 
 	key = setelem_key(elem);
-	if (key->etype == EXPR_SET_ELEM_CATCHALL)
+	if (expr_type_catchall(key))
 		return 0;
 
 	if (next_elem) {
 		next_key = setelem_key(next_elem);
-		if (next_key->etype == EXPR_SET_ELEM_CATCHALL)
+		if (expr_type_catchall(next_key))
 			next_key = NULL;
 	}
 
diff --git a/src/segtree.c b/src/segtree.c
index fd77e03fbff5..607f002f181e 100644
--- a/src/segtree.c
+++ b/src/segtree.c
@@ -579,7 +579,7 @@ void interval_map_decompose(struct expr *set)
 		else if (i->etype == EXPR_MAPPING)
 			key = i->left->key;
 
-		if (key && key->etype == EXPR_SET_ELEM_CATCHALL) {
+		if (key && expr_type_catchall(key)) {
 			list_del(&i->list);
 			catchall = i;
 			continue;
-- 
2.30.2

[PATCH nft,v2 02/11] src: replace compound_expr_add() by type safe set_expr_add()

[Pablo Neira Ayuso]

Replace compound_expr_add() by set_expr_add() to validate type.

Add __set_expr_add() to skip size updates in src/intervals.c

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 include/expression.h |  3 +++
 src/expression.c     | 13 +++++++++++++
 src/intervals.c      |  8 ++++----
 src/monitor.c        |  2 +-
 src/netlink.c        |  2 +-
 src/optimize.c       | 20 ++++++++++----------
 src/parser_bison.y   |  8 ++++----
 src/parser_json.c    |  4 ++--
 src/payload.c        |  6 +++---
 src/segtree.c        | 20 ++++++++++----------
 10 files changed, 51 insertions(+), 35 deletions(-)

diff --git a/include/expression.h b/include/expression.h
index 2e0754edaaae..21be74068468 100644
--- a/include/expression.h
+++ b/include/expression.h
@@ -531,6 +531,9 @@ struct expr *list_expr_to_binop(struct expr *expr);
 
 extern struct expr *set_expr_alloc(const struct location *loc,
 				   const struct set *set);
+void __set_expr_add(struct expr *set, struct expr *elem);
+void set_expr_add(struct expr *set, struct expr *elem);
+
 extern void concat_range_aggregate(struct expr *set);
 extern void interval_map_decompose(struct expr *set);
 
diff --git a/src/expression.c b/src/expression.c
index 8cb639797284..32e101ea9070 100644
--- a/src/expression.c
+++ b/src/expression.c
@@ -1392,6 +1392,19 @@ struct expr *set_expr_alloc(const struct location *loc, const struct set *set)
 	return set_expr;
 }
 
+void __set_expr_add(struct expr *set, struct expr *elem)
+{
+	list_add_tail(&elem->list, &expr_set(set)->expressions);
+}
+
+void set_expr_add(struct expr *set, struct expr *elem)
+{
+	struct expr_set *expr_set = expr_set(set);
+
+	list_add_tail(&elem->list, &expr_set->expressions);
+	expr_set->size++;
+}
+
 static void mapping_expr_print(const struct expr *expr, struct output_ctx *octx)
 {
 	expr_print(expr->left, octx);
diff --git a/src/intervals.c b/src/intervals.c
index d5afffd2120a..a63c58ac9606 100644
--- a/src/intervals.c
+++ b/src/intervals.c
@@ -278,7 +278,7 @@ int set_automerge(struct list_head *msgs, struct cmd *cmd, struct set *set,
 			}
 			clone = expr_clone(i);
 			clone->flags |= EXPR_F_KERNEL;
-			list_add_tail(&clone->list, &expr_set(existing_set->init)->expressions);
+			__set_expr_add(existing_set->init, clone);
 		}
 	}
 
@@ -359,7 +359,7 @@ static void split_range(struct set *set, struct expr *prev, struct expr *i,
 	clone = expr_clone(prev);
 	mpz_set(clone->key->range.low, i->key->range.high);
 	mpz_add_ui(clone->key->range.low, i->key->range.high, 1);
-	list_add_tail(&clone->list, &expr_set(set->existing_set->init)->expressions);
+	__set_expr_add(set->existing_set->init, clone);
 
 	mpz_set(prev->key->range.high, i->key->range.low);
 	mpz_sub_ui(prev->key->range.high, i->key->range.low, 1);
@@ -527,7 +527,7 @@ int set_delete(struct list_head *msgs, struct cmd *cmd, struct set *set,
 	list_for_each_entry(i, &expr_set(existing_set->init)->expressions, list) {
 		if (!(i->flags & EXPR_F_KERNEL)) {
 			clone = expr_clone(i);
-			list_add_tail(&clone->list, &expr_set(add)->expressions);
+			__set_expr_add(add, clone);
 			i->flags |= EXPR_F_KERNEL;
 		}
 	}
@@ -646,7 +646,7 @@ int set_overlap(struct list_head *msgs, struct set *set, struct expr *init)
 		else if (existing_set) {
 			clone = expr_clone(i);
 			clone->flags |= EXPR_F_KERNEL;
-			list_add_tail(&clone->list, &expr_set(existing_set->init)->expressions);
+			__set_expr_add(existing_set->init, clone);
 		}
 	}
 
diff --git a/src/monitor.c b/src/monitor.c
index da1ad880f0c8..e58f62252ca2 100644
--- a/src/monitor.c
+++ b/src/monitor.c
@@ -400,7 +400,7 @@ static bool netlink_event_range_cache(struct set *cached_set,
 
 	/* if cache exists, dummyset must contain the other end of the range */
 	if (cached_set->rg_cache) {
-		compound_expr_add(dummyset->init, cached_set->rg_cache);
+		set_expr_add(dummyset->init, cached_set->rg_cache);
 		cached_set->rg_cache = NULL;
 		goto out_decompose;
 	}
diff --git a/src/netlink.c b/src/netlink.c
index 94cbcbfc6c09..30dd6c9b4f0d 100644
--- a/src/netlink.c
+++ b/src/netlink.c
@@ -1591,7 +1591,7 @@ key_end:
 		expr = mapping_expr_alloc(&netlink_location, expr, data);
 	}
 out:
-	compound_expr_add(set->init, expr);
+	set_expr_add(set->init, expr);
 
 	if (!(flags & NFT_SET_ELEM_INTERVAL_END) &&
 	    nftnl_set_elem_is_set(nlse, NFTNL_SET_ELEM_KEY_END)) {
diff --git a/src/optimize.c b/src/optimize.c
index 40756cecbbc3..b2fd9e829f00 100644
--- a/src/optimize.c
+++ b/src/optimize.c
@@ -569,13 +569,13 @@ static void merge_expr_stmts(const struct optimize_ctx *ctx,
 
 	expr_a = stmt_a->expr->right;
 	elem = set_elem_expr_alloc(&internal_location, expr_get(expr_a));
-	compound_expr_add(set, elem);
+	set_expr_add(set, elem);
 
 	for (i = from + 1; i <= to; i++) {
 		stmt_b = ctx->stmt_matrix[i][merge->stmt[0]];
 		expr_b = stmt_b->expr->right;
 		elem = set_elem_expr_alloc(&internal_location, expr_get(expr_b));
-		compound_expr_add(set, elem);
+		set_expr_add(set, elem);
 	}
 
 	expr_free(stmt_a->expr->right);
@@ -590,7 +590,7 @@ static void merge_vmap(const struct optimize_ctx *ctx,
 	mappings = stmt_b->expr->mappings;
 	list_for_each_entry(expr, &expr_set(mappings)->expressions, list) {
 		mapping = expr_clone(expr);
-		compound_expr_add(stmt_a->expr->mappings, mapping);
+		set_expr_add(stmt_a->expr->mappings, mapping);
 	}
 }
 
@@ -702,7 +702,7 @@ static void __merge_concat_stmts(const struct optimize_ctx *ctx, uint32_t i,
 	list_for_each_entry_safe(concat, next, &concat_list, list) {
 		list_del(&concat->list);
 		elem = set_elem_expr_alloc(&internal_location, concat);
-		compound_expr_add(set, elem);
+		set_expr_add(set, elem);
 	}
 }
 
@@ -759,7 +759,7 @@ static void build_verdict_map(struct expr *expr, struct stmt *verdict,
 
 			mapping = mapping_expr_alloc(&internal_location, elem,
 						     expr_get(verdict->expr));
-			compound_expr_add(set, mapping);
+			set_expr_add(set, mapping);
 		}
 		stmt_free(counter);
 		break;
@@ -773,7 +773,7 @@ static void build_verdict_map(struct expr *expr, struct stmt *verdict,
 
 			mapping = mapping_expr_alloc(&internal_location, elem,
 						     expr_get(verdict->expr));
-			compound_expr_add(set, mapping);
+			set_expr_add(set, mapping);
 		}
 		stmt_free(counter);
 		break;
@@ -790,7 +790,7 @@ static void build_verdict_map(struct expr *expr, struct stmt *verdict,
 
 		mapping = mapping_expr_alloc(&internal_location, elem,
 					     expr_get(verdict->expr));
-		compound_expr_add(set, mapping);
+		set_expr_add(set, mapping);
 		break;
 	default:
 		assert(0);
@@ -898,7 +898,7 @@ static void __merge_concat_stmts_vmap(const struct optimize_ctx *ctx,
 
 		mapping = mapping_expr_alloc(&internal_location, elem,
 					     expr_get(verdict->expr));
-		compound_expr_add(set, mapping);
+		set_expr_add(set, mapping);
 	}
 	stmt_free(counter);
 }
@@ -1061,7 +1061,7 @@ static void merge_nat(const struct optimize_ctx *ctx,
 
 		elem = set_elem_expr_alloc(&internal_location, expr_get(expr));
 		mapping = mapping_expr_alloc(&internal_location, elem, nat_expr);
-		compound_expr_add(set, mapping);
+		set_expr_add(set, mapping);
 	}
 
 	stmt = ctx->stmt_matrix[from][merge->stmt[0]];
@@ -1118,7 +1118,7 @@ static void merge_concat_nat(const struct optimize_ctx *ctx,
 
 		elem = set_elem_expr_alloc(&internal_location, concat);
 		mapping = mapping_expr_alloc(&internal_location, elem, nat_expr);
-		compound_expr_add(set, mapping);
+		set_expr_add(set, mapping);
 	}
 
 	concat = concat_expr_alloc(&internal_location);
diff --git a/src/parser_bison.y b/src/parser_bison.y
index 1e4b3f8a50c5..aab1cc675234 100644
--- a/src/parser_bison.y
+++ b/src/parser_bison.y
@@ -3276,11 +3276,11 @@ verdict_map_expr	:	'{'	verdict_map_list_expr	'}'
 verdict_map_list_expr	:	verdict_map_list_member_expr
 			{
 				$$ = set_expr_alloc(&@$, NULL);
-				compound_expr_add($$, $1);
+				set_expr_add($$, $1);
 			}
 			|	verdict_map_list_expr	COMMA	verdict_map_list_member_expr
 			{
-				compound_expr_add($1, $3);
+				set_expr_add($1, $3);
 				$$ = $1;
 			}
 			|	verdict_map_list_expr	COMMA	opt_newline
@@ -4533,11 +4533,11 @@ set_expr		:	'{'	set_list_expr		'}'
 set_list_expr		:	set_list_member_expr
 			{
 				$$ = set_expr_alloc(&@$, NULL);
-				compound_expr_add($$, $1);
+				set_expr_add($$, $1);
 			}
 			|	set_list_expr		COMMA	set_list_member_expr
 			{
-				compound_expr_add($1, $3);
+				set_expr_add($1, $3);
 				$$ = $1;
 			}
 			|	set_list_expr		COMMA	opt_newline
diff --git a/src/parser_json.c b/src/parser_json.c
index 71e44f19c9f1..1a37246ef7f6 100644
--- a/src/parser_json.c
+++ b/src/parser_json.c
@@ -1454,7 +1454,7 @@ static struct expr *json_parse_set_expr(struct json_ctx *ctx,
 
 		expr = set_elem_expr_alloc(int_loc, expr);
 		set_expr = set_expr_alloc(int_loc, NULL);
-		compound_expr_add(set_expr, expr);
+		set_expr_add(set_expr, expr);
 		return set_expr;
 	}
 
@@ -1498,7 +1498,7 @@ static struct expr *json_parse_set_expr(struct json_ctx *ctx,
 
 		if (!set_expr)
 			set_expr = set_expr_alloc(int_loc, NULL);
-		compound_expr_add(set_expr, expr);
+		set_expr_add(set_expr, expr);
 	}
 	return set_expr;
 }
diff --git a/src/payload.c b/src/payload.c
index a38f5bf730d1..162367eb7fd0 100644
--- a/src/payload.c
+++ b/src/payload.c
@@ -1534,13 +1534,13 @@ __payload_gen_icmp_echo_dependency(struct eval_ctx *ctx, const struct expr *expr
 				    BYTEORDER_BIG_ENDIAN, BITS_PER_BYTE,
 				    constant_data_ptr(echo, BITS_PER_BYTE));
 	right = set_elem_expr_alloc(&expr->location, right);
-	compound_expr_add(set, right);
+	set_expr_add(set, right);
 
 	right = constant_expr_alloc(&expr->location, icmp_type,
 				    BYTEORDER_BIG_ENDIAN, BITS_PER_BYTE,
 				    constant_data_ptr(reply, BITS_PER_BYTE));
 	right = set_elem_expr_alloc(&expr->location, right);
-	compound_expr_add(set, right);
+	set_expr_add(set, right);
 
 	dep = relational_expr_alloc(&expr->location, OP_IMPLICIT, left, set);
 	return expr_stmt_alloc(&dep->location, dep);
@@ -1571,7 +1571,7 @@ __payload_gen_icmp6_addr_dependency(struct eval_ctx *ctx, const struct expr *exp
 					    constant_data_ptr(icmp_addr_types[i],
 							      BITS_PER_BYTE));
 		right = set_elem_expr_alloc(&expr->location, right);
-		compound_expr_add(set, right);
+		set_expr_add(set, right);
 	}
 
 	dep = relational_expr_alloc(&expr->location, OP_IMPLICIT, left, set);
diff --git a/src/segtree.c b/src/segtree.c
index 607f002f181e..9395b5388507 100644
--- a/src/segtree.c
+++ b/src/segtree.c
@@ -64,7 +64,7 @@ static void set_elem_add(const struct set *set, struct expr *init, mpz_t value,
 	expr = set_elem_expr_alloc(&internal_location, expr);
 	expr->flags = flags;
 
-	compound_expr_add(init, expr);
+	set_expr_add(init, expr);
 }
 
 struct expr *get_set_intervals(const struct set *set, const struct expr *init)
@@ -86,12 +86,12 @@ struct expr *get_set_intervals(const struct set *set, const struct expr *init)
 				     i->flags, byteorder);
 			break;
 		case EXPR_CONCAT:
-			compound_expr_add(new_init, expr_clone(i));
+			set_expr_add(new_init, expr_clone(i));
 			i->flags |= EXPR_F_INTERVAL_END;
-			compound_expr_add(new_init, expr_clone(i));
+			set_expr_add(new_init, expr_clone(i));
 			break;
 		case EXPR_SET_ELEM_CATCHALL:
-			compound_expr_add(new_init, expr_clone(i));
+			set_expr_add(new_init, expr_clone(i));
 			break;
 		default:
 			range_expr_value_low(low, i);
@@ -214,16 +214,16 @@ static void set_compound_expr_add(struct expr *compound, struct expr *expr, stru
 	switch (expr->etype) {
 	case EXPR_SET_ELEM:
 		list_splice_init(&orig->stmt_list, &expr->stmt_list);
-		compound_expr_add(compound, expr);
+		set_expr_add(compound, expr);
 		break;
 	case EXPR_MAPPING:
 		list_splice_init(&orig->left->stmt_list, &expr->left->stmt_list);
-		compound_expr_add(compound, expr);
+		set_expr_add(compound, expr);
 		break;
 	default:
 		elem = set_elem_expr_alloc(&orig->location, expr);
 		list_splice_init(&orig->stmt_list, &elem->stmt_list);
-		compound_expr_add(compound, elem);
+		set_expr_add(compound, elem);
 		break;
 	}
 }
@@ -551,7 +551,7 @@ add_interval(struct expr *set, struct expr *low, struct expr *i)
 	} else
 		expr = interval_to_range(low, i, range);
 
-	compound_expr_add(set, expr);
+	set_expr_add(set, expr);
 
 	mpz_clear(range);
 	mpz_clear(p);
@@ -645,7 +645,7 @@ void interval_map_decompose(struct expr *set)
 	mpz_bitmask(i->value, i->len);
 
 	if (!mpz_cmp(i->value, expr_value(low)->value)) {
-		compound_expr_add(set, low);
+		set_expr_add(set, low);
 	} else {
 		add_interval(set, low, i);
 		expr_free(low);
@@ -656,7 +656,7 @@ void interval_map_decompose(struct expr *set)
 out:
 	if (catchall) {
 		catchall->flags |= EXPR_F_KERNEL;
-		compound_expr_add(set, catchall);
+		set_expr_add(set, catchall);
 	}
 
 	free(ranges);
-- 
2.30.2

[PATCH nft,v2 03/11] src: replace compound_expr_add() by type safe concat_expr_add()

[Pablo Neira Ayuso]

Replace compound_expr_add by concat_expr_add() to validate type.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 include/expression.h      |  1 +
 src/expression.c          | 10 +++++++++-
 src/netlink.c             |  4 ++--
 src/netlink_delinearize.c |  6 +++---
 src/optimize.c            | 18 +++++++++---------
 src/parser_bison.y        |  4 ++--
 src/parser_json.c         |  6 +++---
 7 files changed, 29 insertions(+), 20 deletions(-)

diff --git a/include/expression.h b/include/expression.h
index 21be74068468..71a7298891cc 100644
--- a/include/expression.h
+++ b/include/expression.h
@@ -525,6 +525,7 @@ extern void list_expr_sort(struct list_head *head);
 extern void list_splice_sorted(struct list_head *list, struct list_head *head);
 
 extern struct expr *concat_expr_alloc(const struct location *loc);
+void concat_expr_add(struct expr *concat, struct expr *item);
 
 extern struct expr *list_expr_alloc(const struct location *loc);
 struct expr *list_expr_to_binop(struct expr *expr);
diff --git a/src/expression.c b/src/expression.c
index 32e101ea9070..106208f2b19c 100644
--- a/src/expression.c
+++ b/src/expression.c
@@ -1211,7 +1211,7 @@ static struct expr *concat_expr_parse_udata(const struct nftnl_udata *attr)
 			goto err_free;
 
 		dt = concat_subtype_add(dt, expr->dtype->type);
-		compound_expr_add(concat_expr, expr);
+		concat_expr_add(concat_expr, expr);
 		len += netlink_padded_len(expr->len);
 	}
 
@@ -1245,6 +1245,14 @@ struct expr *concat_expr_alloc(const struct location *loc)
 	return compound_expr_alloc(loc, EXPR_CONCAT);
 }
 
+void concat_expr_add(struct expr *concat, struct expr *item)
+{
+	struct expr_concat *expr_concat = expr_concat(concat);
+
+	list_add_tail(&item->list, &expr_concat->expressions);
+	expr_concat->size++;
+}
+
 static void list_expr_print(const struct expr *expr, struct output_ctx *octx)
 {
 	compound_expr_print(expr, ",", octx);
diff --git a/src/netlink.c b/src/netlink.c
index 30dd6c9b4f0d..edf8c5197e65 100644
--- a/src/netlink.c
+++ b/src/netlink.c
@@ -1366,7 +1366,7 @@ static struct expr *netlink_parse_concat_elem_key(const struct set *set,
 	concat = concat_expr_alloc(&data->location);
 	while (off > 0) {
 		expr = concat_elem_expr(set, n, dtype, data, &off);
-		compound_expr_add(concat, expr);
+		concat_expr_add(concat, expr);
 		if (set->key->etype == EXPR_CONCAT)
 			n = list_next_entry(n, list);
 	}
@@ -1405,7 +1405,7 @@ static struct expr *netlink_parse_concat_elem(const struct set *set,
 
 			range = range_expr_alloc(&data->location, left, expr);
 			range = range_expr_reduce(range);
-			compound_expr_add(concat, range);
+			concat_expr_add(concat, range);
 		}
 		assert(list_empty(&expressions));
 	} else {
diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c
index b97962a30ca2..aa43ca85c5eb 100644
--- a/src/netlink_delinearize.c
+++ b/src/netlink_delinearize.c
@@ -128,7 +128,7 @@ static struct expr *netlink_parse_concat_expr(struct netlink_parse_ctx *ctx,
 				      "Relational expression size mismatch");
 			goto err;
 		}
-		compound_expr_add(concat, expr);
+		concat_expr_add(concat, expr);
 
 		consumed = netlink_padded_len(expr->len);
 		assert(consumed > 0);
@@ -171,7 +171,7 @@ static struct expr *netlink_parse_concat_key(struct netlink_parse_ctx *ctx,
 			expr_set_type(expr, i, i->byteorder);
 		}
 
-		compound_expr_add(concat, expr);
+		concat_expr_add(concat, expr);
 
 		consumed = netlink_padded_len(expr->len);
 		assert(consumed > 0);
@@ -204,7 +204,7 @@ static struct expr *netlink_parse_concat_data(struct netlink_parse_ctx *ctx,
 		}
 		i = constant_expr_splice(data, expr->len);
 		data->len -= netlink_padding_len(expr->len);
-		compound_expr_add(concat, i);
+		concat_expr_add(concat, i);
 
 		len -= netlink_padded_len(expr->len);
 		reg += netlink_register_space(expr->len);
diff --git a/src/optimize.c b/src/optimize.c
index b2fd9e829f00..cdd6913a306d 100644
--- a/src/optimize.c
+++ b/src/optimize.c
@@ -657,7 +657,7 @@ static void __merge_concat(const struct optimize_ctx *ctx, uint32_t i,
 				list_for_each_entry(expr, &expr_set(stmt_a->expr->right)->expressions, list) {
 					concat_clone = expr_clone(concat);
 					clone = expr_clone(expr->key);
-					compound_expr_add(concat_clone, clone);
+					concat_expr_add(concat_clone, clone);
 					list_add_tail(&concat_clone->list, &pending_list);
 				}
 				list_del(&concat->list);
@@ -670,13 +670,13 @@ static void __merge_concat(const struct optimize_ctx *ctx, uint32_t i,
 			case EXPR_RANGE:
 			case EXPR_RANGE_VALUE:
 				clone = expr_clone(stmt_a->expr->right);
-				compound_expr_add(concat, clone);
+				concat_expr_add(concat, clone);
 				break;
 			case EXPR_LIST:
 				list_for_each_entry(expr, &expr_list(stmt_a->expr->right)->expressions, list) {
 					concat_clone = expr_clone(concat);
 					clone = expr_clone(expr);
-					compound_expr_add(concat_clone, clone);
+					concat_expr_add(concat_clone, clone);
 					list_add_tail(&concat_clone->list, &pending_list);
 				}
 				list_del(&concat->list);
@@ -720,7 +720,7 @@ static void merge_concat_stmts(const struct optimize_ctx *ctx,
 
 	for (k = 0; k < merge->num_stmts; k++) {
 		stmt_a = ctx->stmt_matrix[from][merge->stmt[k]];
-		compound_expr_add(concat, expr_get(stmt_a->expr->left));
+		concat_expr_add(concat, expr_get(stmt_a->expr->left));
 	}
 	expr_free(stmt->expr->left);
 	stmt->expr->left = concat;
@@ -920,7 +920,7 @@ static void merge_concat_stmts_vmap(const struct optimize_ctx *ctx,
 	concat_a = concat_expr_alloc(&internal_location);
 	for (i = 0; i < merge->num_stmts; i++) {
 		stmt_a = ctx->stmt_matrix[from][merge->stmt[i]];
-		compound_expr_add(concat_a, expr_get(stmt_a->expr->left));
+		concat_expr_add(concat_a, expr_get(stmt_a->expr->left));
 	}
 
 	/* build set data contenation, eg. { eth0 . 1.1.1.1 . 22 : accept } */
@@ -1021,8 +1021,8 @@ static struct expr *stmt_nat_expr(struct stmt *nat_stmt)
 	if (nat_stmt->nat.proto) {
 		if (nat_stmt->nat.addr) {
 			nat_expr = concat_expr_alloc(&internal_location);
-			compound_expr_add(nat_expr, expr_get(nat_stmt->nat.addr));
-			compound_expr_add(nat_expr, expr_get(nat_stmt->nat.proto));
+			concat_expr_add(nat_expr, expr_get(nat_stmt->nat.addr));
+			concat_expr_add(nat_expr, expr_get(nat_stmt->nat.proto));
 		} else {
 			nat_expr = expr_get(nat_stmt->nat.proto);
 		}
@@ -1110,7 +1110,7 @@ static void merge_concat_nat(const struct optimize_ctx *ctx,
 		for (j = 0; j < merge->num_stmts; j++) {
 			stmt = ctx->stmt_matrix[i][merge->stmt[j]];
 			expr = stmt->expr->right;
-			compound_expr_add(concat, expr_get(expr));
+			concat_expr_add(concat, expr_get(expr));
 		}
 
 		nat_stmt = ctx->stmt_matrix[i][k];
@@ -1131,7 +1131,7 @@ static void merge_concat_nat(const struct optimize_ctx *ctx,
 			else if (left->payload.desc == &proto_ip6)
 				family = NFPROTO_IPV6;
 		}
-		compound_expr_add(concat, expr_get(left));
+		concat_expr_add(concat, expr_get(left));
 	}
 	expr = map_expr_alloc(&internal_location, concat, set);
 
diff --git a/src/parser_bison.y b/src/parser_bison.y
index aab1cc675234..401c17b0d3c6 100644
--- a/src/parser_bison.y
+++ b/src/parser_bison.y
@@ -121,7 +121,7 @@ static struct expr *handle_concat_expr(const struct location *loc,
 {
 	if (expr->etype != EXPR_CONCAT) {
 		expr = concat_expr_alloc(loc);
-		compound_expr_add(expr, expr_l);
+		concat_expr_add(expr, expr_l);
 	} else {
 		location_update(&expr_r->location, loc_rhs, 2);
 
@@ -129,7 +129,7 @@ static struct expr *handle_concat_expr(const struct location *loc,
 		expr->location = *loc;
 	}
 
-	compound_expr_add(expr, expr_r);
+	concat_expr_add(expr, expr_r);
 	return expr;
 }
 
diff --git a/src/parser_json.c b/src/parser_json.c
index 1a37246ef7f6..2216d41563b0 100644
--- a/src/parser_json.c
+++ b/src/parser_json.c
@@ -1330,10 +1330,10 @@ static struct expr *json_parse_concat_expr(struct json_ctx *ctx,
 			struct expr *concat;
 
 			concat = concat_expr_alloc(int_loc);
-			compound_expr_add(concat, expr);
+			concat_expr_add(concat, expr);
 			expr = concat;
 		}
-		compound_expr_add(expr, tmp);
+		concat_expr_add(expr, tmp);
 	}
 	return expr ? json_check_concat_expr(ctx, expr) : NULL;
 }
@@ -1806,7 +1806,7 @@ static struct expr *json_parse_dtype_expr(struct json_ctx *ctx, json_t *root)
 				expr_free(expr);
 				return NULL;
 			}
-			compound_expr_add(expr, i);
+			concat_expr_add(expr, i);
 		}
 
 		return json_check_concat_expr(ctx, expr);
-- 
2.30.2

[PATCH nft,v2 04/11] src: replace compound_expr_add() by type safe list_expr_add()

[Pablo Neira Ayuso]

Replace compound_expr_add() by list_expr_add() to validate type.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 include/expression.h      |  1 +
 src/expression.c          |  8 ++++++++
 src/netlink_delinearize.c |  2 +-
 src/parser_bison.y        | 20 ++++++++++----------
 src/parser_json.c         |  6 +++---
 src/trace.c               | 10 +++++-----
 6 files changed, 28 insertions(+), 19 deletions(-)

diff --git a/include/expression.h b/include/expression.h
index 71a7298891cc..c2c59891a8a1 100644
--- a/include/expression.h
+++ b/include/expression.h
@@ -528,6 +528,7 @@ extern struct expr *concat_expr_alloc(const struct location *loc);
 void concat_expr_add(struct expr *concat, struct expr *item);
 
 extern struct expr *list_expr_alloc(const struct location *loc);
+void list_expr_add(struct expr *expr, struct expr *item);
 struct expr *list_expr_to_binop(struct expr *expr);
 
 extern struct expr *set_expr_alloc(const struct location *loc,
diff --git a/src/expression.c b/src/expression.c
index 106208f2b19c..22234567d2b1 100644
--- a/src/expression.c
+++ b/src/expression.c
@@ -1272,6 +1272,14 @@ struct expr *list_expr_alloc(const struct location *loc)
 	return compound_expr_alloc(loc, EXPR_LIST);
 }
 
+void list_expr_add(struct expr *expr, struct expr *item)
+{
+	struct expr_list *expr_list = expr_list(expr);
+
+	list_add_tail(&item->list, &expr_list->expressions);
+	expr_list->size++;
+}
+
 /* list is assumed to have two items at least, otherwise extend this! */
 struct expr *list_expr_to_binop(struct expr *expr)
 {
diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c
index aa43ca85c5eb..c8644da960fa 100644
--- a/src/netlink_delinearize.c
+++ b/src/netlink_delinearize.c
@@ -2438,7 +2438,7 @@ static struct expr *binop_tree_to_list(struct expr *list, struct expr *expr)
 	} else {
 		if (list == NULL)
 			return expr_get(expr);
-		compound_expr_add(list, expr_get(expr));
+		list_expr_add(list, expr_get(expr));
 	}
 
 	return list;
diff --git a/src/parser_bison.y b/src/parser_bison.y
index 401c17b0d3c6..6d4eb98f3b5a 100644
--- a/src/parser_bison.y
+++ b/src/parser_bison.y
@@ -2463,11 +2463,11 @@ flowtable_expr		:	'{'	flowtable_list_expr	'}'
 flowtable_list_expr	:	flowtable_expr_member
 			{
 				$$ = compound_expr_alloc(&@$, EXPR_LIST);
-				compound_expr_add($$, $1);
+				list_expr_add($$, $1);
 			}
 			|	flowtable_list_expr	COMMA	flowtable_expr_member
 			{
-				compound_expr_add($1, $3);
+				list_expr_add($1, $3);
 				$$ = $1;
 			}
 			|	flowtable_list_expr	COMMA	opt_newline
@@ -2802,14 +2802,14 @@ dev_spec		:	DEVICE	string
 					YYERROR;
 
 				$$ = compound_expr_alloc(&@$, EXPR_LIST);
-				compound_expr_add($$, expr);
+				list_expr_add($$, expr);
 
 			}
 			|	DEVICE	variable_expr
 			{
 				datatype_set($2->sym->expr, &ifname_type);
 				$$ = compound_expr_alloc(&@$, EXPR_LIST);
-				compound_expr_add($$, $2);
+				list_expr_add($$, $2);
 			}
 			|	DEVICES		'='	flowtable_expr
 			{
@@ -4977,13 +4977,13 @@ relational_expr		:	expr	/* implicit */	rhs_expr
 list_rhs_expr		:	basic_rhs_expr		COMMA		basic_rhs_expr
 			{
 				$$ = list_expr_alloc(&@$);
-				compound_expr_add($$, $1);
-				compound_expr_add($$, $3);
+				list_expr_add($$, $1);
+				list_expr_add($$, $3);
 			}
 			|	list_rhs_expr		COMMA		basic_rhs_expr
 			{
 				$1->location = @$;
-				compound_expr_add($1, $3);
+				list_expr_add($1, $3);
 				$$ = $1;
 			}
 			;
@@ -5531,13 +5531,13 @@ symbol_stmt_expr		:	symbol_expr
 list_stmt_expr		:	symbol_stmt_expr	COMMA	symbol_stmt_expr
 			{
 				$$ = list_expr_alloc(&@$);
-				compound_expr_add($$, $1);
-				compound_expr_add($$, $3);
+				list_expr_add($$, $1);
+				list_expr_add($$, $3);
 			}
 			|	list_stmt_expr	COMMA	symbol_stmt_expr
 			{
 				$1->location = @$;
-				compound_expr_add($1, $3);
+				list_expr_add($1, $3);
 				$$ = $1;
 			}
 			;
diff --git a/src/parser_json.c b/src/parser_json.c
index 2216d41563b0..17e13ebe4458 100644
--- a/src/parser_json.c
+++ b/src/parser_json.c
@@ -1686,7 +1686,7 @@ static struct expr *json_parse_expr(struct json_ctx *ctx, json_t *root)
 				expr_free(list);
 				return NULL;
 			}
-			compound_expr_add(list, expr);
+			list_expr_add(list, expr);
 		}
 		return list;
 	case JSON_TRUE:
@@ -3002,7 +3002,7 @@ static struct expr *json_parse_devs(struct json_ctx *ctx, json_t *root)
 			return NULL;
 		}
 
-		compound_expr_add(expr, tmp);
+		list_expr_add(expr, tmp);
 		return expr;
 	}
 	if (!json_is_array(root)) {
@@ -3023,7 +3023,7 @@ static struct expr *json_parse_devs(struct json_ctx *ctx, json_t *root)
 			expr_free(expr);
 			return NULL;
 		}
-		compound_expr_add(expr, tmp);
+		list_expr_add(expr, tmp);
 	}
 	return expr;
 }
diff --git a/src/trace.c b/src/trace.c
index b270951025b8..b0f26e03169b 100644
--- a/src/trace.c
+++ b/src/trace.c
@@ -267,11 +267,11 @@ static struct expr *trace_alloc_list(const struct datatype *dtype,
 		if (bitv == 0)
 			continue;
 
-		compound_expr_add(list_expr,
-				  constant_expr_alloc(&netlink_location,
-						      dtype, byteorder,
-						      len * BITS_PER_BYTE,
-						      &bitv));
+		list_expr_add(list_expr,
+			      constant_expr_alloc(&netlink_location,
+						  dtype, byteorder,
+						  len * BITS_PER_BYTE,
+						  &bitv));
 	}
 
 	mpz_clear(value);
-- 
2.30.2

[PATCH nft,v2 05/11] segtree: rename set_compound_expr_add() to set_expr_add_splice()

[Pablo Neira Ayuso]

To avoid confusion when perfoming git grep to search for compound_expr_add()

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 src/segtree.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/segtree.c b/src/segtree.c
index 9395b5388507..e6b14f5dd120 100644
--- a/src/segtree.c
+++ b/src/segtree.c
@@ -207,7 +207,7 @@ static struct expr *expr_to_set_elem(struct expr *e)
 	return __expr_to_set_elem(e, expr);
 }
 
-static void set_compound_expr_add(struct expr *compound, struct expr *expr, struct expr *orig)
+static void set_expr_add_splice(struct expr *compound, struct expr *expr, struct expr *orig)
 {
 	struct expr *elem;
 
@@ -250,7 +250,7 @@ int get_set_decompose(struct set *cache_set, struct set *set)
 				return -1;
 			}
 
-			set_compound_expr_add(new_init, range, left);
+			set_expr_add_splice(new_init, range, left);
 
 			expr_free(left);
 			expr_free(i);
@@ -262,9 +262,9 @@ int get_set_decompose(struct set *cache_set, struct set *set)
 							      left, NULL);
 
 				if (range)
-					set_compound_expr_add(new_init, range, left);
+					set_expr_add_splice(new_init, range, left);
 				else
-					set_compound_expr_add(new_init,
+					set_expr_add_splice(new_init,
 							      expr_to_set_elem(left), left);
 			}
 			left = i;
@@ -273,9 +273,9 @@ int get_set_decompose(struct set *cache_set, struct set *set)
 	if (left) {
 		range = get_set_interval_find(cache_set, left, NULL);
 		if (range)
-			set_compound_expr_add(new_init, range, left);
+			set_expr_add_splice(new_init, range, left);
 		else
-			set_compound_expr_add(new_init, expr_to_set_elem(left), left);
+			set_expr_add_splice(new_init, expr_to_set_elem(left), left);
 	}
 
 	expr_free(set->init);
-- 
2.30.2

[PATCH nft,v2 06/11] expression: replace compound_expr_clone() by type safe function

[Pablo Neira Ayuso]

Replace compound_expr_clone() by:

- concat_expr_clone()
- list_expr_clone()
- set_expr_clone()

to validate type.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 src/expression.c | 42 ++++++++++++++++++++++++++++++------------
 1 file changed, 30 insertions(+), 12 deletions(-)

diff --git a/src/expression.c b/src/expression.c
index 22234567d2b1..6c5140b749f9 100644
--- a/src/expression.c
+++ b/src/expression.c
@@ -1027,15 +1027,6 @@ struct expr *compound_expr_alloc(const struct location *loc,
 	return expr;
 }
 
-static void compound_expr_clone(struct expr *new, const struct expr *expr)
-{
-	struct expr *i;
-
-	init_list_head(&new->expr_set.expressions);
-	list_for_each_entry(i, &expr->expr_set.expressions, list)
-		compound_expr_add(new, expr_clone(i));
-}
-
 static void compound_expr_destroy(struct expr *expr)
 {
 	struct expr *i, *next;
@@ -1079,6 +1070,15 @@ static void concat_expr_print(const struct expr *expr, struct output_ctx *octx)
 	compound_expr_print(expr, " . ", octx);
 }
 
+static void concat_expr_clone(struct expr *new, const struct expr *expr)
+{
+	struct expr *i;
+
+	init_list_head(&expr_concat(new)->expressions);
+	list_for_each_entry(i, &expr_concat(expr)->expressions, list)
+		concat_expr_add(new, expr_clone(i));
+}
+
 #define NFTNL_UDATA_SET_KEY_CONCAT_NEST 0
 #define NFTNL_UDATA_SET_KEY_CONCAT_NEST_MAX  NFT_REG32_SIZE
 
@@ -1234,7 +1234,7 @@ static const struct expr_ops concat_expr_ops = {
 	.name		= "concat",
 	.print		= concat_expr_print,
 	.json		= concat_expr_json,
-	.clone		= compound_expr_clone,
+	.clone		= concat_expr_clone,
 	.destroy	= concat_expr_destroy,
 	.build_udata	= concat_expr_build_udata,
 	.parse_udata	= concat_expr_parse_udata,
@@ -1258,12 +1258,21 @@ static void list_expr_print(const struct expr *expr, struct output_ctx *octx)
 	compound_expr_print(expr, ",", octx);
 }
 
+static void list_expr_clone(struct expr *new, const struct expr *expr)
+{
+	struct expr *i;
+
+	init_list_head(&expr_list(new)->expressions);
+	list_for_each_entry(i, &expr_list(expr)->expressions, list)
+		list_expr_add(new, expr_clone(i));
+}
+
 static const struct expr_ops list_expr_ops = {
 	.type		= EXPR_LIST,
 	.name		= "list",
 	.print		= list_expr_print,
 	.json		= list_expr_json,
-	.clone		= compound_expr_clone,
+	.clone		= list_expr_clone,
 	.destroy	= compound_expr_destroy,
 };
 
@@ -1375,6 +1384,15 @@ static void set_expr_print(const struct expr *expr, struct output_ctx *octx)
 	nft_print(octx, " }");
 }
 
+static void set_expr_clone(struct expr *new, const struct expr *expr)
+{
+	struct expr *i;
+
+	init_list_head(&expr_set(new)->expressions);
+	list_for_each_entry(i, &expr_set(expr)->expressions, list)
+		set_expr_add(new, expr_clone(i));
+}
+
 static void set_expr_set_type(const struct expr *expr,
 			      const struct datatype *dtype,
 			      enum byteorder byteorder)
@@ -1391,7 +1409,7 @@ static const struct expr_ops set_expr_ops = {
 	.print		= set_expr_print,
 	.json		= set_expr_json,
 	.set_type	= set_expr_set_type,
-	.clone		= compound_expr_clone,
+	.clone		= set_expr_clone,
 	.destroy	= compound_expr_destroy,
 };
 
-- 
2.30.2

[PATCH nft,v2 07/11] expression: remove compound_expr_add()

[Pablo Neira Ayuso]

No more users of this function after conversion to type safe variant,
remove it.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 include/expression.h | 1 -
 src/expression.c     | 6 ------
 2 files changed, 7 deletions(-)

diff --git a/include/expression.h b/include/expression.h
index c2c59891a8a1..ce774fd32887 100644
--- a/include/expression.h
+++ b/include/expression.h
@@ -519,7 +519,6 @@ struct expr *range_expr_to_prefix(struct expr *range);
 
 extern struct expr *compound_expr_alloc(const struct location *loc,
 					enum expr_types etypes);
-extern void compound_expr_add(struct expr *compound, struct expr *expr);
 extern void compound_expr_remove(struct expr *compound, struct expr *expr);
 extern void list_expr_sort(struct list_head *head);
 extern void list_splice_sorted(struct list_head *list, struct list_head *head);
diff --git a/src/expression.c b/src/expression.c
index 6c5140b749f9..ee1fe34e2de9 100644
--- a/src/expression.c
+++ b/src/expression.c
@@ -1048,12 +1048,6 @@ static void compound_expr_print(const struct expr *expr, const char *delim,
 	}
 }
 
-void compound_expr_add(struct expr *compound, struct expr *expr)
-{
-	list_add_tail(&expr->list, &compound->expr_set.expressions);
-	compound->expr_set.size++;
-}
-
 void compound_expr_remove(struct expr *compound, struct expr *expr)
 {
 	compound->expr_set.size--;
-- 
2.30.2

[PATCH nft,v2 08/11] expression: replace compound_expr_remove() by type safe function

[Pablo Neira Ayuso]

Replace this function by {list,concat,set}_expr_remove() to validate
expression type.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 include/expression.h |  4 +++-
 src/expression.c     | 24 ++++++++++++++++++------
 src/segtree.c        |  6 +++---
 3 files changed, 24 insertions(+), 10 deletions(-)

diff --git a/include/expression.h b/include/expression.h
index ce774fd32887..198ead603da6 100644
--- a/include/expression.h
+++ b/include/expression.h
@@ -519,21 +519,23 @@ struct expr *range_expr_to_prefix(struct expr *range);
 
 extern struct expr *compound_expr_alloc(const struct location *loc,
 					enum expr_types etypes);
-extern void compound_expr_remove(struct expr *compound, struct expr *expr);
 extern void list_expr_sort(struct list_head *head);
 extern void list_splice_sorted(struct list_head *list, struct list_head *head);
 
 extern struct expr *concat_expr_alloc(const struct location *loc);
 void concat_expr_add(struct expr *concat, struct expr *item);
+void concat_expr_remove(struct expr *concat, struct expr *expr);
 
 extern struct expr *list_expr_alloc(const struct location *loc);
 void list_expr_add(struct expr *expr, struct expr *item);
+void list_expr_remove(struct expr *expr, struct expr *item);
 struct expr *list_expr_to_binop(struct expr *expr);
 
 extern struct expr *set_expr_alloc(const struct location *loc,
 				   const struct set *set);
 void __set_expr_add(struct expr *set, struct expr *elem);
 void set_expr_add(struct expr *set, struct expr *elem);
+void set_expr_remove(struct expr *expr, struct expr *item);
 
 extern void concat_range_aggregate(struct expr *set);
 extern void interval_map_decompose(struct expr *set);
diff --git a/src/expression.c b/src/expression.c
index ee1fe34e2de9..582d0e7f8d66 100644
--- a/src/expression.c
+++ b/src/expression.c
@@ -1048,12 +1048,6 @@ static void compound_expr_print(const struct expr *expr, const char *delim,
 	}
 }
 
-void compound_expr_remove(struct expr *compound, struct expr *expr)
-{
-	compound->expr_set.size--;
-	list_del(&expr->list);
-}
-
 static void concat_expr_destroy(struct expr *expr)
 {
 	compound_expr_destroy(expr);
@@ -1247,6 +1241,12 @@ void concat_expr_add(struct expr *concat, struct expr *item)
 	expr_concat->size++;
 }
 
+void concat_expr_remove(struct expr *concat, struct expr *expr)
+{
+	expr_concat(concat)->size--;
+	list_del(&expr->list);
+}
+
 static void list_expr_print(const struct expr *expr, struct output_ctx *octx)
 {
 	compound_expr_print(expr, ",", octx);
@@ -1283,6 +1283,12 @@ void list_expr_add(struct expr *expr, struct expr *item)
 	expr_list->size++;
 }
 
+void list_expr_remove(struct expr *list, struct expr *expr)
+{
+	expr_list(list)->size--;
+	list_del(&expr->list);
+}
+
 /* list is assumed to have two items at least, otherwise extend this! */
 struct expr *list_expr_to_binop(struct expr *expr)
 {
@@ -1433,6 +1439,12 @@ void set_expr_add(struct expr *set, struct expr *elem)
 	expr_set->size++;
 }
 
+void set_expr_remove(struct expr *set, struct expr *expr)
+{
+	expr_set(set)->size--;
+	list_del(&expr->list);
+}
+
 static void mapping_expr_print(const struct expr *expr, struct output_ctx *octx)
 {
 	expr_print(expr->left, octx);
diff --git a/src/segtree.c b/src/segtree.c
index e6b14f5dd120..88207a3987b8 100644
--- a/src/segtree.c
+++ b/src/segtree.c
@@ -448,13 +448,13 @@ next:
 			mpz_clear(range);
 
 			r2 = list_entry(r2_next, typeof(*r2), list);
-			compound_expr_remove(expr_value(start), r1);
+			concat_expr_remove(expr_value(start), r1);
 
 			if (free_r1)
 				expr_free(r1);
 		}
 
-		compound_expr_remove(set, start);
+		set_expr_remove(set, start);
 		expr_free(start);
 		start = NULL;
 	}
@@ -584,7 +584,7 @@ void interval_map_decompose(struct expr *set)
 			catchall = i;
 			continue;
 		}
-		compound_expr_remove(set, i);
+		set_expr_remove(set, i);
 		elements[n++] = i;
 	}
 	qsort(elements, n, sizeof(elements[0]), expr_value_cmp);
-- 
2.30.2

[PATCH nft,v2 09/11] expression: replace compound_expr_destroy() by type safe funtion

[Pablo Neira Ayuso]

Replace it by {set,list,concat}_expr_destroy() to validate type.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 src/expression.c | 35 +++++++++++++++++++++++------------
 1 file changed, 23 insertions(+), 12 deletions(-)

diff --git a/src/expression.c b/src/expression.c
index 582d0e7f8d66..8d604fba265e 100644
--- a/src/expression.c
+++ b/src/expression.c
@@ -90,7 +90,7 @@ void expr_free(struct expr *expr)
 	datatype_free(expr->dtype);
 
 	/* EXPR_INVALID expressions lack ->ops structure.
-	 * This happens for compound types.
+	 * This happens for set, list and concat types.
 	 */
 	if (expr->etype != EXPR_INVALID)
 		expr_destroy(expr);
@@ -1027,14 +1027,6 @@ struct expr *compound_expr_alloc(const struct location *loc,
 	return expr;
 }
 
-static void compound_expr_destroy(struct expr *expr)
-{
-	struct expr *i, *next;
-
-	list_for_each_entry_safe(i, next, &expr->expr_set.expressions, list)
-		expr_free(i);
-}
-
 static void compound_expr_print(const struct expr *expr, const char *delim,
 				 struct output_ctx *octx)
 {
@@ -1050,7 +1042,10 @@ static void compound_expr_print(const struct expr *expr, const char *delim,
 
 static void concat_expr_destroy(struct expr *expr)
 {
-	compound_expr_destroy(expr);
+	struct expr *i, *next;
+
+	list_for_each_entry_safe(i, next, &expr_concat(expr)->expressions, list)
+		expr_free(i);
 }
 
 static void concat_expr_print(const struct expr *expr, struct output_ctx *octx)
@@ -1261,13 +1256,21 @@ static void list_expr_clone(struct expr *new, const struct expr *expr)
 		list_expr_add(new, expr_clone(i));
 }
 
+static void list_expr_destroy(struct expr *expr)
+{
+	struct expr *i, *next;
+
+	list_for_each_entry_safe(i, next, &expr_list(expr)->expressions, list)
+		expr_free(i);
+}
+
 static const struct expr_ops list_expr_ops = {
 	.type		= EXPR_LIST,
 	.name		= "list",
 	.print		= list_expr_print,
 	.json		= list_expr_json,
 	.clone		= list_expr_clone,
-	.destroy	= compound_expr_destroy,
+	.destroy	= list_expr_destroy,
 };
 
 struct expr *list_expr_alloc(const struct location *loc)
@@ -1393,6 +1396,14 @@ static void set_expr_clone(struct expr *new, const struct expr *expr)
 		set_expr_add(new, expr_clone(i));
 }
 
+static void set_expr_destroy(struct expr *expr)
+{
+	struct expr *i, *next;
+
+	list_for_each_entry_safe(i, next, &expr_set(expr)->expressions, list)
+		expr_free(i);
+}
+
 static void set_expr_set_type(const struct expr *expr,
 			      const struct datatype *dtype,
 			      enum byteorder byteorder)
@@ -1410,7 +1421,7 @@ static const struct expr_ops set_expr_ops = {
 	.json		= set_expr_json,
 	.set_type	= set_expr_set_type,
 	.clone		= set_expr_clone,
-	.destroy	= compound_expr_destroy,
+	.destroy	= set_expr_destroy,
 };
 
 struct expr *set_expr_alloc(const struct location *loc, const struct set *set)
-- 
2.30.2

[PATCH nft,v2 10/11] expression: replace compound_expr_print() by type safe function

[Pablo Neira Ayuso]

Replace compound_expr_print() by {list,set,concat}_expr_print() to
validate expression type.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 src/expression.c | 31 ++++++++++++++++---------------
 1 file changed, 16 insertions(+), 15 deletions(-)

diff --git a/src/expression.c b/src/expression.c
index 8d604fba265e..c0ab5d5598f2 100644
--- a/src/expression.c
+++ b/src/expression.c
@@ -1027,19 +1027,6 @@ struct expr *compound_expr_alloc(const struct location *loc,
 	return expr;
 }
 
-static void compound_expr_print(const struct expr *expr, const char *delim,
-				 struct output_ctx *octx)
-{
-	const struct expr *i;
-	const char *d = "";
-
-	list_for_each_entry(i, &expr->expr_set.expressions, list) {
-		nft_print(octx, "%s", d);
-		expr_print(i, octx);
-		d = delim;
-	}
-}
-
 static void concat_expr_destroy(struct expr *expr)
 {
 	struct expr *i, *next;
@@ -1050,7 +1037,14 @@ static void concat_expr_destroy(struct expr *expr)
 
 static void concat_expr_print(const struct expr *expr, struct output_ctx *octx)
 {
-	compound_expr_print(expr, " . ", octx);
+	const struct expr *i;
+	const char *d = "";
+
+	list_for_each_entry(i, &expr_concat(expr)->expressions, list) {
+		nft_print(octx, "%s", d);
+		expr_print(i, octx);
+		d = " . ";
+	}
 }
 
 static void concat_expr_clone(struct expr *new, const struct expr *expr)
@@ -1244,7 +1238,14 @@ void concat_expr_remove(struct expr *concat, struct expr *expr)
 
 static void list_expr_print(const struct expr *expr, struct output_ctx *octx)
 {
-	compound_expr_print(expr, ",", octx);
+	const struct expr *i;
+	const char *d = "";
+
+	list_for_each_entry(i, &expr_list(expr)->expressions, list) {
+		nft_print(octx, "%s", d);
+		expr_print(i, octx);
+		d = ",";
+	}
 }
 
 static void list_expr_clone(struct expr *new, const struct expr *expr)
-- 
2.30.2

[PATCH nft,v2 11/11] src: replace compound_expr_alloc() by type safe function

[Pablo Neira Ayuso]

Replace compound_expr_alloc() by {set,list,concat}_expr_alloc() to
validate expression type.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 include/expression.h |  2 --
 src/evaluate.c       |  2 +-
 src/expression.c     | 30 ++++++++++++++++--------------
 src/parser_bison.y   |  8 ++++----
 src/parser_json.c    |  2 +-
 5 files changed, 22 insertions(+), 22 deletions(-)

diff --git a/include/expression.h b/include/expression.h
index 198ead603da6..650063bd0e9e 100644
--- a/include/expression.h
+++ b/include/expression.h
@@ -517,8 +517,6 @@ extern struct expr *range_expr_alloc(const struct location *loc,
 				     struct expr *low, struct expr *high);
 struct expr *range_expr_to_prefix(struct expr *range);
 
-extern struct expr *compound_expr_alloc(const struct location *loc,
-					enum expr_types etypes);
 extern void list_expr_sort(struct list_head *head);
 extern void list_splice_sorted(struct list_head *list, struct list_head *head);
 
diff --git a/src/evaluate.c b/src/evaluate.c
index fc9177d3a91e..f4420866bf22 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -5475,7 +5475,7 @@ static struct expr *expr_set_to_list(struct eval_ctx *ctx, struct expr *dev_expr
 
 	loc = dev_expr->location;
 	expr_free(dev_expr);
-	dev_expr = compound_expr_alloc(&loc, EXPR_LIST);
+	dev_expr = list_expr_alloc(&loc);
 	list_splice_init(&tmp, &expr_list(dev_expr)->expressions);
 
 	return dev_expr;
diff --git a/src/expression.c b/src/expression.c
index c0ab5d5598f2..c0d644249a71 100644
--- a/src/expression.c
+++ b/src/expression.c
@@ -1016,17 +1016,6 @@ struct expr *range_expr_alloc(const struct location *loc,
 	return expr;
 }
 
-struct expr *compound_expr_alloc(const struct location *loc,
-				 enum expr_types etype)
-{
-	struct expr *expr;
-
-	expr = expr_alloc(loc, etype, &invalid_type, BYTEORDER_INVALID, 0);
-	/* same layout for EXPR_CONCAT, EXPR_SET and EXPR_LIST. */
-	init_list_head(&expr->expr_set.expressions);
-	return expr;
-}
-
 static void concat_expr_destroy(struct expr *expr)
 {
 	struct expr *i, *next;
@@ -1219,7 +1208,12 @@ static const struct expr_ops concat_expr_ops = {
 
 struct expr *concat_expr_alloc(const struct location *loc)
 {
-	return compound_expr_alloc(loc, EXPR_CONCAT);
+	struct expr *expr;
+
+	expr = expr_alloc(loc, EXPR_CONCAT, &invalid_type, BYTEORDER_INVALID, 0);
+	init_list_head(&expr_concat(expr)->expressions);
+
+	return expr;
 }
 
 void concat_expr_add(struct expr *concat, struct expr *item)
@@ -1276,7 +1270,12 @@ static const struct expr_ops list_expr_ops = {
 
 struct expr *list_expr_alloc(const struct location *loc)
 {
-	return compound_expr_alloc(loc, EXPR_LIST);
+	struct expr *expr;
+
+	expr = expr_alloc(loc, EXPR_LIST, &invalid_type, BYTEORDER_INVALID, 0);
+	init_list_head(&expr_list(expr)->expressions);
+
+	return expr;
 }
 
 void list_expr_add(struct expr *expr, struct expr *item)
@@ -1427,7 +1426,10 @@ static const struct expr_ops set_expr_ops = {
 
 struct expr *set_expr_alloc(const struct location *loc, const struct set *set)
 {
-	struct expr *set_expr = compound_expr_alloc(loc, EXPR_SET);
+	struct expr *set_expr;
+
+	set_expr = expr_alloc(loc, EXPR_SET, &invalid_type, BYTEORDER_INVALID, 0);
+	init_list_head(&expr_set(set_expr)->expressions);
 
 	if (!set)
 		return set_expr;
diff --git a/src/parser_bison.y b/src/parser_bison.y
index 6d4eb98f3b5a..9fd2ab9e7117 100644
--- a/src/parser_bison.y
+++ b/src/parser_bison.y
@@ -2462,7 +2462,7 @@ flowtable_expr		:	'{'	flowtable_list_expr	'}'
 
 flowtable_list_expr	:	flowtable_expr_member
 			{
-				$$ = compound_expr_alloc(&@$, EXPR_LIST);
+				$$ = list_expr_alloc(&@$);
 				list_expr_add($$, $1);
 			}
 			|	flowtable_list_expr	COMMA	flowtable_expr_member
@@ -2801,14 +2801,14 @@ dev_spec		:	DEVICE	string
 				if (!expr)
 					YYERROR;
 
-				$$ = compound_expr_alloc(&@$, EXPR_LIST);
+				$$ = list_expr_alloc(&@$);
 				list_expr_add($$, expr);
 
 			}
 			|	DEVICE	variable_expr
 			{
 				datatype_set($2->sym->expr, &ifname_type);
-				$$ = compound_expr_alloc(&@$, EXPR_LIST);
+				$$ = list_expr_alloc(&@$);
 				list_expr_add($$, $2);
 			}
 			|	DEVICES		'='	flowtable_expr
@@ -4697,7 +4697,7 @@ set_rhs_expr		:	concat_rhs_expr
 
 initializer_expr	:	rhs_expr
 			|	list_rhs_expr
-			|	'{' '}'		{ $$ = compound_expr_alloc(&@$, EXPR_SET); }
+			|	'{' '}'		{ $$ = set_expr_alloc(&@$, NULL); }
 			|	DASH	NUM
 			{
 				int32_t num = -$2;
diff --git a/src/parser_json.c b/src/parser_json.c
index 17e13ebe4458..cff27a764a9e 100644
--- a/src/parser_json.c
+++ b/src/parser_json.c
@@ -2990,7 +2990,7 @@ static struct expr *ifname_expr_alloc(struct json_ctx *ctx,
 
 static struct expr *json_parse_devs(struct json_ctx *ctx, json_t *root)
 {
-	struct expr *tmp, *expr = compound_expr_alloc(int_loc, EXPR_LIST);
+	struct expr *tmp, *expr = list_expr_alloc(int_loc);
 	const char *dev;
 	json_t *value;
 	size_t index;
-- 
2.30.2

Re: [PATCH nft,v2 00/11] replace compound_expr_*() by type safe function

[Pablo Neira Ayuso]

On Thu, Aug 21, 2025 at 11:23:19AM +0200, Pablo Neira Ayuso wrote:
> Hi,
> 
> This is v2 with minimal changes wrt. previous series.
> 
> The aim is to replace (and remove) the existing compound_expr_*()
> helper functions which is common to set, list and concat expressions
> by expression type safe variants that validate the expression type.
> 
> I will hold on with this until nftables 1.1.5 including the fix for
> the JSON regression is released.

I have applied this series.