On Thursday, 21 November 2024 10:23:33 CET Sven Eckelmann wrote: > On Thursday, 21 November 2024 10:13:15 CET Remi Pommarel wrote: > > Looks right to me, I would even simplify that more for readability with: > [...] > > -del: > > - list_del(&entry->list); > > - kmem_cache_free(batadv_tt_change_cache, entry); > > - bat_priv->tt.local_changes--; > > discard: > > kmem_cache_free(batadv_tt_change_cache, tt_change_node); > > goto unlock; > > > > The "discard" is unused. But the rest looks good.
No, it doesn't. You've accidental removed "entry->change.flags = flags;". So
it should look more like this:
--- a/net/batman-adv/translation-table.c
+++ b/net/batman-adv/translation-table.c
@@ -484,34 +484,25 @@ static void batadv_tt_local_event(struct batadv_priv
*bat_priv,
if (!batadv_compare_eth(entry->change.addr, common->addr))
continue;
- /* DEL+ADD in the same orig interval have no effect and can be
- * removed to avoid silly behaviour on the receiver side. The
- * other way around (ADD+DEL) can happen in case of roaming of
- * a client still in the NEW state. Roaming of NEW clients is
- * now possible due to automatically recognition of "temporary"
- * clients
- */
- del_op_entry = entry->change.flags & BATADV_TT_CLIENT_DEL;
- if (!del_op_requested && del_op_entry)
- goto del;
- if (del_op_requested && !del_op_entry)
- goto del;
-
- /* this is a second add or del in the same originator interval.
- * It could mean that flags have been changed (e.g. double
- * add): update them
- */
- if (del_op_requested == del_op_entry) {
+ if (del_op_requested != del_op_entry) {
+ /* DEL+ADD in the same orig interval have no effect and
+ * can be removed to avoid silly behaviour on the
+ * receiver side. The other way around (ADD+DEL) can
+ * happen in case of roaming of a client still in the
+ * NEW state. Roaming of NEW clients is now possible due
+ * to automatically recognition of "temporary" clients
+ */
+ list_del(&entry->list);
+ kmem_cache_free(batadv_tt_change_cache, entry);
+ bat_priv->tt.local_changes--;
+ } else {
+ /* this is a second add or del in the same originator
+ * interval. It could mean that flags have been changed
+ * (e.g. double add): update them
+ */
entry->change.flags = flags;
- goto discard;
}
- continue;
-del:
- list_del(&entry->list);
- kmem_cache_free(batadv_tt_change_cache, entry);
- bat_priv->tt.local_changes--;
-discard:
kmem_cache_free(batadv_tt_change_cache, tt_change_node);
goto unlock;
}
Kind regards,
Sven
signature.asc
Description: This is a digitally signed message part.
