diff mbox

[v2,3/4] batman-adv: let tt_global_entry_has_orig() return the orig_entry or NULL instead of 1 or 0 only

Message ID 1333543332-26942-4-git-send-email-ordex@autistici.org (mailing list archive)
State Superseded, archived
Headers

Commit Message

Antonio Quartulli April 4, 2012, 12:42 p.m. UTC 
  Instead of returning only 1 or 0 this function has to return the found
orig_entry (if any). In this way, operations that have to to modify the
found orig_entry structure will not need to reiterate over the list again to
find the wanted node.

Signed-off-by: Antonio Quartulli <ordex@autistici.org>
---
 translation-table.c |   13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

Comments

Martin Hundebøll April 9, 2012, 4:55 p.m. UTC | #1 
Hi Antonio,

On 04/04/2012 02:42 PM, Antonio Quartulli wrote:

>   	rcu_read_lock();
>   	head =&entry->orig_list;
>   	hlist_for_each_entry_rcu(tmp_orig_entry, node, head, list) {
>   		if (tmp_orig_entry->orig_node == orig_node) {
> -			found = true;
> +			orig_entry = tmp_orig_entry;
>   			break;
>   		}
>   	}
>   	rcu_read_unlock();
> -	return found;
> +	return orig_entry;
>   }

I'm not that well into the tt code, but shouldn't there be some reference counting when handing pointers round like this?
Antonio Quartulli April 10, 2012, 6:17 p.m. UTC | #2 
Hi Martin,

On Mon, Apr 09, 2012 at 06:55:10PM +0200, Martin Hundebøll wrote:
> Hi Antonio,
> 
> On 04/04/2012 02:42 PM, Antonio Quartulli wrote:
> 
> >   	rcu_read_lock();
> >   	head =&entry->orig_list;
> >   	hlist_for_each_entry_rcu(tmp_orig_entry, node, head, list) {
> >   		if (tmp_orig_entry->orig_node == orig_node) {
> > -			found = true;
> > +			orig_entry = tmp_orig_entry;
> >   			break;
> >   		}
> >   	}
> >   	rcu_read_unlock();
> > -	return found;
> > +	return orig_entry;
> >   }
> 
> I'm not that well into the tt code, but shouldn't there be some reference counting when handing pointers round like this?
> 

mh..I'd agree with you. The entry could be freed somewhere else causing
something we would never imagine ;P

Actually the tt_orig_list_entry doesn't have any mechanism like that and so I
forgot about taking care of this issue.

Thank you very much!

Cheers,
diff mbox

Patch

diff --git a/translation-table.c b/translation-table.c
index a2c1240..0ce2ed9 100644
--- a/translation-table.c
+++ b/translation-table.c
@@ -543,25 +543,26 @@  static void tt_changes_list_free(struct bat_priv *bat_priv)
 }
 
 /* find out if an orig_node is already in the list of a tt_global_entry.
- * returns 1 if found, 0 otherwise */
-static bool tt_global_entry_has_orig(const struct tt_global_entry *entry,
-				     const struct orig_node *orig_node)
+ * returns NULL if not found, the pointer to the orig_entry otherwise */
+static struct tt_orig_list_entry *tt_global_entry_has_orig(
+					const struct tt_global_entry *entry,
+					const struct orig_node *orig_node)
 {
 	struct tt_orig_list_entry *tmp_orig_entry;
 	const struct hlist_head *head;
 	struct hlist_node *node;
-	bool found = false;
+	struct tt_orig_list_entry *orig_entry = NULL;
 
 	rcu_read_lock();
 	head = &entry->orig_list;
 	hlist_for_each_entry_rcu(tmp_orig_entry, node, head, list) {
 		if (tmp_orig_entry->orig_node == orig_node) {
-			found = true;
+			orig_entry = tmp_orig_entry;
 			break;
 		}
 	}
 	rcu_read_unlock();
-	return found;
+	return orig_entry;
 }
 
 static void tt_global_add_orig_entry(struct tt_global_entry *tt_global_entry,