From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752763AbbG1RuB (ORCPT ); Tue, 28 Jul 2015 13:50:01 -0400 Received: from mail-pd0-f172.google.com ([209.85.192.172]:35546 "EHLO mail-pd0-f172.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751386AbbG1Rt7 (ORCPT ); Tue, 28 Jul 2015 13:49:59 -0400 From: Joshua Clayton To: Dan Carpenter Cc: Julia Lawall , devel@driverdev.osuosl.org, Florian Schilhabel , Greg Kroah-Hartman , Nitin Kuppelur , linux-kernel@vger.kernel.org, Sudip Mukherjee , Larry Finger Subject: Re: [PATCH V3 RESEND 8/8] staging: rtl8712: change SupportedRates to rates Date: Tue, 28 Jul 2015 10:49:52 -0700 Message-ID: <11512207.BLTbrXTcg1@jclayton-pc> User-Agent: KMail/4.14.6 (Linux/3.19.0-21-generic; KDE/4.14.6; x86_64; ; ) In-Reply-To: <20150728155638.GE5180@mwanda> References: <9594685.hpOiZGHKrk@jclayton-pc> <20150728155638.GE5180@mwanda> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tuesday, July 28, 2015 06:56:38 PM Dan Carpenter wrote: > On Tue, Jul 28, 2015 at 08:50:04AM -0700, Joshua Clayton wrote: > > On Tuesday, July 28, 2015 05:37:16 PM Julia Lawall wrote: > > > > diff --git a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c > > > > index 83d4bca..1ded691 100644 > > > > --- a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c > > > > +++ b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c > > > > @@ -204,13 +204,13 @@ static inline char *translate_scan(struct _adapter *padapter, > > > > /* Add the protocol name */ > > > > iwe.cmd = SIOCGIWNAME; > > > > if ((r8712_is_cckratesonly_included(pnetwork->network. > > > > - SupportedRates)) == true) { > > > > + rates)) == true) { > > > > 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) { > > > > + rates)) == true) { > > > > if (ht_cap == true) > > > > snprintf(iwe.u.name, IFNAMSIZ, "IEEE 802.11bgn"); > > > > else > > > > > > The placement of the line breaks here is partucularly unpleasant. Perhaps > > > that could be adjusted now that the field name is shorter. Theer may be > > > some other cases in the patch where the line breaks could be adjusted to > > > improve readability. > > > > > > julia > > I agree. Dan Carpenter earlier complained about me fixing up the lines > > while changing the variable name. Should that be a separate patch? > > > > 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? 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. > > regards, > dan carpenter > -- ~Joshua Clayton