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

[Antonio Quartulli <ordex@autistici.org>]

[-- Attachment #1: Type: text/plain, Size: 2226 bytes --]

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,


-- 
Antonio Quartulli

..each of us alone is worth nothing..
Ernesto "Che" Guevara

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]