From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934166AbdKBSvt (ORCPT ); Thu, 2 Nov 2017 14:51:49 -0400 Received: from mail-io0-f196.google.com ([209.85.223.196]:47374 "EHLO mail-io0-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932571AbdKBSvs (ORCPT ); Thu, 2 Nov 2017 14:51:48 -0400 X-Google-Smtp-Source: ABhQp+SGj/NKVvpJ9mMeGGrppqqVYLSQmSdbTjoQsIpvoFSFjg08G5EY9YxuCTNjGhE5U8CzNLUfmw== Message-ID: <1509648700.1011.11.camel@gmail.com> Subject: Re: [PATCH v2] staging: rtlwifi: Fix line too long warning From: Kien Ha To: Greg KH Cc: Dan Carpenter , devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org Date: Thu, 02 Nov 2017 14:51:40 -0400 In-Reply-To: <20171101164723.GA9034@kroah.com> References: <1509297176.4894.4.camel@gmail.com> <20171030120214.knmechriwu62yh2t@mwanda> <1509425284.1199.11.camel@gmail.com> <20171101164723.GA9034@kroah.com> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.26.1 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 Wed, 2017-11-01 at 17:47 +0100, Greg KH wrote: > On Tue, Oct 31, 2017 at 12:48:04AM -0400, Kien Ha wrote: > > > From aa0f4ae8c325545b1fd794d6bbf8c4d2f64e2ec2 Mon Sep 17 00:00:00 > > > 2001 > > > > From: Kien Ha > > Date: Fri, 27 Oct 2017 14:07:55 -0400 > > Subject: [PATCH v2] staging: rtlwifi: Fix line too long warning > > Why is all of this here in the "changelog body" of the patch? Noted. And thanks! > > > > > Made nested if else statement more concise to help conform to > > coding > > style. > > > > Signed-off-by: Kien Ha > > --- > > Changes in v2: > > - Improve block of code to be more concise > > > > drivers/staging/rtlwifi/base.c | 25 ++++++++----------------- > > 1 file changed, 8 insertions(+), 17 deletions(-) > > > > diff --git a/drivers/staging/rtlwifi/base.c > > b/drivers/staging/rtlwifi/base.c > > index b88b0e8edd3d..fdd1ab1e38c5 100644 > > --- a/drivers/staging/rtlwifi/base.c > > +++ b/drivers/staging/rtlwifi/base.c > > @@ -1273,23 +1273,14 @@ void rtl_get_tcb_desc(struct ieee80211_hw > > *hw, > > * and N rate will all be controlled by FW > > * when tcb_desc->use_driver_rate = false > > */ > > - if (sta && sta->vht_cap.vht_supported) { > > - tcb_desc->hw_rate = > > - _rtl_get_vht_highest_n_rate(hw, > > sta); > > - } else { > > - if (sta && (sta- > > >ht_cap.ht_supported)) { > > - tcb_desc->hw_rate = > > - _rtl_get_highest_n_rat > > e(hw, sta); > > - } else { > > - if (rtlmac->mode == > > WIRELESS_MODE_B) { > > - tcb_desc->hw_rate > > = > > - rtlpriv->cfg- > > >maps[RTL_RC_CCK_RATE11M]; > > - } else { > > - tcb_desc->hw_rate > > = > > - rtlpriv->cfg- > > >maps[RTL_RC_OFDM_RATE54M]; > > - } > > - } > > - } > > + tcb_desc->hw_rate = > > + sta && sta->vht_cap.vht_supported > > ? > > + rtl_get_vht_highest_n_rate > > (hw, sta) : > > + sta && sta->ht_cap.ht_supported ? > > + _rtl_get_highest_n_rate(hw > > , sta) : > > + rtlmac->mode == WIRELESS_MODE_B ? > > + rtlpriv->cfg- > > >maps[RTL_RC_CCK_RATE11M] : > > + rtlpriv->cfg- > > >maps[RTL_RC_OFDM_RATE54M]; > > That's horrible to read, can you understand it? > > I hate ? : logic, please write code for people to read, not > compilers. I agree, I find it difficult to read. I'm wondering, is it necessary to have these nested if and else statements? Sorry if it isn't terribly obvious at a glance since I'm writing this while in class. Maybe something like the code snippet below would be better. if (sta && sta->vht_cap.vht_supported) { tcb_desc->hw_rate = - _rtl_get_vht_highest_n_rate(hw, sta); + _rtl_get_vht_highest_n_rate(hw, sta); + } else if (sta && sta->ht_cap.ht_supported) { + tcb_desc->hw_rate = + _rtl_get_highest_n_rate(hw, sta); + } else if (rtlmac->mode == WIRELESS_MODE_B) { + tcb_desc->hw_rate = + rtlpriv->cfg->maps[RTL_RC_CCK_RATE11M]; } else { - if (sta && (sta->ht_cap.ht_supported)) { - tcb_desc->hw_rate = - _rtl_get_highest_n_rate(hw, sta); - } else { - if (rtlmac->mode == WIRELESS_MODE_B) { - tcb_desc->hw_rate = - rtlpriv->cfg->maps[RTL_RC_CCK_RATE11M]; - } else { - tcb_desc->hw_rate = - rtlpriv->cfg->maps[RTL_RC_OFDM_RATE54M]; - } - } + tcb_desc_hw_rate = + rtlpriv->cfg->maps[RTL_RC_OFDM_RATE54M]; } } Thanks, Kien Ha