linux-kernel - Re: [PATCH V3 RESEND 8/8] staging: rtl8712: change SupportedRates to rates

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20150728220758.GB5096@mwanda>
Date:	Wed, 29 Jul 2015 01:07:58 +0300
From:	Dan Carpenter <dan.carpenter@...cle.com>
To:	Joshua Clayton <stillcompiling@...il.com>
Cc:	Julia Lawall <julia.lawall@...6.fr>, devel@...verdev.osuosl.org,
	Florian Schilhabel <florian.c.schilhabel@...glemail.com>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Nitin Kuppelur <nitinkuppelur@...il.com>,
	linux-kernel@...r.kernel.org,
	Sudip Mukherjee <sudipm.mukherjee@...il.com>,
	Larry Finger <Larry.Finger@...inger.net>
Subject: Re: [PATCH V3 RESEND 8/8] staging: rtl8712: change SupportedRates to
 rates

On Tue, Jul 28, 2015 at 10:49:52AM -0700, Joshua Clayton wrote:
> > Changing the line breaks here is a tiny change on the same line and so
> > it's fine.  It fits into the one thing per patch rule.
> 
> This is the style I prefer (getting rid of the explicit == true)
> 
> -	if ((r8712_is_cckratesonly_included(pnetwork->network.
> -	     SupportedRates)) == true) {
> +	if (r8712_is_cckratesonly_included(pnetwork->network.rates)) {
>  		if (ht_cap == true)
>  			snprintf(iwe.u.name, IFNAMSIZ, "IEEE 802.11bn");
>  		else
>  			snprintf(iwe.u.name, IFNAMSIZ, "IEEE 802.11b");
> -	} else if ((r8712_is_cckrates_included(pnetwork->network.
> -		    SupportedRates)) == true) {
> +	} else if (r8712_is_cckrates_included(pnetwork->network.rates)) {
>  		if (ht_cap == true)
>  			snprintf(iwe.u.name, IFNAMSIZ, "IEEE 802.11bgn");
> 
> Does that look ok?

Yes.  It looks better.

> If we keep the "== true" and the extra set of parentheses, the "else if" case goes over 80 lines.
> I will happily submit the change as follow up patch if that is too much to change at once. 

The "one thing per patch" rule is a fuzzy line. My scripts don't care
about white space very much so moving the .rates to the other line is
not a big deal.  Also the line break was really bad and the == true is
only mildly untidy at worst.  If it's a massive patch then removing the
== true makes things more difficult, yes, but if it's small then it's
probably fine to do it at once since it's on the same line and makes the
80 character rule work.

What I'm saying is that just use your best judgement what is easy to
review and we try to be reasonable as well.

regards,
dan carpenter

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