src: detach set, list and concatenation expression layout

These three expressions use the same layout, but they have a different
purpose. Several fields are specific of a given expression:

- set_flags is only required by set expressions.
- field_len and field_count are only used by concatenation expressions.

Add accessors to validate the expression type before accessing the union
fields:

 #define expr_set(__expr)       (assert((__expr)->etype == EXPR_SET), &(__expr)->expr_set)
 #define expr_concat(__expr)    (assert((__expr)->etype == EXPR_CONCAT), &(__expr)->expr_concat)
 #define expr_list(__expr)      (assert((__expr)->etype == EXPR_LIST), &(__expr)->expr_list)

This should help catch subtle bugs due to type confusion.

assert() could be later enabled only in debugging builds to run tests,
keep it by now.

compound_expr_*() still works and it needs the same initial layout for
all of these expressions:

      struct list_head        expressions;
      unsigned int            size;

This is implicitly reducing the size of one of the largest structs
in the union area of struct expr, still EXPR_SET_ELEM remains the
largest so no gain is achieved in this iteration.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

1 files changed, 11 insertions, 11 deletions

diff --git a/src/optimize.c b/src/optimize.c
index 89ba0d9d..40756cec 100644
--- a/src/optimize.c
+++ b/src/optimize.c
@@ -565,7 +565,7 @@ static void merge_expr_stmts(const struct optimize_ctx *ctx,
 	uint32_t i;
 
 	set = set_expr_alloc(&internal_location, NULL);
-	set->set_flags |= NFT_SET_ANONYMOUS;
+	expr_set(set)->set_flags |= NFT_SET_ANONYMOUS;
 
 	expr_a = stmt_a->expr->right;
 	elem = set_elem_expr_alloc(&internal_location, expr_get(expr_a));
@@ -588,7 +588,7 @@ static void merge_vmap(const struct optimize_ctx *ctx,
 	struct expr *mappings, *mapping, *expr;
 
 	mappings = stmt_b->expr->mappings;
-	list_for_each_entry(expr, &mappings->expressions, list) {
+	list_for_each_entry(expr, &expr_set(mappings)->expressions, list) {
 		mapping = expr_clone(expr);
 		compound_expr_add(stmt_a->expr->mappings, mapping);
 	}
@@ -654,7 +654,7 @@ static void __merge_concat(const struct optimize_ctx *ctx, uint32_t i,
 			stmt_a = ctx->stmt_matrix[i][merge->stmt[k]];
 			switch (stmt_a->expr->right->etype) {
 			case EXPR_SET:
-				list_for_each_entry(expr, &stmt_a->expr->right->expressions, list) {
+				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);
@@ -673,7 +673,7 @@ static void __merge_concat(const struct optimize_ctx *ctx, uint32_t i,
 				compound_expr_add(concat, clone);
 				break;
 			case EXPR_LIST:
-				list_for_each_entry(expr, &stmt_a->expr->right->expressions, 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);
@@ -727,7 +727,7 @@ static void merge_concat_stmts(const struct optimize_ctx *ctx,
 
 	/* build set data contenation, eg. { eth0 . 1.1.1.1 . 22 } */
 	set = set_expr_alloc(&internal_location, NULL);
-	set->set_flags |= NFT_SET_ANONYMOUS;
+	expr_set(set)->set_flags |= NFT_SET_ANONYMOUS;
 
 	for (i = from; i <= to; i++)
 		__merge_concat_stmts(ctx, i, merge, set);
@@ -750,7 +750,7 @@ static void build_verdict_map(struct expr *expr, struct stmt *verdict,
 
 	switch (expr->etype) {
 	case EXPR_LIST:
-		list_for_each_entry(item, &expr->expressions, list) {
+		list_for_each_entry(item, &expr_list(expr)->expressions, list) {
 			elem = set_elem_expr_alloc(&internal_location, expr_get(item));
 			if (counter) {
 				counter_elem = counter_stmt_alloc(&counter->location);
@@ -764,7 +764,7 @@ static void build_verdict_map(struct expr *expr, struct stmt *verdict,
 		stmt_free(counter);
 		break;
 	case EXPR_SET:
-		list_for_each_entry(item, &expr->expressions, list) {
+		list_for_each_entry(item, &expr_set(expr)->expressions, list) {
 			elem = set_elem_expr_alloc(&internal_location, expr_get(item->key));
 			if (counter) {
 				counter_elem = counter_stmt_alloc(&counter->location);
@@ -851,7 +851,7 @@ static void merge_stmts_vmap(const struct optimize_ctx *ctx,
 	assert(k >= 0);
 
 	set = set_expr_alloc(&internal_location, NULL);
-	set->set_flags |= NFT_SET_ANONYMOUS;
+	expr_set(set)->set_flags |= NFT_SET_ANONYMOUS;
 
 	expr_a = stmt_a->expr->right;
 	verdict_a = ctx->stmt_matrix[from][k];
@@ -925,7 +925,7 @@ static void merge_concat_stmts_vmap(const struct optimize_ctx *ctx,
 
 	/* build set data contenation, eg. { eth0 . 1.1.1.1 . 22 : accept } */
 	set = set_expr_alloc(&internal_location, NULL);
-	set->set_flags |= NFT_SET_ANONYMOUS;
+	expr_set(set)->set_flags |= NFT_SET_ANONYMOUS;
 
 	for (i = from; i <= to; i++) {
 		verdict = ctx->stmt_matrix[i][k];
@@ -1050,7 +1050,7 @@ static void merge_nat(const struct optimize_ctx *ctx,
 	assert(k >= 0);
 
 	set = set_expr_alloc(&internal_location, NULL);
-	set->set_flags |= NFT_SET_ANONYMOUS;
+	expr_set(set)->set_flags |= NFT_SET_ANONYMOUS;
 
 	for (i = from; i <= to; i++) {
 		stmt = ctx->stmt_matrix[i][merge->stmt[0]];
@@ -1102,7 +1102,7 @@ static void merge_concat_nat(const struct optimize_ctx *ctx,
 	assert(k >= 0);
 
 	set = set_expr_alloc(&internal_location, NULL);
-	set->set_flags |= NFT_SET_ANONYMOUS;
+	expr_set(set)->set_flags |= NFT_SET_ANONYMOUS;
 
 	for (i = from; i <= to; i++) {