[PATCH 0/6] netlink: style and readability fixes

[PATCH 0/6] netlink: style and readability fixes

[Patrick McHardy]

Some cleanups for netlink related functions as preparation for the
concat support, mainly consisting of getting rid of style differences,
removing overly long lines and similar reformatting.

[PATCH 1/6] netlink: remove unnecessary temporary variable

[Patrick McHardy]

Waste less space and return the results of the batch/compat
functions directly.

Signed-off-by: Patrick McHardy <kaber@trash.net>
---
 src/netlink.c | 72 +++++++++++++++--------------------------------------------
 1 file changed, 18 insertions(+), 54 deletions(-)

diff --git a/src/netlink.c b/src/netlink.c
index 5347372..ceb1d3f 100644
--- a/src/netlink.c
+++ b/src/netlink.c
@@ -546,14 +546,10 @@ int netlink_add_chain(struct netlink_ctx *ctx, const struct handle *h,
 		      const struct location *loc, const struct chain *chain,
 		      bool excl)
 {
-	int ret;
-
 	if (ctx->batch_supported)
-		ret = netlink_add_chain_batch(ctx, h, loc, chain, excl);
+		return netlink_add_chain_batch(ctx, h, loc, chain, excl);
 	else
-		ret = netlink_add_chain_compat(ctx, h, loc, chain, excl);
-
-	return ret;
+		return netlink_add_chain_compat(ctx, h, loc, chain, excl);
 }
 
 static int netlink_rename_chain_compat(struct netlink_ctx *ctx,
@@ -600,14 +596,10 @@ static int netlink_rename_chain_batch(struct netlink_ctx *ctx,
 int netlink_rename_chain(struct netlink_ctx *ctx, const struct handle *h,
 			 const struct location *loc, const char *name)
 {
-	int ret;
-
 	if (ctx->batch_supported)
-		ret = netlink_rename_chain_batch(ctx, h, loc, name);
+		return netlink_rename_chain_batch(ctx, h, loc, name);
 	else
-		ret = netlink_rename_chain_compat(ctx, h, loc, name);
-
-	return ret;
+		return netlink_rename_chain_compat(ctx, h, loc, name);
 }
 
 static int netlink_del_chain_compat(struct netlink_ctx *ctx,
@@ -651,14 +643,10 @@ static int netlink_del_chain_batch(struct netlink_ctx *ctx,
 int netlink_delete_chain(struct netlink_ctx *ctx, const struct handle *h,
 			 const struct location *loc)
 {
-	int ret;
-
 	if (ctx->batch_supported)
-		ret = netlink_del_chain_batch(ctx, h, loc);
+		return netlink_del_chain_batch(ctx, h, loc);
 	else
-		ret = netlink_del_chain_compat(ctx, h, loc);
-
-	return ret;
+		return netlink_del_chain_compat(ctx, h, loc);
 }
 
 static struct chain *netlink_delinearize_chain(struct netlink_ctx *ctx,
@@ -823,14 +811,10 @@ int netlink_add_table(struct netlink_ctx *ctx, const struct handle *h,
 		      const struct location *loc,
 		      const struct table *table, bool excl)
 {
-	int ret;
-
 	if (ctx->batch_supported)
-		ret = netlink_add_table_batch(ctx, h, loc, table, excl);
+		return netlink_add_table_batch(ctx, h, loc, table, excl);
 	else
-		ret = netlink_add_table_compat(ctx, h, loc, table, excl);
-
-	return ret;
+		return netlink_add_table_compat(ctx, h, loc, table, excl);
 }
 
 static int netlink_del_table_compat(struct netlink_ctx *ctx,
@@ -871,14 +855,10 @@ static int netlink_del_table_batch(struct netlink_ctx *ctx,
 int netlink_delete_table(struct netlink_ctx *ctx, const struct handle *h,
 			 const struct location *loc)
 {
-	int ret;
-
 	if (ctx->batch_supported)
-		ret = netlink_del_table_batch(ctx, h, loc);
+		return netlink_del_table_batch(ctx, h, loc);
 	else
-		ret = netlink_del_table_compat(ctx, h, loc);
-
-	return ret;
+		return netlink_del_table_compat(ctx, h, loc);
 }
 
 void netlink_dump_table(struct nft_table *nlt)
@@ -1145,14 +1125,10 @@ static int netlink_add_set_batch(struct netlink_ctx *ctx,
 int netlink_add_set(struct netlink_ctx *ctx, const struct handle *h,
 		    struct set *set)
 {
-	int ret;
-
 	if (ctx->batch_supported)
-		ret = netlink_add_set_batch(ctx, h, set);
+		return netlink_add_set_batch(ctx, h, set);
 	else
-		ret = netlink_add_set_compat(ctx, h, set);
-
-	return ret;
+		return netlink_add_set_compat(ctx, h, set);
 }
 
 static int netlink_del_set_compat(struct netlink_ctx *ctx,
@@ -1192,14 +1168,10 @@ static int netlink_del_set_batch(struct netlink_ctx *ctx,
 int netlink_delete_set(struct netlink_ctx *ctx, const struct handle *h,
 		       const struct location *loc)
 {
-	int ret;
-
 	if (ctx->batch_supported)
-		ret = netlink_del_set_batch(ctx, h, loc);
+		return netlink_del_set_batch(ctx, h, loc);
 	else
-		ret = netlink_del_set_compat(ctx, h, loc);
-
-	return ret;
+		return netlink_del_set_compat(ctx, h, loc);
 }
 
 static int list_set_cb(struct nft_set *nls, void *arg)
@@ -1316,14 +1288,10 @@ static int netlink_add_setelems_compat(struct netlink_ctx *ctx,
 int netlink_add_setelems(struct netlink_ctx *ctx, const struct handle *h,
 			 const struct expr *expr)
 {
-	int ret;
-
 	if (ctx->batch_supported)
-		ret = netlink_add_setelems_batch(ctx, h, expr);
+		return netlink_add_setelems_batch(ctx, h, expr);
 	else
-		ret = netlink_add_setelems_compat(ctx, h, expr);
-
-	return ret;
+		return netlink_add_setelems_compat(ctx, h, expr);
 }
 
 static int netlink_del_setelems_batch(struct netlink_ctx *ctx,
@@ -1418,14 +1386,10 @@ out:
 int netlink_delete_setelems(struct netlink_ctx *ctx, const struct handle *h,
 			    const struct expr *expr)
 {
-	int ret;
-
 	if (ctx->batch_supported)
-		ret = netlink_del_setelems_batch(ctx, h, expr);
+		return netlink_del_setelems_batch(ctx, h, expr);
 	else
-		ret = netlink_del_setelems_compat(ctx, h, expr);
-
-	return ret;
+		return netlink_del_setelems_compat(ctx, h, expr);
 }
 
 static int list_setelem_cb(struct nft_set_elem *nlse, void *arg)
-- 
2.1.0

[PATCH 2/6] netlink: style fixes

[Patrick McHardy]

We include an empty line between variable definitions and code.

Signed-off-by: Patrick McHardy <kaber@trash.net>
---
 src/netlink.c | 30 ++++++++++++++++++------------
 1 file changed, 18 insertions(+), 12 deletions(-)

diff --git a/src/netlink.c b/src/netlink.c
index ceb1d3f..cb22c30 100644
--- a/src/netlink.c
+++ b/src/netlink.c
@@ -48,10 +48,11 @@ const struct location netlink_location = {
 
 static struct mnl_socket *nfsock_open(void)
 {
-	struct mnl_socket *s = mnl_socket_open(NETLINK_NETFILTER);
+	struct mnl_socket *s;
+
+	s = mnl_socket_open(NETLINK_NETFILTER);
 	if (s == NULL)
 		netlink_open_error();
-
 	return s;
 }
 
@@ -1480,10 +1481,11 @@ struct nft_ruleset *netlink_dump_ruleset(struct netlink_ctx *ctx,
 
 static struct nft_table *netlink_table_alloc(const struct nlmsghdr *nlh)
 {
-	struct nft_table *nlt = nft_table_alloc();
+	struct nft_table *nlt;
+
+	nlt = nft_table_alloc();
 	if (nlt == NULL)
 		memory_allocation_error();
-
 	if (nft_table_nlmsg_parse(nlh, nlt) < 0)
 		netlink_abi_error();
 
@@ -1492,10 +1494,11 @@ static struct nft_table *netlink_table_alloc(const struct nlmsghdr *nlh)
 
 static struct nft_chain *netlink_chain_alloc(const struct nlmsghdr *nlh)
 {
-	struct nft_chain *nlc = nft_chain_alloc();
+	struct nft_chain *nlc;
+
+	nlc = nft_chain_alloc();
 	if (nlc == NULL)
 		memory_allocation_error();
-
 	if (nft_chain_nlmsg_parse(nlh, nlc) < 0)
 		netlink_abi_error();
 
@@ -1504,10 +1507,11 @@ static struct nft_chain *netlink_chain_alloc(const struct nlmsghdr *nlh)
 
 static struct nft_set *netlink_set_alloc(const struct nlmsghdr *nlh)
 {
-	struct nft_set *nls = nft_set_alloc();
+	struct nft_set *nls;
+
+	nls = nft_set_alloc();
 	if (nls == NULL)
 		memory_allocation_error();
-
 	if (nft_set_nlmsg_parse(nlh, nls) < 0)
 		netlink_abi_error();
 
@@ -1516,10 +1520,11 @@ static struct nft_set *netlink_set_alloc(const struct nlmsghdr *nlh)
 
 static struct nft_set *netlink_setelem_alloc(const struct nlmsghdr *nlh)
 {
-	struct nft_set *nls = nft_set_alloc();
+	struct nft_set *nls;
+
+	nls = nft_set_alloc();
 	if (nls == NULL)
 		memory_allocation_error();
-
 	if (nft_set_elems_nlmsg_parse(nlh, nls) < 0)
 		netlink_abi_error();
 
@@ -1528,10 +1533,11 @@ static struct nft_set *netlink_setelem_alloc(const struct nlmsghdr *nlh)
 
 static struct nft_rule *netlink_rule_alloc(const struct nlmsghdr *nlh)
 {
-	struct nft_rule *nlr = nft_rule_alloc();
+	struct nft_rule *nlr;
+
+	nlr = nft_rule_alloc();
 	if (nlr == NULL)
 		memory_allocation_error();
-
 	if (nft_rule_nlmsg_parse(nlh, nlr) < 0)
 		netlink_abi_error();
 
-- 
2.1.0

[PATCH 3/6] netlink: style fixes

[Patrick McHardy]

Remove style discrepancies between different netlink I/O functions:

- we don't use brackets for single line statements
- most functions don't have a newline between error reporting and exit

Signed-off-by: Patrick McHardy <kaber@trash.net>
---
 src/netlink.c | 39 +++++++++++++++------------------------
 1 file changed, 15 insertions(+), 24 deletions(-)

diff --git a/src/netlink.c b/src/netlink.c
index cb22c30..af56d22 100644
--- a/src/netlink.c
+++ b/src/netlink.c
@@ -128,6 +128,7 @@ struct nft_table *alloc_nft_table(const struct handle *h)
 	nft_table_attr_set_u32(nlt, NFT_TABLE_ATTR_FAMILY, h->family);
 	if (h->table != NULL)
 		nft_table_attr_set(nlt, NFT_TABLE_ATTR_NAME, h->table);
+
 	return nlt;
 }
 
@@ -145,6 +146,7 @@ struct nft_chain *alloc_nft_chain(const struct handle *h)
 		nft_chain_attr_set_u64(nlc, NFT_CHAIN_ATTR_HANDLE, h->handle);
 	if (h->chain != NULL)
 		nft_chain_attr_set_str(nlc, NFT_CHAIN_ATTR_NAME, h->chain);
+
 	return nlc;
 }
 
@@ -164,10 +166,10 @@ struct nft_rule *alloc_nft_rule(const struct handle *h)
 		nft_rule_attr_set_u64(nlr, NFT_RULE_ATTR_HANDLE, h->handle);
 	if (h->position)
 		nft_rule_attr_set_u64(nlr, NFT_RULE_ATTR_POSITION, h->position);
-	if (h->comment) {
+	if (h->comment)
 		nft_rule_attr_set_data(nlr, NFT_RULE_ATTR_USERDATA,
 				       h->comment, strlen(h->comment) + 1);
-	}
+
 	return nlr;
 }
 
@@ -178,6 +180,7 @@ struct nft_rule_expr *alloc_nft_expr(const char *name)
 	nle = nft_rule_expr_alloc(name);
 	if (nle == NULL)
 		memory_allocation_error();
+
 	return nle;
 }
 
@@ -223,10 +226,9 @@ static struct nft_set_elem *alloc_nft_setelem(const struct expr *expr)
 		case EXPR_VERDICT:
 			nft_set_elem_attr_set_u32(nlse, NFT_SET_ELEM_ATTR_VERDICT,
 						  expr->right->verdict);
-			if (expr->chain != NULL) {
+			if (expr->chain != NULL)
 				nft_set_elem_attr_set(nlse, NFT_SET_ELEM_ATTR_CHAIN,
 						nld.chain, strlen(nld.chain));
-			}
 			break;
 		case EXPR_VALUE:
 			nft_set_elem_attr_set(nlse, NFT_SET_ELEM_ATTR_DATA,
@@ -238,10 +240,9 @@ static struct nft_set_elem *alloc_nft_setelem(const struct expr *expr)
 		}
 	}
 
-	if (expr->flags & EXPR_F_INTERVAL_END) {
+	if (expr->flags & EXPR_F_INTERVAL_END)
 		nft_set_elem_attr_set_u32(nlse, NFT_SET_ELEM_ATTR_FLAGS,
 					  NFT_SET_ELEM_INTERVAL_END);
-	}
 
 	return nlse;
 }
@@ -364,11 +365,10 @@ int netlink_add_rule_batch(struct netlink_ctx *ctx,
 	netlink_linearize_rule(ctx, nlr, rule);
 	err = mnl_nft_rule_batch_add(nlr, flags | NLM_F_EXCL, ctx->seqnum);
 	nft_rule_free(nlr);
-	if (err < 0) {
+	if (err < 0)
 		netlink_io_error(ctx, &rule->location,
 				 "Could not add rule to batch: %s",
 				 strerror(errno));
-	}
 	return err;
 }
 
@@ -398,7 +398,6 @@ int netlink_del_rule_batch(struct netlink_ctx *ctx, const struct handle *h,
 	if (err < 0)
 		netlink_io_error(ctx, loc, "Could not delete rule to batch: %s",
 				 strerror(errno));
-
 	return err;
 }
 
@@ -536,10 +535,9 @@ static int netlink_add_chain_batch(struct netlink_ctx *ctx,
 				      ctx->seqnum);
 	nft_chain_free(nlc);
 
-	if (err < 0) {
+	if (err < 0)
 		netlink_io_error(ctx, loc, "Could not add chain: %s",
 				 strerror(errno));
-	}
 	return err;
 }
 
@@ -587,10 +585,9 @@ static int netlink_rename_chain_batch(struct netlink_ctx *ctx,
 	err = mnl_nft_chain_batch_add(nlc, 0, ctx->seqnum);
 	nft_chain_free(nlc);
 
-	if (err < 0) {
+	if (err < 0)
 		netlink_io_error(ctx, loc, "Could not rename chain: %s",
 				 strerror(errno));
-	}
 	return err;
 }
 
@@ -615,10 +612,9 @@ static int netlink_del_chain_compat(struct netlink_ctx *ctx,
 	err = mnl_nft_chain_delete(nf_sock, nlc, 0);
 	nft_chain_free(nlc);
 
-	if (err < 0) {
+	if (err < 0)
 		netlink_io_error(ctx, loc, "Could not delete chain: %s",
 				 strerror(errno));
-	}
 	return err;
 }
 
@@ -634,10 +630,9 @@ static int netlink_del_chain_batch(struct netlink_ctx *ctx,
 	err = mnl_nft_chain_batch_del(nlc, 0, ctx->seqnum);
 	nft_chain_free(nlc);
 
-	if (err < 0) {
+	if (err < 0)
 		netlink_io_error(ctx, loc, "Could not delete chain: %s",
 				 strerror(errno));
-	}
 	return err;
 }
 
@@ -801,10 +796,9 @@ static int netlink_add_table_batch(struct netlink_ctx *ctx,
 				      ctx->seqnum);
 	nft_table_free(nlt);
 
-	if (err < 0) {
+	if (err < 0)
 		netlink_io_error(ctx, loc, "Could not add table: %s",
 				 strerror(errno));
-	}
 	return err;
 }
 
@@ -846,10 +840,9 @@ static int netlink_del_table_batch(struct netlink_ctx *ctx,
 	err = mnl_nft_table_batch_del(nlt, 0, ctx->seqnum);
 	nft_table_free(nlt);
 
-	if (err < 0) {
+	if (err < 0)
 		netlink_io_error(ctx, loc, "Could not delete table: %s",
 				 strerror(errno));
-	}
 	return err;
 }
 
@@ -1114,10 +1107,9 @@ static int netlink_add_set_batch(struct netlink_ctx *ctx,
 	netlink_dump_set(nls);
 
 	err = mnl_nft_set_batch_add(nls, NLM_F_EXCL, ctx->seqnum);
-	if (err < 0) {
+	if (err < 0)
 		netlink_io_error(ctx, &set->location, "Could not add set: %s",
 				 strerror(errno));
-	}
 	nft_set_free(nls);
 
 	return err;
@@ -1457,7 +1449,6 @@ int netlink_flush_ruleset(struct netlink_ctx *ctx, const struct handle *h,
 	if (err < 0)
 		netlink_io_error(ctx, loc, "Could not flush the ruleset: %s",
 				 strerror(errno));
-
 	return err;
 }
 
-- 
2.1.0

[PATCH 4/6] netlink: readability fixes

[Patrick McHardy]

Improve readability by using local variables for netlink attributes,
ordering variables more logically, don't arbitrarily initialize
some variables in the definition section and in the body and generally
make similar functions look similar.

Signed-off-by: Patrick McHardy <kaber@trash.net>
---
 src/netlink.c | 127 ++++++++++++++++++++++++++++++++--------------------------
 1 file changed, 71 insertions(+), 56 deletions(-)

diff --git a/src/netlink.c b/src/netlink.c
index af56d22..84d9d27 100644
--- a/src/netlink.c
+++ b/src/netlink.c
@@ -432,11 +432,16 @@ static int list_rule_cb(struct nft_rule *nlr, void *arg)
 	struct netlink_ctx *ctx = arg;
 	const struct handle *h = ctx->data;
 	struct rule *rule;
+	const char *table, *chain;
+	uint32_t family;
+
+	family = nft_rule_attr_get_u32(nlr, NFT_RULE_ATTR_FAMILY);
+	table  = nft_rule_attr_get_str(nlr, NFT_RULE_ATTR_TABLE);
+	chain  = nft_rule_attr_get_str(nlr, NFT_RULE_ATTR_CHAIN);
 
-	if ((h->family != nft_rule_attr_get_u32(nlr, NFT_RULE_ATTR_FAMILY)) ||
-	    strcmp(nft_rule_attr_get_str(nlr, NFT_RULE_ATTR_TABLE), h->table) != 0 ||
-	    (h->chain &&
-	     strcmp(nft_rule_attr_get_str(nlr, NFT_RULE_ATTR_CHAIN), h->chain) != 0))
+	if (h->family != family ||
+	    strcmp(table, h->table) != 0 ||
+	    (h->chain && strcmp(chain, h->chain) != 0))
 		return 0;
 
 	netlink_dump_rule(nlr);
@@ -677,14 +682,17 @@ static int list_chain_cb(struct nft_chain *nlc, void *arg)
 {
 	struct netlink_ctx *ctx = arg;
 	const struct handle *h = ctx->data;
-	const char *table = nft_chain_attr_get_str(nlc, NFT_CHAIN_ATTR_TABLE);
-	const char *name = nft_chain_attr_get_str(nlc, NFT_CHAIN_ATTR_NAME);
+	const char *table;
+	const char *name;
 	struct chain *chain;
+	uint32_t family;
 
-	if ((h->family != nft_chain_attr_get_u32(nlc, NFT_CHAIN_ATTR_FAMILY)) ||
-	    strcmp(table, h->table) != 0)
-		return 0;
+	table  = nft_chain_attr_get_str(nlc, NFT_CHAIN_ATTR_TABLE);
+	name   = nft_chain_attr_get_str(nlc, NFT_CHAIN_ATTR_NAME);
+	family = nft_chain_attr_get_u32(nlc, NFT_CHAIN_ATTR_FAMILY);
 
+	if (h->family != family || strcmp(table, h->table) != 0)
+		return 0;
 	if (h->chain && strcmp(name, h->chain) != 0)
 		return 0;
 
@@ -740,16 +748,16 @@ int netlink_get_chain(struct netlink_ctx *ctx, const struct handle *h,
 	nlc = alloc_nft_chain(h);
 	err = mnl_nft_chain_get(nf_sock, nlc, 0);
 	if (err < 0) {
-		nft_chain_free(nlc);
-		return netlink_io_error(ctx, loc,
-					"Could not receive chain from kernel: %s",
-					strerror(errno));
+		netlink_io_error(ctx, loc,
+				 "Could not receive chain from kernel: %s",
+				 strerror(errno));
+		goto out;
 	}
 
 	chain = netlink_delinearize_chain(ctx, nlc);
 	list_add_tail(&chain->list, &ctx->list);
+out:
 	nft_chain_free(nlc);
-
 	return err;
 }
 
@@ -925,9 +933,9 @@ int netlink_get_table(struct netlink_ctx *ctx, const struct handle *h,
 	nft_table_free(nlt);
 
 	if (err < 0)
-		return netlink_io_error(ctx, loc,
-					"Could not receive table from kernel: %s",
-					strerror(errno));
+		netlink_io_error(ctx, loc,
+				 "Could not receive table from kernel: %s",
+				 strerror(errno));
 	return err;
 }
 
@@ -1015,10 +1023,10 @@ static struct set *netlink_delinearize_set(struct netlink_ctx *ctx,
 		xstrdup(nft_set_attr_get_str(nls, NFT_SET_ATTR_NAME));
 
 	set->keytype = keytype;
-	set->keylen        =
+	set->keylen  =
 		nft_set_attr_get_u32(nls, NFT_SET_ATTR_KEY_LEN) * BITS_PER_BYTE;
 
-	set->flags         = nft_set_attr_get_u32(nls, NFT_SET_ATTR_FLAGS);
+	set->flags   = nft_set_attr_get_u32(nls, NFT_SET_ATTR_FLAGS);
 
 	set->datatype = datatype;
 	if (nft_set_attr_is_set(nls, NFT_SET_ATTR_DATA_LEN)) {
@@ -1434,13 +1442,11 @@ int netlink_batch_send(struct list_head *err_list)
 int netlink_flush_ruleset(struct netlink_ctx *ctx, const struct handle *h,
 			  const struct location *loc)
 {
-	int err;
 	struct nft_table *nlt;
+	int err;
 
-	if (!ctx->batch_supported) {
-		netlink_io_error(ctx, loc, "Operation not supported.");
-		return -1;
-	}
+	if (!ctx->batch_supported)
+		return netlink_io_error(ctx, loc, "Operation not supported");
 
 	nlt = alloc_nft_table(h);
 	err = mnl_nft_table_batch_del(nlt, 0, ctx->seqnum);
@@ -1559,12 +1565,12 @@ static void nlr_for_each_set(struct nft_rule *nlr,
 			     void (*cb)(struct set *s, void *data),
 			     void *data)
 {
-	struct set *s;
-	uint32_t family;
-	const char *set_name, *table;
-	struct nft_rule_expr *nlre;
 	struct nft_rule_expr_iter *nlrei;
+	struct nft_rule_expr *nlre;
+	const char *set_name, *table;
 	const char *name;
+	struct set *s;
+	uint32_t family;
 
 	nlrei = nft_rule_expr_iter_create(nlr);
 	if (nlrei == NULL)
@@ -1594,8 +1600,10 @@ next:
 static int netlink_events_table_cb(const struct nlmsghdr *nlh, int type,
 				   struct netlink_mon_handler *monh)
 {
+	struct nft_table *nlt;
 	uint32_t family;
-	struct nft_table *nlt = netlink_table_alloc(nlh);
+
+	nlt = netlink_table_alloc(nlh);
 
 	switch (monh->format) {
 	case NFT_OUTPUT_DEFAULT:
@@ -1628,9 +1636,11 @@ static int netlink_events_table_cb(const struct nlmsghdr *nlh, int type,
 static int netlink_events_chain_cb(const struct nlmsghdr *nlh, int type,
 				   struct netlink_mon_handler *monh)
 {
+	struct nft_chain *nlc;
 	struct chain *c;
 	uint32_t family;
-	struct nft_chain *nlc = netlink_chain_alloc(nlh);
+
+	nlc = netlink_chain_alloc(nlh);
 
 	switch (monh->format) {
 	case NFT_OUTPUT_DEFAULT:
@@ -1671,10 +1681,11 @@ static int netlink_events_chain_cb(const struct nlmsghdr *nlh, int type,
 static int netlink_events_set_cb(const struct nlmsghdr *nlh, int type,
 				 struct netlink_mon_handler *monh)
 {
+	struct nft_set *nls;
 	struct set *set;
 	uint32_t family, flags;
-	struct nft_set *nls = netlink_set_alloc(nlh);
 
+	nls = netlink_set_alloc(nlh);
 	flags = nft_set_attr_get_u32(nls, NFT_SET_ATTR_FLAGS);
 	if (flags & SET_F_ANONYMOUS)
 		goto out;
@@ -1718,14 +1729,15 @@ out:
 static int netlink_events_setelem_cb(const struct nlmsghdr *nlh, int type,
 				     struct netlink_mon_handler *monh)
 {
-	struct nft_set_elem *nlse;
 	struct nft_set_elems_iter *nlsei;
+	struct nft_set_elem *nlse;
+	struct nft_set *nls;
 	struct set *dummyset;
 	struct set *set;
 	const char *setname, *table;
 	uint32_t family;
-	struct nft_set *nls = netlink_setelem_alloc(nlh);
 
+	nls = netlink_setelem_alloc(nlh);
 	table = nft_set_attr_get_str(nls, NFT_SET_ATTR_TABLE);
 	setname = nft_set_attr_get_str(nls, NFT_SET_ATTR_NAME);
 	family = nft_set_attr_get_u32(nls, NFT_SET_ATTR_FAMILY);
@@ -1803,14 +1815,15 @@ static void rule_map_decompose_cb(struct set *s, void *data)
 static int netlink_events_rule_cb(const struct nlmsghdr *nlh, int type,
 				  struct netlink_mon_handler *monh)
 {
-	struct rule *r;
-	uint32_t fam;
+	struct nft_rule *nlr;
 	const char *family;
 	const char *table;
 	const char *chain;
+	struct rule *r;
 	uint64_t handle;
-	struct nft_rule *nlr = netlink_rule_alloc(nlh);
+	uint32_t fam;
 
+	nlr = netlink_rule_alloc(nlh);
 	switch (monh->format) {
 	case NFT_OUTPUT_DEFAULT:
 		fam = nft_rule_attr_get_u32(nlr, NFT_RULE_ATTR_FAMILY);
@@ -1851,53 +1864,54 @@ static int netlink_events_rule_cb(const struct nlmsghdr *nlh, int type,
 static void netlink_events_cache_addtable(struct netlink_mon_handler *monh,
 					  const struct nlmsghdr *nlh)
 {
+	struct nft_table *nlt;
 	struct table *t;
-	struct nft_table *nlt = netlink_table_alloc(nlh);
 
+	nlt = netlink_table_alloc(nlh);
 	t = netlink_delinearize_table(monh->ctx, nlt);
-	table_add_hash(t);
-
 	nft_table_free(nlt);
+
+	table_add_hash(t);
 }
 
 static void netlink_events_cache_deltable(struct netlink_mon_handler *monh,
 					  const struct nlmsghdr *nlh)
 {
+	struct nft_table *nlt;
 	struct table *t;
 	struct handle h;
-	struct nft_table *nlt = netlink_table_alloc(nlh);
 
+	nlt      = netlink_table_alloc(nlh);
 	h.family = nft_table_attr_get_u32(nlt, NFT_TABLE_ATTR_FAMILY);
-	h.table = nft_table_attr_get_str(nlt, NFT_TABLE_ATTR_NAME);
+	h.table  = nft_table_attr_get_str(nlt, NFT_TABLE_ATTR_NAME);
+	nft_table_free(nlt);
 
 	t = table_lookup(&h);
 	if (t == NULL)
-		goto out;
+		return;
 
 	list_del(&t->list);
 	table_free(t);
-
-out:
-	nft_table_free(nlt);
 }
 
 static void netlink_events_cache_addset(struct netlink_mon_handler *monh,
 					const struct nlmsghdr *nlh)
 {
+	struct netlink_ctx set_tmpctx;
+	struct nft_set *nls;
+	struct table *t;
 	struct set *s;
 	LIST_HEAD(msgs);
-	struct table *t;
-	struct netlink_ctx set_tmpctx;
-	struct nft_set *nls = netlink_set_alloc(nlh);
 
 	memset(&set_tmpctx, 0, sizeof(set_tmpctx));
 	init_list_head(&set_tmpctx.list);
 	init_list_head(&msgs);
 	set_tmpctx.msgs = &msgs;
 
+	nls = netlink_set_alloc(nlh);
 	s = netlink_delinearize_set(&set_tmpctx, nls);
 	if (s == NULL)
-		return;
+		goto out;
 	s->init = set_expr_alloc(monh->loc);
 
 	t = table_lookup(&s->handle);
@@ -1915,17 +1929,19 @@ out:
 static void netlink_events_cache_addsetelem(struct netlink_mon_handler *monh,
 					    const struct nlmsghdr *nlh)
 {
-	struct set *set;
-	struct nft_set_elem *nlse;
 	struct nft_set_elems_iter *nlsei;
+	struct nft_set_elem *nlse;
+	struct nft_set *nls;
+	struct set *set;
 	const char *table, *setname;
-	struct nft_set *nls = netlink_setelem_alloc(nlh);
+	uint32_t family;
 
-	table = nft_set_attr_get_str(nls, NFT_SET_ATTR_TABLE);
+	nls     = netlink_setelem_alloc(nlh);
+	family  = nft_set_attr_get_u32(nls, NFT_SET_ATTR_FAMILY);
+	table   = nft_set_attr_get_str(nls, NFT_SET_ATTR_TABLE);
 	setname = nft_set_attr_get_str(nls, NFT_SET_ATTR_NAME);
 
-	set = set_lookup_global(nft_set_attr_get_u32(nls, NFT_SET_ATTR_FAMILY),
-				table, setname);
+	set = set_lookup_global(family, table, setname);
 	if (set == NULL) {
 		fprintf(stderr,
 			"W: Unable to cache set_elem. Set not found.\n");
@@ -1948,7 +1964,6 @@ static void netlink_events_cache_addsetelem(struct netlink_mon_handler *monh,
 		nlse = nft_set_elems_iter_next(nlsei);
 	}
 	nft_set_elems_iter_destroy(nlsei);
-
 out:
 	nft_set_free(nls);
 }
-- 
2.1.0

[PATCH 5/6] netlink_delinearize: rename netlink_parse_*_sreg/dreg functions

[Patrick McHardy]

These are really badly chosen names, use parse_expr and parse_stmt instead.

Signed-off-by: Patrick McHardy <kaber@trash.net>
---
 src/netlink_delinearize.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c
index 79d5af6..5a89fce 100644
--- a/src/netlink_delinearize.c
+++ b/src/netlink_delinearize.c
@@ -342,7 +342,7 @@ static void netlink_parse_exthdr(struct netlink_parse_ctx *ctx,
 			     expr);
 }
 
-static void netlink_parse_meta_dreg(struct netlink_parse_ctx *ctx,
+static void netlink_parse_meta_expr(struct netlink_parse_ctx *ctx,
 				    const struct location *loc,
 				    const struct nft_rule_expr *nle)
 {
@@ -355,7 +355,7 @@ static void netlink_parse_meta_dreg(struct netlink_parse_ctx *ctx,
 			     expr);
 }
 
-static void netlink_parse_meta_sreg(struct netlink_parse_ctx *ctx,
+static void netlink_parse_meta_stmt(struct netlink_parse_ctx *ctx,
 				    const struct location *loc,
 				    const struct nft_rule_expr *nle)
 {
@@ -377,12 +377,12 @@ static void netlink_parse_meta(struct netlink_parse_ctx *ctx,
 			       const struct nft_rule_expr *nle)
 {
 	if (nft_rule_expr_is_set(nle, NFT_EXPR_META_DREG))
-		netlink_parse_meta_dreg(ctx, loc, nle);
+		netlink_parse_meta_expr(ctx, loc, nle);
 	else
-		netlink_parse_meta_sreg(ctx, loc, nle);
+		netlink_parse_meta_stmt(ctx, loc, nle);
 }
 
-static void netlink_parse_ct_sreg(struct netlink_parse_ctx *ctx,
+static void netlink_parse_ct_stmt(struct netlink_parse_ctx *ctx,
 				  const struct location *loc,
 				  const struct nft_rule_expr *nle)
 {
@@ -400,7 +400,7 @@ static void netlink_parse_ct_sreg(struct netlink_parse_ctx *ctx,
 	list_add_tail(&stmt->list, &ctx->rule->stmts);
 }
 
-static void netlink_parse_ct_dreg(struct netlink_parse_ctx *ctx,
+static void netlink_parse_ct_expr(struct netlink_parse_ctx *ctx,
 				  const struct location *loc,
 				  const struct nft_rule_expr *nle)
 {
@@ -417,9 +417,9 @@ static void netlink_parse_ct(struct netlink_parse_ctx *ctx,
 			     const struct nft_rule_expr *nle)
 {
 	if (nft_rule_expr_is_set(nle, NFT_EXPR_CT_DREG))
-		netlink_parse_ct_dreg(ctx, loc, nle);
+		netlink_parse_ct_expr(ctx, loc, nle);
 	else
-		netlink_parse_ct_sreg(ctx, loc, nle);
+		netlink_parse_ct_stmt(ctx, loc, nle);
 }
 
 static void netlink_parse_counter(struct netlink_parse_ctx *ctx,
-- 
2.1.0

[PATCH 6/6] netlink_delinearize: cleanup hard to read code

[Patrick McHardy]

The netlink parsing code is full of long function calls spawning multiple
lines and in some cases parses netlink attributes multiple times.

Use local variables for the registers and other data required to
reconstruct the expressions and statements and reorder the code in
some cases to move related processing next to each other.

Signed-off-by: Patrick McHardy <kaber@trash.net>
---
 src/netlink_delinearize.c | 139 ++++++++++++++++++++++++++--------------------
 1 file changed, 79 insertions(+), 60 deletions(-)

diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c
index 5a89fce..27801be 100644
--- a/src/netlink_delinearize.c
+++ b/src/netlink_delinearize.c
@@ -143,19 +143,21 @@ static void netlink_parse_cmp(struct netlink_parse_ctx *ctx,
 			      const struct nft_rule_expr *nle)
 {
 	struct nft_data_delinearize nld;
+	enum nft_registers sreg;
 	struct expr *expr, *left, *right;
 	struct stmt *stmt;
 	enum ops op;
 
-	nld.value = nft_rule_expr_get(nle, NFT_EXPR_CMP_DATA, &nld.len);
-	left = netlink_get_register(ctx, loc,
-			nft_rule_expr_get_u32(nle, NFT_EXPR_CMP_SREG));
+	sreg = nft_rule_expr_get_u32(nle, NFT_EXPR_CMP_SREG);
+	left = netlink_get_register(ctx, loc, sreg);
 	if (left == NULL)
 		return netlink_error(ctx, loc,
 				     "Relational expression has no left "
 				     "hand side");
 
 	op = netlink_parse_cmp_op(nle);
+
+	nld.value = nft_rule_expr_get(nle, NFT_EXPR_CMP_DATA, &nld.len);
 	right = netlink_alloc_value(loc, &nld);
 
 	// FIXME
@@ -173,23 +175,24 @@ static void netlink_parse_lookup(struct netlink_parse_ctx *ctx,
 				 const struct location *loc,
 				 const struct nft_rule_expr *nle)
 {
+	enum nft_registers sreg, dreg;
+	const char *name;
 	struct stmt *stmt;
 	struct expr *expr, *left, *right;
 	struct set *set;
-	enum nft_registers dreg;
 
-	left = netlink_get_register(ctx, loc,
-			nft_rule_expr_get_u32(nle, NFT_EXPR_LOOKUP_SREG));
+	sreg = nft_rule_expr_get_u32(nle, NFT_EXPR_LOOKUP_SREG);
+	left = netlink_get_register(ctx, loc, sreg);
 	if (left == NULL)
 		return netlink_error(ctx, loc,
 				     "Lookup expression has no left hand side");
 
-	set = set_lookup(ctx->table,
-			 nft_rule_expr_get_str(nle, NFT_EXPR_LOOKUP_SET));
+	name = nft_rule_expr_get_str(nle, NFT_EXPR_LOOKUP_SET);
+	set  = set_lookup(ctx->table, name);
 	if (set == NULL)
 		return netlink_error(ctx, loc,
 				     "Unknown set '%s' in lookup expression",
-				     nft_rule_expr_get_str(nle, NFT_EXPR_LOOKUP_SET));
+				     name);
 
 	right = set_ref_expr_alloc(loc, set);
 
@@ -210,12 +213,13 @@ static void netlink_parse_bitwise(struct netlink_parse_ctx *ctx,
 				  const struct location *loc,
 				  const struct nft_rule_expr *nle)
 {
+	struct nft_data_delinearize nld;
+	enum nft_registers sreg, dreg;
 	struct expr *expr, *left, *mask, *xor, *or;
 	mpz_t m, x, o;
-	struct nft_data_delinearize nld;
 
-	left = netlink_get_register(ctx, loc,
-			nft_rule_expr_get_u32(nle, NFT_EXPR_BITWISE_SREG));
+	sreg = nft_rule_expr_get_u32(nle, NFT_EXPR_BITWISE_SREG);
+	left = netlink_get_register(ctx, loc, sreg);
 	if (left == NULL)
 		return netlink_error(ctx, loc,
 				     "Bitwise expression has no left "
@@ -224,12 +228,10 @@ static void netlink_parse_bitwise(struct netlink_parse_ctx *ctx,
 	expr = left;
 
 	nld.value = nft_rule_expr_get(nle, NFT_EXPR_BITWISE_MASK, &nld.len);
-
 	mask = netlink_alloc_value(loc, &nld);
 	mpz_init_set(m, mask->value);
 
 	nld.value = nft_rule_expr_get(nle, NFT_EXPR_BITWISE_XOR, &nld.len);
-
 	xor  = netlink_alloc_value(loc, &nld);
 	mpz_init_set(x, xor->value);
 
@@ -272,20 +274,20 @@ static void netlink_parse_bitwise(struct netlink_parse_ctx *ctx,
 	mpz_clear(x);
 	mpz_clear(o);
 
-	netlink_set_register(ctx,
-			     nft_rule_expr_get_u32(nle, NFT_EXPR_BITWISE_DREG),
-			     expr);
+	dreg = nft_rule_expr_get_u32(nle, NFT_EXPR_BITWISE_DREG);
+	netlink_set_register(ctx, dreg, expr);
 }
 
 static void netlink_parse_byteorder(struct netlink_parse_ctx *ctx,
 				    const struct location *loc,
 				    const struct nft_rule_expr *nle)
 {
+	enum nft_registers sreg, dreg;
 	struct expr *expr, *arg;
 	enum ops op;
 
-	arg = netlink_get_register(ctx, loc,
-		nft_rule_expr_get_u32(nle, NFT_EXPR_BYTEORDER_SREG));
+	sreg = nft_rule_expr_get_u32(nle, NFT_EXPR_BYTEORDER_SREG);
+	arg = netlink_get_register(ctx, loc, sreg);
 	if (arg == NULL)
 		return netlink_error(ctx, loc,
 				     "Byteorder expression has no left "
@@ -305,68 +307,79 @@ static void netlink_parse_byteorder(struct netlink_parse_ctx *ctx,
 
 	expr = unary_expr_alloc(loc, op, arg);
 	expr->len = arg->len;
-	netlink_set_register(ctx,
-			     nft_rule_expr_get_u32(nle, NFT_EXPR_BYTEORDER_DREG),
-			     expr);
+
+	dreg = nft_rule_expr_get_u32(nle, NFT_EXPR_BYTEORDER_DREG);
+	netlink_set_register(ctx, dreg, expr);
 }
 
 static void netlink_parse_payload(struct netlink_parse_ctx *ctx,
 				  const struct location *loc,
 				  const struct nft_rule_expr *nle)
 {
+	enum nft_registers dreg;
+	uint32_t base, offset, len;
 	struct expr *expr;
 
+	base   = nft_rule_expr_get_u32(nle, NFT_EXPR_PAYLOAD_BASE) + 1;
+	offset = nft_rule_expr_get_u32(nle, NFT_EXPR_PAYLOAD_OFFSET) * BITS_PER_BYTE;
+	len    = nft_rule_expr_get_u32(nle, NFT_EXPR_PAYLOAD_LEN) * BITS_PER_BYTE;
+
 	expr = payload_expr_alloc(loc, NULL, 0);
-	payload_init_raw(expr, nft_rule_expr_get_u32(nle, NFT_EXPR_PAYLOAD_BASE) + 1,
-			 nft_rule_expr_get_u32(nle, NFT_EXPR_PAYLOAD_OFFSET) * BITS_PER_BYTE,
-			 nft_rule_expr_get_u32(nle, NFT_EXPR_PAYLOAD_LEN) * BITS_PER_BYTE);
+	payload_init_raw(expr, base, offset, len);
 
-	netlink_set_register(ctx,
-			     nft_rule_expr_get_u32(nle, NFT_EXPR_PAYLOAD_DREG),
-			     expr);
+	dreg = nft_rule_expr_get_u32(nle, NFT_EXPR_PAYLOAD_DREG);
+	netlink_set_register(ctx, dreg, expr);
 }
 
 static void netlink_parse_exthdr(struct netlink_parse_ctx *ctx,
 				 const struct location *loc,
 				 const struct nft_rule_expr *nle)
 {
+	enum nft_registers dreg;
+	uint32_t offset, len;
+	uint8_t type;
 	struct expr *expr;
 
+	type   = nft_rule_expr_get_u8(nle, NFT_EXPR_EXTHDR_TYPE);
+	offset = nft_rule_expr_get_u32(nle, NFT_EXPR_EXTHDR_OFFSET) * BITS_PER_BYTE;
+	len    = nft_rule_expr_get_u32(nle, NFT_EXPR_EXTHDR_LEN) * BITS_PER_BYTE;
+
 	expr = exthdr_expr_alloc(loc, NULL, 0);
-	exthdr_init_raw(expr, nft_rule_expr_get_u8(nle, NFT_EXPR_EXTHDR_TYPE),
-			nft_rule_expr_get_u32(nle, NFT_EXPR_EXTHDR_OFFSET) * BITS_PER_BYTE,
-			nft_rule_expr_get_u32(nle, NFT_EXPR_EXTHDR_LEN) * BITS_PER_BYTE);
+	exthdr_init_raw(expr, type, offset, len);
 
-	netlink_set_register(ctx,
-			     nft_rule_expr_get_u32(nle, NFT_EXPR_EXTHDR_DREG),
-			     expr);
+	dreg = nft_rule_expr_get_u32(nle, NFT_EXPR_EXTHDR_DREG);
+	netlink_set_register(ctx, dreg, expr);
 }
 
 static void netlink_parse_meta_expr(struct netlink_parse_ctx *ctx,
 				    const struct location *loc,
 				    const struct nft_rule_expr *nle)
 {
+	enum nft_registers dreg;
+	uint32_t key;
 	struct expr *expr;
 
-	expr = meta_expr_alloc(loc,
-			       nft_rule_expr_get_u32(nle, NFT_EXPR_META_KEY));
-	netlink_set_register(ctx,
-			     nft_rule_expr_get_u32(nle, NFT_EXPR_META_DREG),
-			     expr);
+	key  = nft_rule_expr_get_u32(nle, NFT_EXPR_META_KEY);
+	expr = meta_expr_alloc(loc, key);
+
+	dreg = nft_rule_expr_get_u32(nle, NFT_EXPR_META_DREG);
+	netlink_set_register(ctx, dreg, expr);
 }
 
 static void netlink_parse_meta_stmt(struct netlink_parse_ctx *ctx,
 				    const struct location *loc,
 				    const struct nft_rule_expr *nle)
 {
+	enum nft_registers sreg;
+	uint32_t key;
 	struct stmt *stmt;
 	struct expr *expr;
 
-	expr = netlink_get_register(ctx, loc,
-			nft_rule_expr_get_u32(nle, NFT_EXPR_META_SREG));
-	stmt = meta_stmt_alloc(loc,
-			       nft_rule_expr_get_u32(nle, NFT_EXPR_META_KEY),
-			       expr);
+	sreg = nft_rule_expr_get_u32(nle, NFT_EXPR_META_SREG);
+	expr = netlink_get_register(ctx, loc, sreg);
+
+	key  = nft_rule_expr_get_u32(nle, NFT_EXPR_META_KEY);
+	stmt = meta_stmt_alloc(loc, key, expr);
 	expr_set_type(expr, stmt->meta.tmpl->dtype, stmt->meta.tmpl->byteorder);
 
 	list_add_tail(&stmt->list, &ctx->rule->stmts);
@@ -386,15 +399,16 @@ static void netlink_parse_ct_stmt(struct netlink_parse_ctx *ctx,
 				  const struct location *loc,
 				  const struct nft_rule_expr *nle)
 {
+	enum nft_registers sreg;
+	uint32_t key;
 	struct stmt *stmt;
 	struct expr *expr;
 
-	expr = netlink_get_register(ctx, loc,
-				    nft_rule_expr_get_u32(nle,
-							  NFT_EXPR_CT_SREG));
-	stmt = ct_stmt_alloc(loc,
-			     nft_rule_expr_get_u32(nle, NFT_EXPR_CT_KEY),
-			     expr);
+	sreg = nft_rule_expr_get_u32(nle, NFT_EXPR_CT_SREG);
+	expr = netlink_get_register(ctx, loc, sreg);
+
+	key  = nft_rule_expr_get_u32(nle, NFT_EXPR_CT_KEY);
+	stmt = ct_stmt_alloc(loc, key, expr);
 	expr_set_type(expr, stmt->ct.tmpl->dtype, stmt->ct.tmpl->byteorder);
 
 	list_add_tail(&stmt->list, &ctx->rule->stmts);
@@ -404,12 +418,15 @@ static void netlink_parse_ct_expr(struct netlink_parse_ctx *ctx,
 				  const struct location *loc,
 				  const struct nft_rule_expr *nle)
 {
+	enum nft_registers dreg;
+	uint32_t key;
 	struct expr *expr;
 
-	expr = ct_expr_alloc(loc, nft_rule_expr_get_u32(nle, NFT_EXPR_CT_KEY));
-	netlink_set_register(ctx,
-			     nft_rule_expr_get_u32(nle, NFT_EXPR_CT_DREG),
-			     expr);
+	key  = nft_rule_expr_get_u32(nle, NFT_EXPR_CT_KEY);
+	expr = ct_expr_alloc(loc, key);
+
+	dreg = nft_rule_expr_get_u32(nle, NFT_EXPR_CT_DREG);
+	netlink_set_register(ctx, dreg, expr);
 }
 
 static void netlink_parse_ct(struct netlink_parse_ctx *ctx,
@@ -479,8 +496,8 @@ static void netlink_parse_limit(struct netlink_parse_ctx *ctx,
 	struct stmt *stmt;
 
 	stmt = limit_stmt_alloc(loc);
-	stmt->limit.rate  = nft_rule_expr_get_u64(nle, NFT_EXPR_LIMIT_RATE);
-	stmt->limit.unit  = nft_rule_expr_get_u64(nle, NFT_EXPR_LIMIT_UNIT);
+	stmt->limit.rate = nft_rule_expr_get_u64(nle, NFT_EXPR_LIMIT_RATE);
+	stmt->limit.unit = nft_rule_expr_get_u64(nle, NFT_EXPR_LIMIT_UNIT);
 	list_add_tail(&stmt->list, &ctx->rule->stmts);
 }
 
@@ -587,12 +604,14 @@ static void netlink_parse_masq(struct netlink_parse_ctx *ctx,
 			       const struct nft_rule_expr *nle)
 {
 	struct stmt *stmt;
+	uint32_t flags;
 
-	stmt = masq_stmt_alloc(loc);
-
+	flags = 0;
 	if (nft_rule_expr_is_set(nle, NFT_EXPR_MASQ_FLAGS))
-		stmt->masq.flags = nft_rule_expr_get_u32(nle,
-							 NFT_EXPR_MASQ_FLAGS);
+		flags = nft_rule_expr_get_u32(nle, NFT_EXPR_MASQ_FLAGS);
+
+	stmt = masq_stmt_alloc(loc);
+	stmt->masq.flags = flags;
 
 	list_add_tail(&stmt->list, &ctx->rule->stmts);
 }
-- 
2.1.0