On Wednesday, September 9, 2020 5:27:56 PM CEST Linus Lüssing wrote:
On Wed, Sep 09, 2020 at 02:15:51PM +0200, Simon Wunderlich wrote:
On Friday, September 4, 2020 8:28:03 PM CEST Linus Lüssing wrote:
@@ -1626,7 +1626,8 @@ bool batadv_bla_check_bcast_duplist(struct batadv_priv *bat_priv, if (entry->crc != crc)
continue;
if (batadv_compare_eth(entry->orig, bcast_packet->orig))
if (!is_zero_ether_addr(entry->orig) &&batadv_compare_eth(entry->orig, orig)) continue; /* this entry seems to match: same crc, not too old,Shouldn't this check also be skipped if the orig parameter is a zero mac address? i.e.:
if (!is_zero_ether_addr(orig)) {
if (!is_zero_ether_addr(entry->orig) && batadv_compare_eth(entry->orig,
orig))
continue;}
Would be redundant. If entry->orig is non-zero and the compare_eth() says they are equal, then orig must also be non-zero.
OK good point, that's not really obvious (at least to me).
I initially wanted to leave the code as unchanged as possible for net / maint. Should I do the restructuring to enhance readability, with the bool in this patch or in additional patch for net-next?
Personally, I would prefer having a bit more readability or verbose comments in front of those kind of logic if statements. Or avoid those logic connections and have multiple "ifs" in a row where possible to enhance readbility.
This patch is pretty heavy already as is, adding a bool doesn't make a big difference IMHO.
Cheers, Simon