Re: [PATCH 9/9] netfilter: conntrack: replace notify chain by function pointer

[Patrick McHardy <kaber@trash.net>]

Pablo Neira Ayuso wrote:
> Patrick McHardy wrote:
>> Pablo Neira Ayuso wrote:
>>> -int nf_ct_expect_unregister_notifier(struct notifier_block *nb)
>>> +int nf_ct_expect_unregister_notifier(struct nf_exp_event_notifier *new)
>>>  {
>>> -    return atomic_notifier_chain_unregister(&nf_ct_expect_chain, nb);
>>> +    int ret = 0;
>>> +    struct nf_exp_event_notifier *notify;
>>> +
>>> +    mutex_lock(&nf_ct_ecache_mutex);
>>> +    notify = rcu_dereference(nf_expect_event_cb);
>>> +    if (notify != new) {
>>> +        ret = -EINVAL;
>>> +        goto out_unlock;
>>> +    }
>> I think these unregistration functions should return void. The only
>> reason why they don't currently is because the notifier_chain_unregister
>> function for some unknown reasons don't return void, but there's
>> a) nothing the caller could possibly do to handle this and b) a bug
>> anyways. So I'd suggest to just unconditionally assign NULL.
> 
> Would you be OK with something like:
> 
> BUG_ON(notify != new);
> 
> So we can catch this very unlikely bug, if so.

Sure. We don't do this is 99% of the other unregistration functions
however, so I don't think its particulary useful. It only affects
out of tree code anyways, unless we've done something really stupid,
like remove error checking in the initialization function :)