Re: [PATCH v2 01/15] staging:rtl8192u: Remove macro eqMacAddr - Style

From: John Whitmore
Date: Thu Aug 09 2018 - 04:44:55 EST


On Wed, Aug 08, 2018 at 03:14:19PM -0700, Joe Perches wrote:
> On Wed, 2018-08-08 at 22:00 +0100, John Whitmore wrote:
> > The macro eqMacAddr implements the same functionality as the
> > ether_addr_equal function defined in etherdevice.h, as a result the
> > macro has been removed from the code, and its use replaced with the
> > function call.
> []
> > diff --git a/drivers/staging/rtl8192u/r8192U_core.c b/drivers/staging/rtl8192u/r8192U_core.c
> []
> > @@ -4460,15 +4460,15 @@ static void TranslateRxSignalStuff819xUsb(struct sk_buff *skb,
> >
> > /* Check if the received packet is acceptable. */
> > bpacket_match_bssid = (type != IEEE80211_FTYPE_CTL) &&
> > - (eqMacAddr(priv->ieee80211->current_network.bssid, (fc & IEEE80211_FCTL_TODS) ? hdr->addr1 : (fc & IEEE80211_FCTL_FROMDS) ? hdr->addr2 : hdr->addr3))
> > + (ether_addr_equal(priv->ieee80211->current_network.bssid, (fc & IEEE80211_FCTL_TODS) ? hdr->addr1 : (fc & IEEE80211_FCTL_FROMDS) ? hdr->addr2 : hdr->addr3))
> > && (!pstats->bHwError) && (!pstats->bCRC) && (!pstats->bICV);
> > bpacket_toself = bpacket_match_bssid &
> > - (eqMacAddr(praddr, priv->ieee80211->dev->dev_addr));
> > + (ether_addr_equal(praddr, priv->ieee80211->dev->dev_addr));
>
> Likely this should be
>
> bpacket_match_bssid = bpacket_match_bss &&
>
> as boolean use with a bitwise & is odd.
>

Yes have to agree odd. Somebody famous in Computer Science said that
debugging was twice as difficult as writing code, so why would you
write the cleverest code you possibly could, or something like that.
That's what I thought when I now looked at that line of code.

I've been going through this driver cleaning up header files first.
Once those are sorted I was going to turn to the C and try get my
head around the logic. I'll have to look at the functionality of that
bpacket_toself variable but a bitwise operation does look odd.