From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753742AbbGRALm (ORCPT ); Fri, 17 Jul 2015 20:11:42 -0400 Received: from smtprelay0151.hostedemail.com ([216.40.44.151]:48886 "EHLO smtprelay.hostedemail.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752002AbbGRALl (ORCPT ); Fri, 17 Jul 2015 20:11:41 -0400 X-Session-Marker: 6A6F6540706572636865732E636F6D X-Spam-Summary: 2,0,0,,d41d8cd98f00b204,joe@perches.com,:::::::,RULES_HIT:41:355:379:541:599:960:973:988:989:1260:1277:1311:1313:1314:1345:1359:1373:1437:1515:1516:1518:1535:1544:1593:1594:1605:1711:1730:1747:1777:1792:2393:2559:2562:2828:2914:3138:3139:3140:3141:3142:3622:3865:3866:3867:3868:3870:3871:3872:3873:3874:4321:4605:5007:6117:6119:6238:6261:7903:8957:10004:10848:11026:11232:11473:11658:11914:12043:12296:12438:12517:12519:12555:12679:12740:13255:21080,0,RBL:none,CacheIP:none,Bayesian:0.5,0.5,0.5,Netcheck:none,DomainCache:0,MSF:not bulk,SPF:fn,MSBL:0,DNSBL:none,Custom_rules:0:0:0 X-HE-Tag: brake41_2b12a86e52c27 X-Filterd-Recvd-Size: 5113 Message-ID: <1437178294.2495.102.camel@perches.com> Subject: Re: [PATCH 2/5] drivers: staging: rtl8192u: fixing coding style issues in r8192U_core.c From: Joe Perches To: Joseph-Eugene Winzer Cc: gregkh@linuxfoundation.org, devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org Date: Fri, 17 Jul 2015 17:11:34 -0700 In-Reply-To: <1437141560-22517-2-git-send-email-m999@openmailbox.org> References: <1437141560-22517-1-git-send-email-m999@openmailbox.org> <1437141560-22517-2-git-send-email-m999@openmailbox.org> Content-Type: text/plain; charset="ISO-8859-1" X-Mailer: Evolution 3.12.11-0ubuntu3 Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, 2015-07-17 at 15:59 +0200, Joseph-Eugene Winzer wrote: > Reformatting the code without introducing other warnings > like 'Avoid unnecessary line continuations' or breaking strings. Very few of these seem to be improvements and many of them should likely be in separate patches. > diff --git a/drivers/staging/rtl8192u/r8192U_core.c b/drivers/staging/rtl8192u/r8192U_core.c [] > @@ -143,17 +143,35 @@ struct CHANNEL_LIST { > }; > > static struct CHANNEL_LIST ChannelPlan[] = { > - {{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 36, 40, 44, 48, 52, 56, 60, 64, 149, 153, 157, 161, 165}, 24}, /* FCC */ > - {{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}, 11}, /* IC */ > - {{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 36, 40, 44, 48, 52, 56, 60, 64}, 21}, /* ETSI */ > - {{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13}, 13}, /* Spain. Change to ETSI. */ > - {{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13}, 13}, /* France. Change to ETSI. */ > - {{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 36, 40, 44, 48, 52, 56, 60, 64}, 22}, /* MKK //MKK */ > - {{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 36, 40, 44, 48, 52, 56, 60, 64}, 22}, /* MKK1 */ > - {{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13}, 13}, /* Israel. */ > - {{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 36, 40, 44, 48, 52, 56, 60, 64}, 22}, /* For 11a , TELEC */ > - {{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 36, 40, 44, 48, 52, 56, 60, 64}, 22}, /* MIC */ > - {{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14}, 14} /* For Global Domain. 1-11:active scan, 12-14 passive scan. //+YJ, 080626 */ If these were to be reformatted, it'd probably be better to use 10 per line so that the Len value that follows is more obvious. Likely this struct should be const too. Maybe something like: static const struct CHANNEL_LIST ChannelPlan[] = { { .Channel = { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 36, 40, 44, 48, 52, 56, 60, 64,149, 153,157,161,165 }, .Len = 24, }, etc. Add more spaces between channels if you really want. > @@ -179,7 +197,9 @@ static void rtl819x_set_channel_map(u8 channel_plan, struct r8192_priv *priv) > min_chan = 1; > max_chan = 14; > } else { > - RT_TRACE(COMP_ERR, "unknown rf chip, can't set channel map in function:%s()\n", __func__); > + RT_TRACE(COMP_ERR, > + "unknown rf chip, can't set channel map in function:%s()\n" > + , __func__); No leading commas please. Put the comma after the format. > @@ -187,7 +207,10 @@ static void rtl819x_set_channel_map(u8 channel_plan, struct r8192_priv *priv) > sizeof(GET_DOT11D_INFO(ieee)->channel_map)); > /* Set new channel map */ > for (i = 0; i < ChannelPlan[channel_plan].Len; i++) { > - if (ChannelPlan[channel_plan].Channel[i] < min_chan || ChannelPlan[channel_plan].Channel[i] > max_chan) > + if (ChannelPlan[channel_plan].Channel[i] < > + min_chan || > + ChannelPlan[channel_plan].Channel[i] > > + max_chan) Likely better to use a temporary for ChannelPlan[channel_plan] so these could become: if (cp->Channel[i] < min_chan || cp->Channel[i] > max_chan) etc... > @@ -1490,7 +1523,8 @@ static u8 QueryIsShort(u8 TxHT, u8 TxRate, cb_desc *tcb_desc) > { > u8 tmp_Short; > > - tmp_Short = (TxHT == 1) ? ((tcb_desc->bUseShortGI) ? 1 : 0) : ((tcb_desc->bUseShortPreamble) ? 1 : 0); > + tmp_Short = (TxHT == 1) ? ((tcb_desc->bUseShortGI) ? 1 : 0) : > + ((tcb_desc->bUseShortPreamble) ? 1 : 0); > > if (TxHT == 1 && TxRate != DESC90_RATEMCS15) > tmp_Short = 0; Maybe using bool would be better. > @@ -1673,7 +1711,8 @@ short rtl8192_tx(struct net_device *dev, struct sk_buff *skb) > 0, tx_zero_isr, dev); > status = usb_submit_urb(tx_urb_zero, GFP_ATOMIC); > if (status) { > - RT_TRACE(COMP_ERR, "Error TX URB for zero byte %d, error %d", atomic_read(&priv->tx_pending[tcb_desc->queue_index]), status); > + RT_TRACE(COMP_ERR, > + "Error TX URB for zero byte %d, error %d", atomic_read(&priv->tx_pending[tcb_desc->queue_index]), status); Probably better line wrapped after format etc...