Messages in this thread | | | Subject | Re: [PATCH] Staging: rtl8187se: r8180_core.c: Fix coding style issue | From | Joe Perches <> | Date | Tue, 13 Mar 2012 21:49:15 -0700 |
| |
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...
| |