Hi,
On Mon, Oct 12, 2020 at 02:01:18PM +0200, Pablo Neira Ayuso wrote:
> 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.
Hmm. There might be exceptions, but typically we should have a function
that takes an optional chain name and the body roughly looks like this:
| if (chain) {
| return do_something(nft_chain_find(..., chain));
| }
| return nft_chain_foreach(..., do_something);
[...]
> 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.
Which in particular are those?
The overall agenda is that looking up a chain won't be a trivial
nftnl_chain_list lookup anymore. And iterating over a table's chains
won't be a matter of iterating over a list, because I split base-chains
from user-defined ones:
* Base-chains sit in an array of size NF_INET_NUMHOOKS.
* User-defined chains sit in an (open-coded) list ordered by name.
So the old nft_chain_list_get() is not possible anymore, therefore I
replace everything by either nft_chain_find() or nft_chain_foreach().
Functions should not use both in the same code-path, so if you spot that
I should have a close look again.
Cheers, Phil
