Re: [B.A.T.M.A.N.] [PATCH 2/2] batman-adv: Simple (re)broadcast avoidance

["Linus Lüssing" <linus.luessing@c0d3.blue>]

On Fri, Jul 22, 2016 at 03:12:57PM +0200, Sven Eckelmann wrote:
> > @@ -517,7 +518,8 @@ batadv_neigh_node_get(const struct batadv_orig_node *orig_node,
> >   */
> >  static struct batadv_hardif_neigh_node *
> >  batadv_hardif_neigh_create(struct batadv_hard_iface *hard_iface,
> > -			   const u8 *neigh_addr)
> > +			   const u8 *neigh_addr,
> > +			   struct batadv_orig_node *orig_node)
> >  {
> >  	struct batadv_priv *bat_priv = netdev_priv(hard_iface->soft_iface);
> >  	struct batadv_hardif_neigh_node *hardif_neigh = NULL;
> > @@ -534,10 +536,12 @@ batadv_hardif_neigh_create(struct batadv_hard_iface *hard_iface,
> >  		goto out;
> >  
> >  	kref_get(&hard_iface->refcount);
> > +	kref_get(&orig_node->refcount);
> >  	INIT_HLIST_NODE(&hardif_neigh->list);
> >  	ether_addr_copy(hardif_neigh->addr, neigh_addr);
> >  	hardif_neigh->if_incoming = hard_iface;
> >  	hardif_neigh->last_seen = jiffies;
> > +	hardif_neigh->orig_node = orig_node;
> 
> Can you please move the kref_get near the place where new reference is used?

Added this suggestion too, although it seems that there is quite
a mixed style at the moment. Most of these allocation functions seem to first
increase refcounts in one block and then do the assignment parts afterwards.

There are a few exceptions though which follow the style you are
suggesting :). (which might be easier to spot refcounting bugs and
therefore nicer, indeed - although the other style might be more
esthetically pleasing as it visually has better alignment prefix
wise, but not sure how much that matters :) )