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: pgpLdujABmuma.pgp
Description: PGP signature

Reply via email to