On 22.02.2012 11:54, Eric Dumazet wrote:
Le mercredi 22 février 2012 à 10:48 +0400, Tony Zelenoff a écrit :
* ret variable initialization removed as useless
* Similar code strings concatenated and functions code
flow became more plain
Signed-off-by: Tony Zelenoff<antonz@xxxxxxxxxxxxx>
---
net/netfilter/nf_conntrack_ecache.c | 26 ++++++++++----------------
1 files changed, 10 insertions(+), 16 deletions(-)
diff --git a/net/netfilter/nf_conntrack_ecache.c b/net/netfilter/nf_conntrack_ecache.c
index aa15977..9b8e986 100644
--- a/net/netfilter/nf_conntrack_ecache.c
+++ b/net/netfilter/nf_conntrack_ecache.c
@@ -81,21 +81,18 @@ EXPORT_SYMBOL_GPL(nf_ct_deliver_cached_events);
int nf_conntrack_register_notifier(struct net *net,
struct nf_ct_event_notifier *new)
{
- int ret = 0;
+ int ret;
struct nf_ct_event_notifier *notify;
mutex_lock(&nf_ct_ecache_mutex);
notify = rcu_dereference_protected(net->ct.nf_conntrack_event_cb,
lockdep_is_held(&nf_ct_ecache_mutex));
- if (notify != NULL) {
+ if (likely(!notify)) {
+ rcu_assign_pointer(net->ct.nf_conntrack_event_cb, new);
+ ret = 0;
+ } else
ret = -EBUSY;
- goto out_unlock;
- }
- rcu_assign_pointer(net->ct.nf_conntrack_event_cb, new);
- mutex_unlock(&nf_ct_ecache_mutex);
- return ret;
-out_unlock:
mutex_unlock(&nf_ct_ecache_mutex);
return ret;
}
@@ -118,21 +115,18 @@ EXPORT_SYMBOL_GPL(nf_conntrack_unregister_notifier);
int nf_ct_expect_register_notifier(struct net *net,
struct nf_exp_event_notifier *new)
{
- int ret = 0;
+ int ret;
struct nf_exp_event_notifier *notify;
mutex_lock(&nf_ct_ecache_mutex);
notify = rcu_dereference_protected(net->ct.nf_expect_event_cb,
lockdep_is_held(&nf_ct_ecache_mutex));
- if (notify != NULL) {
+ if (likely(!notify)) {
+ rcu_assign_pointer(net->ct.nf_expect_event_cb, new);
+ ret = 0;
+ } else
ret = -EBUSY;
- goto out_unlock;
- }
- rcu_assign_pointer(net->ct.nf_expect_event_cb, new);
- mutex_unlock(&nf_ct_ecache_mutex);
- return ret;
-out_unlock:
mutex_unlock(&nf_ct_ecache_mutex);
return ret;
}
Please leave the code as is, I find it more readable.
It is standard coding practice, and permits stacking of new init code,
with proper error path.
Do not agree a bit. Of course, the code stacking and so on is good, but
there is no reason to write:
rcu_assign_pointer(net->ct.nf_expect_event_cb, new);
mutex_unlock(&nf_ct_ecache_mutex);
return ret;
out_unlock:
mutex_unlock(&nf_ct_ecache_mutex);
return ret;
if you can do it (without breaking logic and stacking ability and as it
done everywhere) this way:
rcu_assign_pointer(net->ct.nf_expect_event_cb, new);
out_unlock:
mutex_unlock(&nf_ct_ecache_mutex);
return ret;
with only one exit with proper locks freeing or deinitialization.
Ok, after that i've remove ret initialization at start and without goto
this label became unused and compiler warn about it. Thus it was removed.
Dont add likely()/unlikely() clauses in slow path, this obfuscate code
for litle gain.
ok
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
