lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <1331700555.27389.51.camel@joe2Laptop>
Date:	Tue, 13 Mar 2012 21:49:15 -0700
From:	Joe Perches <joe@...ches.com>
To:	Ryan Mallon <rmallon@...il.com>
Cc:	Andrew Miller <amiller@...lx.com>, gregkh@...uxfoundation.org,
	devel@...verdev.osuosl.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] Staging: rtl8187se: r8180_core.c: Fix coding style issue

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...

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ