There is no need to use a label and a goto for code that is used once only. Moreover having a goto for a single return statement should always be avoided.
Introduced by e368857f66620b8483166e8e6556d9c87f9b3e71 ("batman-adv: Multicast Listener Announcements via Translation Table")
Cc: Linus Lüssing linus.luessing@web.de Signed-off-by: Antonio Quartulli antonio@meshcoding.com ---
Changes from v2: - added compat code change to 2/11 to accommodate new use of netdev_master_upper_dev_get_rcu()
Changes from v1: - change "we are out of memory" to "in case of memory allocation failure" in 3/11 - fix commit message in 10/11
multicast.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/multicast.c b/multicast.c index 998b429..d92de1e 100644 --- a/multicast.c +++ b/multicast.c @@ -194,12 +194,10 @@ static bool batadv_mcast_has_bridge(struct batadv_priv *bat_priv)
bridge = batadv_mcast_get_bridge(bat_priv); if (!bridge) - goto out; + return false;
dev_put(bridge); return true; -out: - return false; }
/**
Looking for an interface and increasing its refcounter only to check if it really exists is not really useful and makes the code look more complex without a real gain.
Change batadv_mcast_has_bridge() accordingly and avoid to use the pointer to the interface outside of the rcu_read_lock/unlock context.
Introduced by e368857f66620b8483166e8e6556d9c87f9b3e71 ("batman-adv: Multicast Listener Announcements via Translation Table")
Cc: Linus Lüssing linus.luessing@web.de Signed-off-by: Antonio Quartulli antonio@meshcoding.com --- compat.h | 19 ++++--------------- multicast.c | 33 +++++++-------------------------- 2 files changed, 11 insertions(+), 41 deletions(-)
diff --git a/compat.h b/compat.h index e14c6bb..5eb5fe6 100644 --- a/compat.h +++ b/compat.h @@ -164,14 +164,8 @@ static inline int batadv_param_set_copystring(const char *val, #define NET_ADDR_RANDOM 0
#define netdev_master_upper_dev_get_rcu(dev) \ - upper; \ - if (dev->br_port ? 1 : 0) { \ - rcu_read_unlock(); \ - dev_hold(dev); \ - return dev; \ - } else {\ - dev = NULL; \ - } + (dev->br_port ? dev : NULL); \ + break;
#endif /* < KERNEL_VERSION(2, 6, 36) */
@@ -375,13 +369,8 @@ static int __batadv_interface_tx(struct sk_buff *skb, \
#ifndef netdev_master_upper_dev_get_rcu #define netdev_master_upper_dev_get_rcu(dev) \ - upper; \ - if (dev->priv_flags & IFF_BRIDGE_PORT) { \ - rcu_read_unlock(); \ - dev_hold(dev); \ - return dev; \ - } else \ - dev = NULL; + (dev->priv_flags & IFF_BRIDGE_PORT ? dev : NULL); \ + break;
#endif /* netdev_master_upper_dev_get_rcu */
diff --git a/multicast.c b/multicast.c index d92de1e..af3fe49 100644 --- a/multicast.c +++ b/multicast.c @@ -156,16 +156,16 @@ static void batadv_mcast_mla_tt_add(struct batadv_priv *bat_priv, }
/** - * batadv_mcast_get_bridge - get the bridge interface on our soft interface + * batadv_mcast_has_bridge - check whether the soft-iface is bridged * @bat_priv: the bat priv with all the soft interface information * - * Return the next bridge interface on top of our soft interface and increase - * its refcount. If no such bridge interface exists, then return NULL. + * Check whether there is a bridge on top of our soft interface. Return + * true if so, false otherwise. */ -static struct net_device * -batadv_mcast_get_bridge(struct batadv_priv *bat_priv) +static bool batadv_mcast_has_bridge(struct batadv_priv *bat_priv) { struct net_device *upper = bat_priv->soft_iface; + bool has_bridge = false;
rcu_read_lock();
@@ -174,30 +174,11 @@ batadv_mcast_get_bridge(struct batadv_priv *bat_priv) } while (upper && !(upper->priv_flags & IFF_EBRIDGE));
if (upper) - dev_hold(upper); + has_bridge = true;
rcu_read_unlock();
- return upper; -} - -/** - * batadv_mcast_has_bridge - check whether the soft-iface is bridged - * @bat_priv: the bat priv with all the soft interface information - * - * Check whether there is a bridge on top of our soft interface. Return - * true if so, false otherwise. - */ -static bool batadv_mcast_has_bridge(struct batadv_priv *bat_priv) -{ - struct net_device *bridge; - - bridge = batadv_mcast_get_bridge(bat_priv); - if (!bridge) - return false; - - dev_put(bridge); - return true; + return has_bridge; }
/**
The kerneldoc should always use the third person singular in the long function description. Moreover it should always try use up to 80 chars per line.
Introduced by 86cb16e5ec1e2d75821006e8f4abbec66fb741ac ("batman-adv: Add IPv4 link-local/IPv6-ll-all-nodes multicast support")
Cc: Linus Lüssing linus.luessing@web.de Signed-off-by: Antonio Quartulli antonio@meshcoding.com --- multicast.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/multicast.c b/multicast.c index af3fe49..2d39938 100644 --- a/multicast.c +++ b/multicast.c @@ -256,11 +256,11 @@ out: * @skb: the IPv4 packet to check * @is_unsnoopable: stores whether the destination is snoopable * - * Check whether the given IPv4 packet has the potential to - * be forwarded with a mode more optimal than classic flooding. + * Checks whether the given IPv4 packet has the potential to be forwarded with a + * mode more optimal than classic flooding. * - * If so then return 0. Otherwise -EINVAL is returned or -ENOMEM if we are - * out of memory. + * If so then returns 0. Otherwise -EINVAL is returned or -ENOMEM in case of + * memory allocation failure. */ static int batadv_mcast_forw_mode_check_ipv4(struct batadv_priv *bat_priv, struct sk_buff *skb, @@ -485,8 +485,8 @@ batadv_mcast_forw_ip_node_get(struct batadv_priv *bat_priv, * batadv_mcast_want_forw_unsnoop_node_get - get a node with an unsnoopable flag * @bat_priv: the bat priv with all the soft interface information * - * Return an orig_node which has the BATADV_MCAST_WANT_ALL_UNSNOOPABLES flag set - * and increase its refcount. + * Returns an orig_node which has the BATADV_MCAST_WANT_ALL_UNSNOOPABLES flag + * set and increases its refcount. */ static struct batadv_orig_node * batadv_mcast_forw_unsnoop_node_get(struct batadv_priv *bat_priv)
The kerneldoc should always use the third person singular in the long function description. Moreover it should always try use up to 80 chars per line.
Introduced by commit 01d665e054254618353ec8d9ea43d1a138de6e6d ("batman-adv: Multicast Listener Announcements via Translation Table")
Cc: Linus Lüssing linus.luessing@web.de Signed-off-by: Antonio Quartulli antonio@meshcoding.com --- multicast.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/multicast.c b/multicast.c index 2d39938..afacf82 100644 --- a/multicast.c +++ b/multicast.c @@ -27,7 +27,7 @@ * Collect multicast addresses of the local multicast listeners * on the given soft interface, dev, in the given mcast_list. * - * Return -ENOMEM on memory allocation error or the number of + * Returns -ENOMEM on memory allocation error or the number of * items added to the mcast_list otherwise. */ static int batadv_mcast_mla_softif_get(struct net_device *dev, @@ -59,7 +59,7 @@ static int batadv_mcast_mla_softif_get(struct net_device *dev, * @mcast_addr: the multicast address to check * @mcast_list: the list with multicast addresses to search in * - * Return true if the given address is already in the given list. + * Returns true if the given address is already in the given list. * Otherwise returns false. */ static bool batadv_mcast_mla_is_duplicate(uint8_t *mcast_addr, @@ -78,7 +78,7 @@ static bool batadv_mcast_mla_is_duplicate(uint8_t *mcast_addr, * batadv_mcast_mla_list_free - free a list of multicast addresses * @mcast_list: the list to free * - * Remove and free all items in the given mcast_list. + * Removes and frees all items in the given mcast_list. */ static void batadv_mcast_mla_list_free(struct hlist_head *mcast_list) { @@ -96,7 +96,7 @@ static void batadv_mcast_mla_list_free(struct hlist_head *mcast_list) * @bat_priv: the bat priv with all the soft interface information * @mcast_list: a list of addresses which should _not_ be removed * - * Retract the announcement of any multicast listener from the + * Retracts the announcement of any multicast listener from the * translation table except the ones listed in the given mcast_list. * * If mcast_list is NULL then all are retracted. @@ -128,7 +128,7 @@ static void batadv_mcast_mla_tt_retract(struct batadv_priv *bat_priv, * @bat_priv: the bat priv with all the soft interface information * @mcast_list: a list of addresses which are going to get added * - * Add multicast listener announcements from the given mcast_list to the + * Adds multicast listener announcements from the given mcast_list to the * translation table if they have not been added yet. */ static void batadv_mcast_mla_tt_add(struct batadv_priv *bat_priv, @@ -159,7 +159,7 @@ static void batadv_mcast_mla_tt_add(struct batadv_priv *bat_priv, * batadv_mcast_has_bridge - check whether the soft-iface is bridged * @bat_priv: the bat priv with all the soft interface information * - * Check whether there is a bridge on top of our soft interface. Return + * Checks whether there is a bridge on top of our soft interface. Returns * true if so, false otherwise. */ static bool batadv_mcast_has_bridge(struct batadv_priv *bat_priv)
b.a.t.m.a.n@lists.open-mesh.org
-
Antonio Quartulli