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 :) )