Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Message-ID: <f5fe04d62b22eb5e09c299e769d9b9d8917f119c.camel@perches.com>
Subject: Re: [PATCH 4/6] staging: rtl8192e: matched alignment with open
 parenthesis
From:   Joe Perches <joe@perches.com>
To:     Mitali Borkar <mitaliborkar810@gmail.com>,
        gregkh@linuxfoundation.org
Cc:     linux-staging@lists.linux.dev, linux-kernel@vger.kernel.org,
        outreachy-kernel@googlegroups.com, mitali_s@me.iitr.ac.in
Date:   Fri, 09 Apr 2021 19:07:09 -0700
In-Reply-To: <YHEA2Te3Hik5J39t@kali>
References: <YHEA2Te3Hik5J39t@kali>
Content-Type: text/plain; charset="ISO-8859-1"
User-Agent: Evolution 3.38.1-1 
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

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.

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.

> @@ -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));

> ?	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...