2025-03-03, 15:45:23 +0100, Antonio Quartulli wrote:
On 03/03/2025 14:08, Sabrina Dubroca wrote:
if (ovpn_sock && ovpn_sock->sock->sk == sk)skip = false;rcu_read_unlock();if (skip)continue;The skip/continue logic looks a tiny bit strange to me, maybe this:
Hehe, it's like a double negation. I agree it can be improved.
hash_for_each_safe(ovpn->peers->by_id, bkt, tmp, peer, hash_entry_id) { bool remove = true;
does the netdev coding style allow to use locally scoped variables? Or should I declare everything at the beginning of the function?
I had this rule in mind, but it may have been eliminated by now.
Based on a few samples from net/core/dev.c, I'd say it's allowed: https://web.git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/tree... https://web.git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/tree... https://web.git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/tree... https://web.git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/tree...
/* if a socket was passed as argument, skip all peers except * those using it */ if (sk) { rcu_read_lock(); ovpn_sock = rcu_dereference(peer->sock); remove = ovpn_sock && ovpn_sock->sock->sk == sk; rcu_read_unlock(); } if (remove) ovpn_peer_remove(peer, reason, &release_list);}
(only if you agree it looks better - if it's my opinion against yours, ignore me since it's really just coding style/taste)
Yours look simpler/cleaner. I'll go with it.
ok :)