Re: [B.A.T.M.A.N.] [PATCH 2/5] batman-adv: clear ADD+DEL (and viceversa) events in the same orig-interval

On Wed, Apr 18, 2012 at 10:33:34AM +0200, Martin Hundebøll wrote:

Hi Antonio,

On 04/18/2012 12:27 AM, Antonio Quartulli wrote:

...

• /* check for ADD+DEL or DEL+ADD events */ spin_lock_bh(&bat_priv->tt_changes_list_lock);
• list_for_each_entry_safe(entry, safe,&bat_priv->tt_changes_list,

• 		 list) {
  

• if (!compare_eth(entry->change.addr, addr))
  

• 	continue;
  

Please add an empty line here.

Is this really needed for some specific reason?

...

• if (!(!(flags&  TT_CLIENT_DEL)&&  /* ADD op */
  

•       entry->change.flags&  TT_CLIENT_DEL)&&
  

•     !(flags&  TT_CLIENT_DEL&&
  

•       !(entry->change.flags&  TT_CLIENT_DEL))) /* ADD op */
  

• 	continue;
  

This is messy and hard to unerstand. Couldn't you use some tmp vars like this:

int local_del = (flags & TT_CLIENT_DEL) == TT_CLIENT_DEL; int change_del = (entry->change.flags & TT_CLIENT_DEL) == TT_CLIENT_DEL;

if (local_del == change_del) continue;

I'm not 100% sure I understood the original if correctly, but that just proofs the need to rework it :)

eheh, I know I like to mess up the code with boolean formulas :D Thank you for your feedback, I will simplify it.

...

• /* DEL+ADD in the same orig interval have no effect and can be
  

•  * removed to avoid silly behaviour on the receiver side. The
  

•  * other way around (ADD+DEL) can happen in case of roaming of
  

•  * a client still in the NEW state. Roaming of NEW clients is
  

•  * now possible due to automatically recognition of "temporary"
  

•  * clients */
  

Remember newline for */ :)

yes .-.

Thanks again!