| 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. | expand |
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
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,
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(-)