[nft] libnftables: got rid of repeated initialization of netlink_ctx variable in loop.
diff mbox series

Message ID 20190718210552.16890-1-jeremy@azazel.net
State Changes Requested
Delegated to: Pablo Neira
Headers show
Series
  • [nft] libnftables: got rid of repeated initialization of netlink_ctx variable in loop.
Related show

Commit Message

Jeremy Sowden July 18, 2019, 9:05 p.m. UTC
Most members in the context doesn't change, so there is no need to
memset it and reassign most of its members on every iteration.  Moved
that code out of the loop.

Fixes: 49900d448ac9 ("libnftables: Move library stuff out of main.c")
Reported-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 src/libnftables.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

Comments

Phil Sutter July 19, 2019, 10:32 a.m. UTC | #1
Hi,

On Thu, Jul 18, 2019 at 10:05:52PM +0100, Jeremy Sowden wrote:
> Most members in the context doesn't change, so there is no need to
> memset it and reassign most of its members on every iteration.  Moved
> that code out of the loop.
> 
> Fixes: 49900d448ac9 ("libnftables: Move library stuff out of main.c")

This commit merely moved the code from src/main.c into the current
location. I would rather point at a72315d2bad47 ("src: add rule batching
support"), which introduced the loop (and already contains some of the
pointless assignments). Note that commit is from 2013. :)

> Reported-by: Pablo Neira Ayuso <pablo@netfilter.org>
> Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
> ---
>  src/libnftables.c | 11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/src/libnftables.c b/src/libnftables.c
> index 2f77a7709e2c..834ea661a146 100644
> --- a/src/libnftables.c
> +++ b/src/libnftables.c
> @@ -23,7 +23,7 @@ static int nft_netlink(struct nft_ctx *nft,
>  {
>  	uint32_t batch_seqnum, seqnum = 0, num_cmds = 0;
>  	struct nftnl_batch *batch;
> -	struct netlink_ctx ctx;
> +	struct netlink_ctx ctx = { .msgs = msgs, .nft = nft };

Use '.list = LIST_HEAD_INIT(ctx.list),' here. Refer to cache_update() in
src/rule.c for inspiration. :)

>  	struct cmd *cmd;
>  	struct mnl_err *err, *tmp;
>  	LIST_HEAD(err_list);
> @@ -32,16 +32,13 @@ static int nft_netlink(struct nft_ctx *nft,
>  	if (list_empty(cmds))
>  		return 0;
>  
> -	batch = mnl_batch_init();
> +	init_list_head(&ctx.list);

Drop this.

> +
> +	ctx.batch = batch = mnl_batch_init();

Move ctx.batch init into declaration as well. Drop dedicated 'batch'
variable and replace by 'ctx.batch'.

Thanks, Phil

Patch
diff mbox series

diff --git a/src/libnftables.c b/src/libnftables.c
index 2f77a7709e2c..834ea661a146 100644
--- a/src/libnftables.c
+++ b/src/libnftables.c
@@ -23,7 +23,7 @@  static int nft_netlink(struct nft_ctx *nft,
 {
 	uint32_t batch_seqnum, seqnum = 0, num_cmds = 0;
 	struct nftnl_batch *batch;
-	struct netlink_ctx ctx;
+	struct netlink_ctx ctx = { .msgs = msgs, .nft = nft };
 	struct cmd *cmd;
 	struct mnl_err *err, *tmp;
 	LIST_HEAD(err_list);
@@ -32,16 +32,13 @@  static int nft_netlink(struct nft_ctx *nft,
 	if (list_empty(cmds))
 		return 0;
 
-	batch = mnl_batch_init();
+	init_list_head(&ctx.list);
+
+	ctx.batch = batch = mnl_batch_init();
 
 	batch_seqnum = mnl_batch_begin(batch, mnl_seqnum_alloc(&seqnum));
 	list_for_each_entry(cmd, cmds, list) {
-		memset(&ctx, 0, sizeof(ctx));
-		ctx.msgs = msgs;
 		ctx.seqnum = cmd->seqnum = mnl_seqnum_alloc(&seqnum);
-		ctx.batch = batch;
-		ctx.nft = nft;
-		init_list_head(&ctx.list);
 		ret = do_command(&ctx, cmd);
 		if (ret < 0) {
 			netlink_io_error(&ctx, &cmd->location,