[3/3] batman-adv: fix rcu pointer comparison problem

Message ID 1385298345-13746-3-git-send-email-sw@simonwunderlich.de (mailing list archive)
State Accepted, archived
Commit f2a130060f657c07fe78d6fae8bb593670d9033f
Headers

Commit Message

Simon Wunderlich Nov. 24, 2013, 1:05 p.m. UTC
  From: Simon Wunderlich <simon@open-mesh.com>

sparse complained about:

routing.c:482:64: error: incompatible types in comparison expression
(different address spaces)

This was due to comparing a __rcu pointer with a regular one. Fix that
by adding the missing rcu_dereference call. A temporary variable is used
to comply with the line length limit, use the opportunity to beautify a
few other places.

This problem was introduced in commit
797edd9e87ac838711e03498a4ae795b600191af ("batman-adv: add bonding
again")

Signed-off-by: Simon Wunderlich <simon@open-mesh.com>
---
 routing.c |   16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)
  

Comments

Marek Lindner Nov. 25, 2013, 5:51 p.m. UTC | #1
On Sunday 24 November 2013 14:05:45 Simon Wunderlich wrote:
> From: Simon Wunderlich <simon@open-mesh.com>
> 
> sparse complained about:
> 
> routing.c:482:64: error: incompatible types in comparison expression
> (different address spaces)
> 
> This was due to comparing a __rcu pointer with a regular one. Fix that
> by adding the missing rcu_dereference call. A temporary variable is used
> to comply with the line length limit, use the opportunity to beautify a
> few other places.
> 
> This problem was introduced in commit
> 797edd9e87ac838711e03498a4ae795b600191af ("batman-adv: add bonding
> again")
> 
> Signed-off-by: Simon Wunderlich <simon@open-mesh.com>
> ---
>  routing.c |   16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)

Applied in revision f2a1300.

Thanks,
Marek
  

Patch

diff --git a/routing.c b/routing.c
index 23311f5..7aac5b3 100644
--- a/routing.c
+++ b/routing.c
@@ -430,8 +430,10 @@  batadv_find_router(struct batadv_priv *bat_priv,
 	struct batadv_neigh_node *first_candidate_router = NULL;
 	struct batadv_neigh_node *next_candidate_router = NULL;
 	struct batadv_neigh_node *router, *cand_router = NULL;
+	struct batadv_neigh_node *last_cand_router = NULL;
 	struct batadv_orig_ifinfo *cand, *first_candidate = NULL;
 	struct batadv_orig_ifinfo *next_candidate = NULL;
+	struct batadv_orig_ifinfo *last_candidate;
 	bool last_candidate_found = false;
 
 	if (!orig_node)
@@ -455,6 +457,10 @@  batadv_find_router(struct batadv_priv *bat_priv,
 	 * router - obviously there are no other candidates.
 	 */
 	rcu_read_lock();
+	last_candidate = orig_node->last_bonding_candidate;
+	if (last_candidate)
+		last_cand_router = rcu_dereference(last_candidate->router);
+
 	hlist_for_each_entry_rcu(cand, &orig_node->ifinfo_list, list) {
 		/* acquire some structures and references ... */
 		if (!atomic_inc_not_zero(&cand->refcount))
@@ -478,9 +484,8 @@  batadv_find_router(struct batadv_priv *bat_priv,
 			goto next;
 
 		/* don't use the same router twice */
-		if (orig_node->last_bonding_candidate &&
-		    (orig_node->last_bonding_candidate->router == cand_router))
-				goto next;
+		if (last_cand_router == cand_router)
+			goto next;
 
 		/* mark the first possible candidate */
 		if (!first_candidate) {
@@ -494,14 +499,13 @@  batadv_find_router(struct batadv_priv *bat_priv,
 		 * candidate ... this function should select the next candidate
 		 * AFTER the previously used bonding candidate.
 		 */
-		if (!orig_node->last_bonding_candidate ||
-		    last_candidate_found) {
+		if (!last_candidate || last_candidate_found) {
 			next_candidate = cand;
 			next_candidate_router = cand_router;
 			break;
 		}
 
-		if (orig_node->last_bonding_candidate == cand)
+		if (last_candidate == cand)
 			last_candidate_found = true;
 next:
 		/* free references */