Re: [B.A.T.M.A.N.] [PATCH 4/5] batman-adv: detect not yet announced clients

On Tue, Jun 26, 2012 at 11:50:22PM +0200, Sven Eckelmann wrote:

...

for (i = 0; i < hash->size; i++) { head = &hash->table[i]; list_lock = &hash->list_locks[i];

spin_lock_bh(list_lock);

• batadv_tt_global_roam_purge_list(bat_priv, head);
  

• hlist_for_each_entry_safe(tt_common_entry, node, node_tmp,
  

• 			  head, hash_entry) {
  

• 	purge = false;
  

• 	tt_global = container_of(tt_common_entry,
  

• 				 struct batadv_tt_global_entry,
  

• 				 common);
  

• 	if ((tt_global->common.flags & BATADV_TT_CLIENT_ROAM) &&
  

• 	    batadv_has_timed_out(tt_global->roam_at,
  

• 				 roam_timeout)) {
  

• 		purge = true;
  

• 		msg = "Roaming timeout\n";
  

• 	}
  

• 	if ((tt_global->common.flags & BATADV_TT_CLIENT_TEMP) &&
  

• 	    batadv_has_timed_out(tt_global->common.added_at,
  

• 				 temp_timeout)) {
  

• 		purge = true;
  

• 		msg = "Temporary client timeout\n";
  

• 	}
  

• 	if (!purge)
  

• 		continue;
  

• 	batadv_dbg(BATADV_DBG_TT, bat_priv,
  

• 		   "Deleting global tt entry (%pM): %s\n",
  

• 		   tt_global->common.addr, msg);
  

• 	hlist_del_rcu(node);
  

• 	batadv_tt_global_entry_free_ref(tt_global);
  

• }
  

  spin_unlock_bh(list_lock); }

Why writing this whole thing in one function? You don't get a price for the highest indentation level that banged the hardest against the 80 character limit.

Mh, I like challenges :-) However, I'll add another function like: has_to_be_purged() or similar in which i will check for the conditions and I will use it only in the loop.

...

• if (tt_common_entry->flags & BATADV_TT_CLIENT_ROAM)

• if (tt_common_entry->flags & BATADV_TT_CLIENT_ROAM ||

•    tt_common_entry->flags & BATADV_TT_CLIENT_TEMP)
  

  return 0;

• tt_global_entry = container_of(tt_common_entry, struct batadv_tt_global_entry, common);

And this new line doesn't make sense

I agree.

Thank you very much Sven.

Cheers,