Re: [iptables PATCH v2 02/10] nft: Implement nft_chain_foreach()

[Pablo Neira Ayuso <pablo@netfilter.org>]

On Wed, Sep 23, 2020 at 07:48:41PM +0200, Phil Sutter wrote:
> This is just a fancy wrapper around nftnl_chain_list_foreach() with the
> added benefit of detecting invalid table names or uninitialized chain
> lists. This in turn allows to drop the checks in flush_rule_cache() and
> ignore the return code of nft_chain_foreach() as it fails only if the
> dropped checks had failed, too.

At quick glance, this is reducing the LoC.

However, I'm not sure this is better, before this code:

1) You fetch the list
2) You use it from several spots in the function

with this patch you might look up for the chain list several times in
the same function.

+int nft_chain_foreach(struct nft_handle *h, const char *table,                
+                   int (*cb)(struct nftnl_chain *c, void *data),              
+                   void *data)                                                
+{                                                                             
+     const struct builtin_table *t;                                           
+                                                                              
+     t = nft_table_builtin_find(h, table);                                    
+     if (!t)                                                                  
+             return -1;                                                       
+                                                                              
+     if (!h->cache->table[t->type].chains)                                    
+             return -1;                                                       
+                                                                              
+     return nftnl_chain_list_foreach(h->cache->table[t->type].chains,         
+                                     cb, data);                               
+}

I can also see calls to:

nft_chain_find(h, table, chain);

and

nft_chain_foreach(...)

from the same function.

This patch also updates paths in very different ways, there is no
common idiom being replaced.