On Friday, August 16, 2013 14:27:29 Antonio Quartulli wrote:
On Fri, Aug 16, 2013 at 11:46:41AM +0800, Marek Lindner wrote:
On Tuesday, August 13, 2013 14:43:44 Antonio Quartulli wrote:
@@ -1011,13 +1011,13 @@ batadv_iv_ogm_update_seqnos(const struct ethhdr *ethhdr, }
/* if the window moved, set the update flag. */
need_update |= batadv_bit_get_packet(bat_priv,tmp_neigh_node->real_bits,
bitmap = tmp_neigh_node->bat_iv.real_bits;need_update |= batadv_bit_get_packet(bat_priv, bitmap, seq_diff, set_mark);
packet_count = bitmap_weight(tmp_neigh_node->real_bits,
packet_count = bitmap_weight(tmp_neigh_node->bat_iv.real_bits, BATADV_TQ_LOCAL_WINDOW_SIZE);
tmp_neigh_node->real_packet_count = packet_count;
tmp_neigh_node->bat_iv.real_packet_count = packet_count;}
You can't make assumptions about the size of bitmap because it is platform specific which is why the variable declaration is a macro in the first place.
where do I make such assumption? Variable bitmap is a just storing the address of the bitmap.
If it is just the address it probably is ok - you are right.
struct batadv_priv *bat_priv = netdev_priv(hard_iface->soft_iface);
struct batadv_neigh_node *neigh_node;
neigh_node = kzalloc(sizeof(*neigh_node), GFP_ATOMIC); if (!neigh_node)
goto out;
INIT_HLIST_NODE(&neigh_node->list);
memcpy(neigh_node->addr, neigh_addr, ETH_ALEN);
spin_lock_init(&neigh_node->lq_update_lock);
neigh_node->if_incoming = hard_iface;
neigh_node->orig_node = orig_node;
spin_lock_bh(&orig_node->neigh_list_lock);
hlist_add_head_rcu(&neigh_node->list, &orig_node->neigh_list);
spin_unlock_bh(&orig_node->neigh_list_lock);
INIT_LIST_HEAD(&neigh_node->bonding_list);
/* extra reference for return */ atomic_set(&neigh_node->refcount, 2);
- batadv_dbg(BATADV_DBG_BATMAN, bat_priv,
"Creating new neighbor %pM on interface %s\n", neigh_addr,hard_iface->net_dev->name);out: return neigh_node;
}
All variables should be initialized before we add the new entry to the any list.
object is allocated with kzalloc: everything is zero. I thought we use kzalloc for this purpose: avoid initialisation of every field..no?
What is "INIT_LIST_HEAD(&neigh_node->bonding_list);" ? Looks like an initialization if I am not mistaken. Moreover, batadv_iv_ogm_neigh_new() inits even more after this function returns ...
Cheers, Marek