[PATCH] wifi: rt2x00: remove useless code in rt2x00queue_create_tx_descriptor()

[PATCH] wifi: rt2x00: remove useless code in rt2x00queue_create_tx_descriptor()

[Dmitry Antipov]

In 'rt2x00queue_create_tx_descriptor()', there is no need to call
'ieee80211_get_rts_cts_rate()' while checking for RTS/CTS frame
since this function returns NULL or pointer to internal bitrate
table entry, and the return value is not actually used. This way,
'rate' becomes block-scoped later when determining the modulation.
Compile tested only.

Found by Linux Verification Center (linuxtesting.org) with SVACE.

Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
---
 drivers/net/wireless/ralink/rt2x00/rt2x00queue.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/ralink/rt2x00/rt2x00queue.c b/drivers/net/wireless/ralink/rt2x00/rt2x00queue.c
index 98df0aef8168..7f9955deb204 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2x00queue.c
+++ b/drivers/net/wireless/ralink/rt2x00/rt2x00queue.c
@@ -389,7 +389,6 @@ static void rt2x00queue_create_tx_descriptor(struct rt2x00_dev *rt2x00dev,
 	struct ieee80211_tx_info *tx_info = IEEE80211_SKB_CB(skb);
 	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
 	struct ieee80211_tx_rate *txrate = &tx_info->control.rates[0];
-	struct ieee80211_rate *rate;
 	const struct rt2x00_rate *hwrate = NULL;
 
 	memset(txdesc, 0, sizeof(*txdesc));
@@ -416,9 +415,6 @@ static void rt2x00queue_create_tx_descriptor(struct rt2x00_dev *rt2x00dev,
 			__set_bit(ENTRY_TXD_RTS_FRAME, &txdesc->flags);
 		else
 			__set_bit(ENTRY_TXD_CTS_FRAME, &txdesc->flags);
-		if (tx_info->control.rts_cts_rate_idx >= 0)
-			rate =
-			    ieee80211_get_rts_cts_rate(rt2x00dev->hw, tx_info);
 	}
 
 	/*
@@ -463,7 +459,8 @@ static void rt2x00queue_create_tx_descriptor(struct rt2x00_dev *rt2x00dev,
 	else if (txrate->flags & IEEE80211_TX_RC_MCS)
 		txdesc->rate_mode = RATE_MODE_HT_MIX;
 	else {
-		rate = ieee80211_get_tx_rate(rt2x00dev->hw, tx_info);
+		struct ieee80211_rate *rate =
+			ieee80211_get_tx_rate(rt2x00dev->hw, tx_info);
 		hwrate = rt2x00_get_rate(rate->hw_value);
 		if (hwrate->flags & DEV_RATE_OFDM)
 			txdesc->rate_mode = RATE_MODE_OFDM;
-- 
2.43.0

Re: [PATCH] wifi: rt2x00: remove useless code in rt2x00queue_create_tx_descriptor()

[Kalle Valo]

Dmitry Antipov <dmantipov@yandex.ru> writes:

> In 'rt2x00queue_create_tx_descriptor()', there is no need to call
> 'ieee80211_get_rts_cts_rate()' while checking for RTS/CTS frame
> since this function returns NULL or pointer to internal bitrate
> table entry, and the return value is not actually used. This way,
> 'rate' becomes block-scoped later when determining the modulation.
> Compile tested only.
>
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
>
> Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
> ---
>  drivers/net/wireless/ralink/rt2x00/rt2x00queue.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/wireless/ralink/rt2x00/rt2x00queue.c b/drivers/net/wireless/ralink/rt2x00/rt2x00queue.c
> index 98df0aef8168..7f9955deb204 100644
> --- a/drivers/net/wireless/ralink/rt2x00/rt2x00queue.c
> +++ b/drivers/net/wireless/ralink/rt2x00/rt2x00queue.c
> @@ -389,7 +389,6 @@ static void rt2x00queue_create_tx_descriptor(struct rt2x00_dev *rt2x00dev,
>  	struct ieee80211_tx_info *tx_info = IEEE80211_SKB_CB(skb);
>  	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
>  	struct ieee80211_tx_rate *txrate = &tx_info->control.rates[0];
> -	struct ieee80211_rate *rate;

I think this should be kept.

> @@ -463,7 +459,8 @@ static void rt2x00queue_create_tx_descriptor(struct rt2x00_dev *rt2x00dev,
>  	else if (txrate->flags & IEEE80211_TX_RC_MCS)
>  		txdesc->rate_mode = RATE_MODE_HT_MIX;
>  	else {
> -		rate = ieee80211_get_tx_rate(rt2x00dev->hw, tx_info);
> +		struct ieee80211_rate *rate =
> +			ieee80211_get_tx_rate(rt2x00dev->hw, tx_info);

As I find it bad practice to declarare variables within if/else blocks.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

[PATCH] [v2] wifi: rt2x00: remove useless code in rt2x00queue_create_tx_descriptor()

[Dmitry Antipov]

In 'rt2x00queue_create_tx_descriptor()', there is no need to call
'ieee80211_get_rts_cts_rate()' while checking for RTS/CTS frame
since this function returns NULL or pointer to internal bitrate
table entry, and the return value is not actually used. Compile
tested only.

Found by Linux Verification Center (linuxtesting.org) with SVACE.

Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
---
v2: avoid scoped locals (Kalle Valo)
---
 drivers/net/wireless/ralink/rt2x00/rt2x00queue.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/net/wireless/ralink/rt2x00/rt2x00queue.c b/drivers/net/wireless/ralink/rt2x00/rt2x00queue.c
index 98df0aef8168..013003777fee 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2x00queue.c
+++ b/drivers/net/wireless/ralink/rt2x00/rt2x00queue.c
@@ -416,9 +416,6 @@ static void rt2x00queue_create_tx_descriptor(struct rt2x00_dev *rt2x00dev,
 			__set_bit(ENTRY_TXD_RTS_FRAME, &txdesc->flags);
 		else
 			__set_bit(ENTRY_TXD_CTS_FRAME, &txdesc->flags);
-		if (tx_info->control.rts_cts_rate_idx >= 0)
-			rate =
-			    ieee80211_get_rts_cts_rate(rt2x00dev->hw, tx_info);
 	}
 
 	/*
-- 
2.43.0

Re: [PATCH] [v2] wifi: rt2x00: remove useless code in rt2x00queue_create_tx_descriptor()

[Stanislaw Gruszka]

On Wed, Dec 13, 2023 at 08:14:43AM +0300, Dmitry Antipov wrote:
> In 'rt2x00queue_create_tx_descriptor()', there is no need to call
> 'ieee80211_get_rts_cts_rate()' while checking for RTS/CTS frame
> since this function returns NULL or pointer to internal bitrate
> table entry, and the return value is not actually used. Compile
> tested only.
> 
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
> 
> Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
> ---
> v2: avoid scoped locals (Kalle Valo)
> ---
>  drivers/net/wireless/ralink/rt2x00/rt2x00queue.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/drivers/net/wireless/ralink/rt2x00/rt2x00queue.c b/drivers/net/wireless/ralink/rt2x00/rt2x00queue.c
> index 98df0aef8168..013003777fee 100644
> --- a/drivers/net/wireless/ralink/rt2x00/rt2x00queue.c
> +++ b/drivers/net/wireless/ralink/rt2x00/rt2x00queue.c
> @@ -416,9 +416,6 @@ static void rt2x00queue_create_tx_descriptor(struct rt2x00_dev *rt2x00dev,
>  			__set_bit(ENTRY_TXD_RTS_FRAME, &txdesc->flags);
>  		else
>  			__set_bit(ENTRY_TXD_CTS_FRAME, &txdesc->flags);
> -		if (tx_info->control.rts_cts_rate_idx >= 0)
> -			rate =
> -			    ieee80211_get_rts_cts_rate(rt2x00dev->hw, tx_info);
>  	}
So we do not choose rate for RTS/CTS. Maybe we should actually,
but the patch does not change the logic that exist here for 
more than 12 years, since

commit 55b585e29095ce64900b6192aadf399fa007161e
Author: Helmut Schaa <helmut.schaa@googlemail.com>
Date:   Thu Mar 3 19:43:49 2011 +0100

    rt2x00: Don't call ieee80211_get_tx_rate for MCS rates

I'm ok with the patch.

Acked-by: Stanislaw Gruszka <stf_xl@wp.pl>

Thanks
Stanislaw

Re: [PATCH] [v2] wifi: rt2x00: remove useless code in rt2x00queue_create_tx_descriptor()

[Kalle Valo]

Dmitry Antipov <dmantipov@yandex.ru> wrote:

> In 'rt2x00queue_create_tx_descriptor()', there is no need to call
> 'ieee80211_get_rts_cts_rate()' while checking for RTS/CTS frame
> since this function returns NULL or pointer to internal bitrate
> table entry, and the return value is not actually used. Compile
> tested only.
> 
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
> 
> Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
> Acked-by: Stanislaw Gruszka <stf_xl@wp.pl>

Patch applied to wireless-next.git, thanks.

5a1745807580 wifi: rt2x00: remove useless code in rt2x00queue_create_tx_descriptor()

-- 
https://patchwork.kernel.org/project/linux-wireless/patch/20231213051449.126963-1-dmantipov@yandex.ru/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches