On Samstag, 29. Oktober 2016 04:46:46 CEST Linus Lüssing wrote: [...]
[...]
+bool batadv_forw_packet_steal(struct batadv_forw_packet *forw_packet,
spinlock_t *lock)+{
- struct hlist_head head = HLIST_HEAD_INIT;
[...]
- /* Just to spot misuse of this function */
- hlist_add_head(&forw_packet->bm_list, &head);
- hlist_add_fake(&forw_packet->bm_list);
Sorry, I don't get how this should spot misuse via this extra hlist_add_head. You first add the packet to the list (on the stack) and then setting pprev pointer to itself. So you basically have a fake hashed node with next pointer set to NULL. Wouldn't it be better here to use INIT_HLIST_NODE instead of hlist_add_head? I would even say that INIT_HLIST_NODE isn't needed here because you already did this during batadv_forw_packet_alloc.
But I would assume that you actually only wanted hlist_add_fake for the WARN_ONCE in batadv_forw_packet_queue, right?
Right :). I'm only using the _fake() variant for this WARN_ONCE. (So I'll leave that as is? Or too confusing? Do you want me to reword / clarify something better in the code?)
I think the _fake stuff + comment is ok. But the add to the hlist_head on the stack is confusing and maybe should be removed.
Kind regards, Sven