On Thu, Jul 18, 2019 at 01:37:04PM +0100, Jeremy Sowden wrote:
> On 2019-07-16, at 21:39:03 +0200, Pablo Neira Ayuso wrote:
> > BTW, not directly related to this, but isn't this strange?
> >
> > 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);
> > ...
> >
> > ctx is reset over and over again. Then, recycled here:
> >
> > ret = mnl_batch_talk(&ctx, &err_list, num_cmds);
> >
> > I wonder if we can get this better.
>
> Something like this?
Yes, something like that would get things in better shape I think,
more comments below.
> struct netlink_ctx ctx = { .msgs = msgs, .nft = nft };
> ...
>
> ctx.batch = batch = mnl_batch_init();
> batch_seqnum = mnl_batch_begin(batch, mnl_seqnum_alloc(&seqnum));
> list_for_each_entry(cmd, cmds, list) {
> ctx.seqnum = cmd->seqnum = mnl_seqnum_alloc(&seqnum);
> init_list_head(&ctx.list);
I think we don't need to re-initialize this list over and over again
(from what I see when doing: git grep "ctx->list").
This always does list_splice_tail_init() to attach the object list
where they belong.
You can probably add something like:
if (!list_empty(&ctx->list))
BUG("command list is not empty\n");
I would make a patch and run tests/shell and tests/py to check if what
I'm suggesting this fine :-)
> ret = do_command(&ctx, cmd);
> ...
Thanks!
