Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Sun, 9 Oct 2022 12:46:44 +0200
From:   Nam Cao <namcaov@gmail.com>
To:     Rui Li <me@lirui.org>
Cc:     Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        linux-staging@lists.linux.dev, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] staging: rtl8192e: Fix braces/tabs/number/OOM warnings
Message-ID: <20221009104644.GA49790@nam-dell>
References: <166528776854.9.8249842126243786800.67724771@lirui.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <166528776854.9.8249842126243786800.67724771@lirui.org>
Precedence: bulk

On Sun, Oct 09, 2022 at 11:55:36AM +0800, Rui Li wrote:
> Fix warnings generated by checkpatch.pl: unnecessary braces after
> if, too many leading tabs, int type conversion before number,
> OOM message ourput.
> 
> Signed-off-by: Rui Li <me@lirui.org>
> ---
>  .../staging/rtl8192e/rtl8192e/r8192E_dev.c    |  3 +-
>  .../staging/rtl8192e/rtl8192e/r8192E_phy.c    |  9 ++----
>  drivers/staging/rtl8192e/rtl8192e/rtl_core.c  |  3 --
>  drivers/staging/rtl8192e/rtl8192e/rtl_dm.c    | 30 +++++++------------
>  drivers/staging/rtl8192e/rtl819x_BAProc.c     |  5 ++--
>  drivers/staging/rtl8192e/rtl819x_HTProc.c     |  1 +
>  drivers/staging/rtl8192e/rtllib_rx.c          |  8 ++---
>  drivers/staging/rtl8192e/rtllib_softmac_wx.c  |  7 ++---
>  8 files changed, 25 insertions(+), 41 deletions(-)
> 
> diff --git a/drivers/staging/rtl8192e/rtl8192e/r8192E_dev.c b/drivers/staging/rtl8192e/rtl8192e/r8192E_dev.c
> index 18e4e5d84878..8d20b0deca37 100644
> --- a/drivers/staging/rtl8192e/rtl8192e/r8192E_dev.c
> +++ b/drivers/staging/rtl8192e/rtl8192e/r8192E_dev.c
> @@ -1112,9 +1112,8 @@ void  rtl92e_fill_tx_desc(struct net_device *dev, struct tx_desc *pdesc,
>  	if (cb_desc->bHwSec) {
>  		static u8 tmp;
>  
> -		if (!tmp) {
> +		if (!tmp)
>  			tmp = 1;
> -		}
>  		switch (priv->rtllib->pairwise_key_type) {
>  		case KEY_TYPE_WEP40:
>  		case KEY_TYPE_WEP104:
> diff --git a/drivers/staging/rtl8192e/rtl8192e/r8192E_phy.c b/drivers/staging/rtl8192e/rtl8192e/r8192E_phy.c
> index 1b592258e640..4e3d183be0f2 100644
> --- a/drivers/staging/rtl8192e/rtl8192e/r8192E_phy.c
> +++ b/drivers/staging/rtl8192e/rtl8192e/r8192E_phy.c
> @@ -522,9 +522,8 @@ static bool _rtl92e_bb_config_para_file(struct net_device *dev)
>  		rtStatus  = rtl92e_check_bb_and_rf(dev,
>  						   (enum hw90_block)eCheckItem,
>  						   (enum rf90_radio_path)0);
> -		if (!rtStatus) {
> +		if (!rtStatus)
>  			return rtStatus;
> -		}
>  	}
>  	rtl92e_set_bb_reg(dev, rFPGA0_RFMOD, bCCKEn|bOFDMEn, 0x0);
>  	_rtl92e_phy_config_bb(dev, BaseBand_Config_PHY_REG);
> @@ -1379,9 +1378,8 @@ static bool _rtl92e_set_rf_power_state(struct net_device *dev,
>  					i++;
>  				}
>  
> -				if (i >= MAX_DOZE_WAITING_TIMES_9x) {
> +				if (i >= MAX_DOZE_WAITING_TIMES_9x)
>  					break;
> -				}
>  			}
>  			rtl92e_set_rf_off(dev);
>  			break;
> @@ -1398,9 +1396,8 @@ static bool _rtl92e_set_rf_power_state(struct net_device *dev,
>  					i++;
>  				}
>  
> -				if (i >= MAX_DOZE_WAITING_TIMES_9x) {
> +				if (i >= MAX_DOZE_WAITING_TIMES_9x)
>  					break;
> -				}
>  			}
>  
>  			if (pPSC->RegRfPsLevel & RT_RF_OFF_LEVL_HALT_NIC &&
> diff --git a/drivers/staging/rtl8192e/rtl8192e/rtl_core.c b/drivers/staging/rtl8192e/rtl8192e/rtl_core.c
> index 89bc989cffba..b2facb273474 100644
> --- a/drivers/staging/rtl8192e/rtl8192e/rtl_core.c
> +++ b/drivers/staging/rtl8192e/rtl8192e/rtl_core.c
> @@ -908,9 +908,6 @@ static void _rtl92e_init_priv_variable(struct net_device *dev)
>  	priv->card_type = PCI;
>  
>  	priv->pFirmware = vzalloc(sizeof(struct rt_firmware));
> -	if (!priv->pFirmware)
> -		netdev_err(dev,
> -			   "rtl8192e: Unable to allocate space for firmware\n");
>  
>  	skb_queue_head_init(&priv->skb_queue);
>  
> diff --git a/drivers/staging/rtl8192e/rtl8192e/rtl_dm.c b/drivers/staging/rtl8192e/rtl8192e/rtl_dm.c
> index 702551056227..32494ad2298b 100644
> --- a/drivers/staging/rtl8192e/rtl8192e/rtl_dm.c
> +++ b/drivers/staging/rtl8192e/rtl8192e/rtl_dm.c
> @@ -267,9 +267,8 @@ static void _rtl92e_dm_check_ac_dc_power(struct net_device *dev)
>  			"PATH=/usr/bin:/bin",
>  			 NULL};
>  
> -	if (priv->ResetProgress == RESET_TYPE_SILENT) {
> +	if (priv->ResetProgress == RESET_TYPE_SILENT)
>  		return;
> -	}
>  
>  	if (priv->rtllib->state != RTLLIB_LINKED)
>  		return;
> @@ -330,9 +329,8 @@ static void _rtl92e_dm_check_rate_adaptive(struct net_device *dev)
>  	bool bshort_gi_enabled = false;
>  	static u8 ping_rssi_state;
>  
> -	if (!priv->up) {
> +	if (!priv->up)
>  		return;
> -	}
>  
>  	if (pra->rate_adaptive_disabled)
>  		return;
> @@ -777,9 +775,8 @@ static void _rtl92e_dm_tx_power_tracking_cb_thermal(struct net_device *dev)
>  		tmpRegA = rtl92e_get_bb_reg(dev, rOFDM0_XATxIQImbalance,
>  					    bMaskDWord);
>  		for (i = 0; i < OFDM_Table_Length; i++) {
> -			if (tmpRegA == OFDMSwingTable[i]) {
> +			if (tmpRegA == OFDMSwingTable[i])
>  				priv->OFDM_index[0] = i;
> -			}
>  		}
>  
>  		TempCCk = rtl92e_get_bb_reg(dev, rCCK0_TxFilter1, bMaskByte2);
> @@ -1066,9 +1063,8 @@ void rtl92e_dm_restore_state(struct net_device *dev)
>  	u32	reg_ratr = priv->rate_adaptive.last_ratr;
>  	u32 ratr_value;
>  
> -	if (!priv->up) {
> +	if (!priv->up)
>  		return;
> -	}
>  
>  	if (priv->rate_adaptive.rate_adaptive_disabled)
>  		return;
> @@ -1877,20 +1873,16 @@ static void _rtl92e_dm_rx_path_sel_byrssi(struct net_device *dev)
>  						tmp_cck_sec_pwdb = cur_cck_pwdb;
>  						cck_rx_ver2_sec_index = i;
>  					} else if (cur_cck_pwdb ==
> -						   tmp_cck_sec_pwdb) {
> -						if (tmp_cck_sec_pwdb ==
> -						    tmp_cck_min_pwdb) {
> -							tmp_cck_sec_pwdb =
> -								 cur_cck_pwdb;
> -							cck_rx_ver2_sec_index =
> -								 i;
> -						}
> +							tmp_cck_sec_pwdb &&
> +							tmp_cck_sec_pwdb == tmp_cck_min_pwdb) {
> +						tmp_cck_sec_pwdb = cur_cck_pwdb;
> +						cck_rx_ver2_sec_index = i;

This is not functionally equivalent. Are you sure the entire if else
chain still behaves correctly?

>  					} else if ((cur_cck_pwdb < tmp_cck_sec_pwdb) &&
>  						   (cur_cck_pwdb > tmp_cck_min_pwdb)) {
>  						;
> -					} else if (cur_cck_pwdb == tmp_cck_min_pwdb) {
> -						if (tmp_cck_sec_pwdb == tmp_cck_min_pwdb)
> -							tmp_cck_min_pwdb = cur_cck_pwdb;
> +					} else if (cur_cck_pwdb == tmp_cck_min_pwdb &&
> +						tmp_cck_sec_pwdb == tmp_cck_min_pwdb) {
> +						tmp_cck_min_pwdb = cur_cck_pwdb;

Same story as above.

>  					} else if (cur_cck_pwdb < tmp_cck_min_pwdb) {
>  						tmp_cck_min_pwdb = cur_cck_pwdb;
>  					}
> diff --git a/drivers/staging/rtl8192e/rtl819x_BAProc.c b/drivers/staging/rtl8192e/rtl819x_BAProc.c
> index 19d13b3fcecf..e932ad1a9e96 100644
> --- a/drivers/staging/rtl8192e/rtl819x_BAProc.c
> +++ b/drivers/staging/rtl8192e/rtl819x_BAProc.c
> @@ -180,11 +180,10 @@ static void rtllib_send_ADDBAReq(struct rtllib_device *ieee, u8 *dst,
>  
>  	skb = rtllib_ADDBA(ieee, dst, pBA, 0, ACT_ADDBAREQ);
>  
> -	if (skb) {
> +	if (skb)
>  		softmac_mgmt_xmit(skb, ieee);
> -	} else {
> +	else
>  		netdev_dbg(ieee->dev, "Failed to generate ADDBAReq packet.\n");
> -	}
>  }
>  
>  static void rtllib_send_ADDBARsp(struct rtllib_device *ieee, u8 *dst,
> diff --git a/drivers/staging/rtl8192e/rtl819x_HTProc.c b/drivers/staging/rtl8192e/rtl819x_HTProc.c
> index ef3dca51cf99..b763cf0ba356 100644
> --- a/drivers/staging/rtl8192e/rtl819x_HTProc.c
> +++ b/drivers/staging/rtl8192e/rtl819x_HTProc.c
> @@ -70,6 +70,7 @@ static u8 LINKSYS_MARVELL_4400N[3] = {0x00, 0x14, 0xa4};
>  void HTUpdateDefaultSetting(struct rtllib_device *ieee)
>  {
>  	struct rt_hi_throughput *pHTInfo = ieee->pHTInfo;
> +
>  	pHTInfo->bRegShortGI20MHz = 1;
>  	pHTInfo->bRegShortGI40MHz = 1;
>  
> diff --git a/drivers/staging/rtl8192e/rtllib_rx.c b/drivers/staging/rtl8192e/rtllib_rx.c
> index 46d75e925ee9..ca7eba826ece 100644
> --- a/drivers/staging/rtl8192e/rtllib_rx.c
> +++ b/drivers/staging/rtl8192e/rtllib_rx.c
> @@ -454,14 +454,14 @@ static bool AddReorderEntry(struct rx_ts_record *pTS,
>  	while (pList->next != &pTS->rx_pending_pkt_list) {
>  		if (SN_LESS(pReorderEntry->SeqNum, ((struct rx_reorder_entry *)
>  		    list_entry(pList->next, struct rx_reorder_entry,
> -		    List))->SeqNum))
> +		    List))->SeqNum)) {
>  			pList = pList->next;
> -		else if (SN_EQUAL(pReorderEntry->SeqNum,
> +			continue;
> +		} else if (SN_EQUAL(pReorderEntry->SeqNum,
>  			((struct rx_reorder_entry *)list_entry(pList->next,
>  			struct rx_reorder_entry, List))->SeqNum))
>  			return false;
> -		else
> -			break;
> +		break;
>  	}
>  	pReorderEntry->List.next = pList->next;
>  	pReorderEntry->List.next->prev = &pReorderEntry->List;
> diff --git a/drivers/staging/rtl8192e/rtllib_softmac_wx.c b/drivers/staging/rtl8192e/rtllib_softmac_wx.c
> index f9589c5b62ba..4fc4fb25d8d6 100644
> --- a/drivers/staging/rtl8192e/rtllib_softmac_wx.c
> +++ b/drivers/staging/rtl8192e/rtllib_softmac_wx.c
> @@ -41,8 +41,8 @@ int rtllib_wx_set_freq(struct rtllib_device *ieee, struct iw_request_info *a,
>  
>  	/* if setting by freq convert to channel */
>  	if (fwrq->e == 1) {
> -		if ((fwrq->m >= (int)2.412e8 &&
> -		     fwrq->m <= (int)2.487e8)) {
> +		if ((fwrq->m >= 2.412e8 &&
> +		     fwrq->m <= 2.487e8)) {

This turns integer conversions into floating point conversions. I don't
this floating point operations are allowed in the kernel.

>  			int f = fwrq->m / 100000;
>  			int c = 0;
>  
> @@ -571,9 +571,8 @@ int rtllib_wx_set_power(struct rtllib_device *ieee,
>  		ieee->ps = RTLLIB_PS_DISABLED;
>  		goto exit;
>  	}
> -	if (wrqu->power.flags & IW_POWER_TIMEOUT) {
> +	if (wrqu->power.flags & IW_POWER_TIMEOUT)
>  		ieee->ps_timeout = wrqu->power.value / 1000;
> -	}
>  
>  	if (wrqu->power.flags & IW_POWER_PERIOD)
>  		ieee->ps_period = wrqu->power.value / 1000;

Your patch does too many things at once. Please try to break it into
smallers patches that only do one (logical) change.

Best regards,
Nam