Hi Pablo,
On Thu, Dec 27, 2018 at 08:48:00PM +0100, Pablo Neira Ayuso wrote:
> On Thu, Dec 20, 2018 at 04:09:17PM +0100, Phil Sutter wrote:
> [...]
> > + if (chain) {
> > + c = nftnl_chain_list_lookup_byname(list, chain);
> > + if (!c) {
> > + errno = ENOENT;
> > + return 0;
> > + }
> > + d.builtin_err = -2;
> > + ret = __nft_chain_user_del(c, &d);
> > + if (ret == -2)
>
> We can probably do an upfront check for built-in chain to avoid this
> special code? __nft_chain_user_del() is only called from
> nft_chain_user_del().
Well, __nft_chain_user_del() is either called for a user-specified chain
or for all chains of the given table. While that builtin_err field could
be avoided by having the nft_chain_builtin() check in the first case
before dispatching to __nft_chain_user_del(), in the second case
__nft_chain_user_del() still has to do the check so it doesn't try to
delete builtin chains by accident. By using this conditional error code,
the nft_chain_builtin() check doesn't happen twice when deleting a
user-specified chain.
If you think it's not worth it, just let me know and I'll send a
follow-up which removes it (and introduces the unavoidable double
check).
Cheers, Phil
