[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <YHEMrQdEFpXgmMh/@kali>
Date: Sat, 10 Apr 2021 07:55:49 +0530
From: Mitali Borkar <mitaliborkar810@...il.com>
To: Joe Perches <joe@...ches.com>
Cc: gregkh@...uxfoundation.org, linux-staging@...ts.linux.dev,
linux-kernel@...r.kernel.org, outreachy-kernel@...glegroups.com,
mitali_s@...iitr.ac.in
Subject: Re: [PATCH 4/6] staging: rtl8192e: matched alignment with open
parenthesis
On Fri, Apr 09, 2021 at 07:07:09PM -0700, Joe Perches wrote:
> On Sat, 2021-04-10 at 07:05 +0530, Mitali Borkar wrote:
> > Matched the alignment with open parenthesis to meet linux kernel coding
> > style.
> > Reported by checkpatch.
> []
> > diff --git a/drivers/staging/rtl8192e/rtl819x_HTProc.c b/drivers/staging/rtl8192e/rtl819x_HTProc.c
> []
> > @@ -154,7 +154,7 @@ bool IsHTHalfNmodeAPs(struct rtllib_device *ieee)
> > (net->ralink_cap_exist))
> > retValue = true;
> > else if (!memcmp(net->bssid, UNKNOWN_BORADCOM, 3) ||
> > - !memcmp(net->bssid, LINKSYSWRT330_LINKSYSWRT300_BROADCOM, 3) ||
> > + !memcmp(net->bssid, LINKSYSWRT330_LINKSYSWRT300_BROADCOM, 3) ||
> > !memcmp(net->bssid, LINKSYSWRT350_LINKSYSWRT150_BROADCOM, 3) ||
> > (net->broadcom_cap_exist))
>
> checkpatch is a stupid script.
> Look further at the code not just at what checkpatch reports.
> Align all the contination lines, not just the first one.
>
Sir, I have aligned them in last patch of this patchset.
> It might be sensible to add a generic function like
>
> static inline bool ether_oui_equal(const u8 *addr, const u8 *oui)
> {
> return addr[0] == oui[0] && addr[1] == oui[1] && addr[2] == oui[2];
> }
>
> to include/linux/etherdevice.h
>
> (Maybe use & instead of && if it's speed sensitive)
>
> so this would read
>
> else if (ether_oui_equal(net->bssid, UNKNOWN_BORADCOM) ||
> ether_oui_equal(net->bssid, LINKSYSWRT330_LINKSYSWRT300_BROADCOM) ||
> ether_oui_equal(net->bssid, LINKSYSWRT350_LINKSYSWRT150_BROADCOM) ||
> net->broacom_cap_exist)
>
> and it'd also be good to correct the typo of UNKNOWN_BORADCOM globally.
>
I will look into this.
> > @@ -654,13 +654,13 @@ void HTInitializeHTInfo(struct rtllib_device *ieee)
> > pHTInfo->CurrentAMPDUFactor = pHTInfo->AMPDU_Factor;
> >
> >
> > memset((void *)(&(pHTInfo->SelfHTCap)), 0,
> > - sizeof(pHTInfo->SelfHTCap));
> > + sizeof(pHTInfo->SelfHTCap));
>
> Doesn't need casts or parentheses.
>
> memset(&pHTInfo->SelfHTCap, 0, sizeof(pHTInfo->SelfHCap));
>
Ok Sir, i am on it.
> > memset((void *)(&(pHTInfo->SelfHTInfo)), 0,
> > - sizeof(pHTInfo->SelfHTInfo));
> > + sizeof(pHTInfo->SelfHTInfo));
>
> etc...
>
> > @@ -815,7 +815,7 @@ void HTUseDefaultSetting(struct rtllib_device *ieee)
> > HTFilterMCSRate(ieee, ieee->Regdot11TxHTOperationalRateSet,
> > ieee->dot11HTOperationalRateSet);
> > ieee->HTHighestOperaRate = HTGetHighestMCSRate(ieee,
> > - ieee->dot11HTOperationalRateSet,
> > + ieee->dot11HTOperationalRateSet,
> > MCS_FILTER_ALL);
>
> multi line statement alignment etc...
>
Sir, I have aligned this patch 6/6
>
Powered by blists - more mailing lists