diff mbox series

wifi: rtl8xxxu: 8188e: parse single one element of RA report for station mode

Message ID 20230216004654.4642-1-pkshih@realtek.com (mailing list archive)
State Accepted
Commit 398eb19415ebb01ccc8e5bb75ff04cb760b01e05
Delegated to: Kalle Valo
Headers show
Series wifi: rtl8xxxu: 8188e: parse single one element of RA report for station mode | expand

Commit Message

Ping-Ke Shih Feb. 16, 2023, 12:46 a.m. UTC
Intentionally parsing single one element of RA report by breaking loop
causes a smatch warning:
  drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188e.c:1678 rtl8188e_handle_ra_tx_report2() warn:
  ignoring unreachable code.

With existing comments, it intends to process single one element for
station mode, but it will parse more elements in AP mode if it's
implemented. Implement program logic according to existing comment to avoid
smatch warning, and also be usable for both AP and stations modes.

Compile test only.

Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <error27@gmail.com>
Link: https://lore.kernel.org/r/202302142135.LCqUTVGY-lkp@intel.com/
Cc: Bitterblue Smith <rtl8821cerfe2@gmail.com>
Signed-off-by: Ping-Ke Shih <pkshih@realtek.com>
---
 .../net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188e.c   | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

Comments

Bitterblue Smith Feb. 16, 2023, 2:12 p.m. UTC | #1
On 16/02/2023 02:46, Ping-Ke Shih wrote:
> Intentionally parsing single one element of RA report by breaking loop
> causes a smatch warning:
>   drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188e.c:1678 rtl8188e_handle_ra_tx_report2() warn:
>   ignoring unreachable code.
> 
> With existing comments, it intends to process single one element for
> station mode, but it will parse more elements in AP mode if it's
> implemented. Implement program logic according to existing comment to avoid
> smatch warning, and also be usable for both AP and stations modes.
> 
> Compile test only.
> 
> Reported-by: kernel test robot <lkp@intel.com>
> Reported-by: Dan Carpenter <error27@gmail.com>
> Link: https://lore.kernel.org/r/202302142135.LCqUTVGY-lkp@intel.com/
> Cc: Bitterblue Smith <rtl8821cerfe2@gmail.com>
> Signed-off-by: Ping-Ke Shih <pkshih@realtek.com>
> ---
>  .../net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188e.c   | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188e.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188e.c
> index a99ddb41cd244..f15b099899e5c 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188e.c
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188e.c
> @@ -1699,6 +1699,12 @@ void rtl8188e_handle_ra_tx_report2(struct rtl8xxxu_priv *priv, struct sk_buff *s
>  
>  	dev_dbg(dev, "%s: len: %d items: %d\n", __func__, tx_rpt_len, items);
>  
> +	/* We only use macid 0, so only the first item is relevant.
> +	 * AP mode will use more of them if it's ever implemented.
> +	 */
> +	if (!priv->vif || priv->vif->type == NL80211_IFTYPE_STATION)
> +		items = 1;
> +
>  	for (macid = 0; macid < items; macid++) {
>  		valid = false;
>  
> @@ -1741,12 +1747,6 @@ void rtl8188e_handle_ra_tx_report2(struct rtl8xxxu_priv *priv, struct sk_buff *s
>  			min_rpt_time = ra->rpt_time;
>  
>  		rpt += TX_RPT2_ITEM_SIZE;
> -
> -		/*
> -		 * We only use macid 0, so only the first item is relevant.
> -		 * AP mode will use more of them if it's ever implemented.
> -		 */
> -		break;
>  	}
>  
>  	if (min_rpt_time != ra->pre_min_rpt_time) {

Tested-by: Bitterblue Smith <rtl8821cerfe2@gmail.com>
Kalle Valo Feb. 22, 2023, 12:29 p.m. UTC | #2
Ping-Ke Shih <pkshih@realtek.com> wrote:

> Intentionally parsing single one element of RA report by breaking loop
> causes a smatch warning:
>   drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188e.c:1678 rtl8188e_handle_ra_tx_report2() warn:
>   ignoring unreachable code.
> 
> With existing comments, it intends to process single one element for
> station mode, but it will parse more elements in AP mode if it's
> implemented. Implement program logic according to existing comment to avoid
> smatch warning, and also be usable for both AP and stations modes.
> 
> Compile test only.
> 
> Reported-by: kernel test robot <lkp@intel.com>
> Reported-by: Dan Carpenter <error27@gmail.com>
> Link: https://lore.kernel.org/r/202302142135.LCqUTVGY-lkp@intel.com/
> Cc: Bitterblue Smith <rtl8821cerfe2@gmail.com>
> Signed-off-by: Ping-Ke Shih <pkshih@realtek.com>
> Tested-by: Bitterblue Smith <rtl8821cerfe2@gmail.com>

Patch applied to wireless-next.git, thanks.

398eb19415eb wifi: rtl8xxxu: 8188e: parse single one element of RA report for station mode
diff mbox series

Patch

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188e.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188e.c
index a99ddb41cd244..f15b099899e5c 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188e.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188e.c
@@ -1699,6 +1699,12 @@  void rtl8188e_handle_ra_tx_report2(struct rtl8xxxu_priv *priv, struct sk_buff *s
 
 	dev_dbg(dev, "%s: len: %d items: %d\n", __func__, tx_rpt_len, items);
 
+	/* We only use macid 0, so only the first item is relevant.
+	 * AP mode will use more of them if it's ever implemented.
+	 */
+	if (!priv->vif || priv->vif->type == NL80211_IFTYPE_STATION)
+		items = 1;
+
 	for (macid = 0; macid < items; macid++) {
 		valid = false;
 
@@ -1741,12 +1747,6 @@  void rtl8188e_handle_ra_tx_report2(struct rtl8xxxu_priv *priv, struct sk_buff *s
 			min_rpt_time = ra->rpt_time;
 
 		rpt += TX_RPT2_ITEM_SIZE;
-
-		/*
-		 * We only use macid 0, so only the first item is relevant.
-		 * AP mode will use more of them if it's ever implemented.
-		 */
-		break;
 	}
 
 	if (min_rpt_time != ra->pre_min_rpt_time) {