On Wed, 2012-03-14 at 15:18 +1100, Ryan Mallon wrote:
> That's still pretty nasty. Eight lines for an if expression! It took me
> a couple of glances to realise that there were only two arguments to
> eqMacAddr. If you add some temporary variables then you can make the
> code a lot more sane, without making it overly verbose:
>
> u8 *addr, *bssid = priv->ieee80211->current_network.bssid;
>
> if (fc & IEEE80211_FCTL_TODS)
> addr = hdr->addr1;
> else if (fc & IEEE80211_FCTL_FROMDS)
> addr = hdr->addr2;
> else
> addr = hdr->addr3;
>
> if (!bHwError && !bCRC && !bICV &&
> type != IEEE80211_FTYPE_CTL && eqMacAddr(bssid, addr)) {
> ...
>
> Also note that the whole if block is indented one too many tab stops.
> Would be best to fix that in a separate patch though. If you fix that
> first, then you will have more horizontal space to re-organise the
> expression inside the if :-).
Yeah, what Ryan said. Clarity is good.
You might separate the addr and bssid declarations
into 2 lines though...
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
