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 <[email protected]>
---
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
Dmitry Antipov <[email protected]> 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 <[email protected]>
> ---
> 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
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 <[email protected]>
---
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
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 <[email protected]>
> ---
> 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 <[email protected]>
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 <[email protected]>
Thanks
Stanislaw
Dmitry Antipov <[email protected]> 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 <[email protected]>
> Acked-by: Stanislaw Gruszka <[email protected]>
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/[email protected]/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches