Re: [B.A.T.M.A.N.] [PATCHv2 7/9] batman-adv: adapt the neighbor purging routine to use the new API functions

[Simon Wunderlich <simon.wunderlich@s2003.tu-chemnitz.de>]

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

On Mon, Aug 12, 2013 at 11:25:35PM +0200, Antonio Quartulli wrote:
> On Mon, Aug 12, 2013 at 07:51:02PM +0200, Antonio Quartulli wrote:
> > On Mon, Aug 12, 2013 at 04:31:29PM +0200, Simon Wunderlich wrote:
> > > On Sun, Aug 11, 2013 at 02:33:21PM +0200, Antonio Quartulli wrote:
> > > > From: Antonio Quartulli <antonio@open-mesh.com>
> > > > 
> > > > Signed-off-by: Antonio Quartulli <antonio@open-mesh.com>
> > > > ---
> > > >  originator.c | 9 ++++++---
> > > >  1 file changed, 6 insertions(+), 3 deletions(-)
> > > > 
> > > > diff --git a/originator.c b/originator.c
> > > > index b3f6910..3ee8806 100644
> > > > --- a/originator.c
> > > > +++ b/originator.c
> > > > @@ -380,6 +380,8 @@ batadv_purge_orig_neighbors(struct batadv_priv *bat_priv,
> > > >  	bool neigh_purged = false;
> > > >  	unsigned long last_seen;
> > > >  	struct batadv_hard_iface *if_incoming;
> > > > +	struct batadv_algo_ops *bao = bat_priv->bat_algo_ops;
> > > > +	uint32_t neigh_metric, best_metric;
> > > >  
> > > >  	*best_neigh_node = NULL;
> > > >  
> > > > @@ -414,10 +416,11 @@ batadv_purge_orig_neighbors(struct batadv_priv *bat_priv,
> > > >  			batadv_bonding_candidate_del(orig_node, neigh_node);
> > > >  			batadv_neigh_node_free_ref(neigh_node);
> > > >  		} else {
> > > > -			if ((!*best_neigh_node) ||
> > > > -			    (neigh_node->bat_iv.tq_avg >
> > > > -			     (*best_neigh_node)->bat_iv.tq_avg))
> > > > +			neigh_metric = bao->bat_metric_get(neigh_node);
> > > > +			if (!*best_neigh_node || (neigh_metric > best_metric)) {
> > > 
> > > Shouldn't you initialize best_metric at some point?
> > 
> > Actually I did initialise it with UINT_MAX but I have the feeling I forgot to
> > "add" the change to the commit. However this is only saving us from a warning
> > (which means the initialisation is needed) but not from any other problem:
> > best_metric will always have a value when best_neigh_node will be not NULL.
> > 
> > Anyhow, good catch :)
> > Thanks for raising the point.
> > 
> > Do you think I should send the entire patchset again? Or can I just resend this
> > patch only? Marek?
> 
> Sorry, I was wrong. The initialisation I was talking about is done in
> batadv_find_ifalter_router() (so I did not forgot to add it :-)).
> 
> Here instead it is not useful at all.

I think we still need an initialisation here. Even my compiler complains. :)

Cheers,
	Simon

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