diff mbox series

wifi: mt76: mt7996: fix uninitialized variable in parsing txfree

Message ID e6c9a6fdeeaa57d73560c895046da4a58983297d.1697146767.git.yi-chia.hsieh@mediatek.com (mailing list archive)
State Accepted
Delegated to: Felix Fietkau
Headers show
Series wifi: mt76: mt7996: fix uninitialized variable in parsing txfree | expand

Commit Message

Yi-Chia Hsieh Oct. 12, 2023, 10 p.m. UTC
Fix the uninitialized variable warning in mt7996_mac_tx_free.

Fixes: 2461599f835e: "wifi: mt76: mt7996: get tx_retries and tx_failed from txfree"
Signed-off-by: Yi-Chia Hsieh <yi-chia.hsieh@mediatek.com>
---
 drivers/net/wireless/mediatek/mt76/mt7996/mac.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

James Dutton Oct. 15, 2023, 9:02 a.m. UTC | #1
On Thu, 12 Oct 2023 at 23:09, Yi-Chia Hsieh <yi-chia.hsieh@mediatek.com> wrote:
>
> Fix the uninitialized variable warning in mt7996_mac_tx_free.
>
> Fixes: 2461599f835e ("wifi: mt76: mt7996: get tx_retries and tx_failed from txfree")
> Signed-off-by: Yi-Chia Hsieh <yi-chia.hsieh@mediatek.com>
> ---
>  drivers/net/wireless/mediatek/mt76/mt7996/mac.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/mediatek/mt76/mt7996/mac.c b/drivers/net/wireless/mediatek/mt76/mt7996/mac.c
> index 04540833485f..59ab07b89087 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt7996/mac.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt7996/mac.c
> @@ -1074,7 +1074,7 @@ mt7996_mac_tx_free(struct mt7996_dev *dev, void *data, int len)
>         struct mt76_phy *phy3 = mdev->phys[MT_BAND2];
>         struct mt76_txwi_cache *txwi;
>         struct ieee80211_sta *sta = NULL;
> -       struct mt76_wcid *wcid;
> +       struct mt76_wcid *wcid = NULL;
>         LIST_HEAD(free_list);
>         struct sk_buff *skb, *tmp;
>         void *end = data + len;
> --
> 2.39.0
>

I am curious. Setting "struct mt76_wcid *wcid=NULL;" at the top of the
function will remove this warning, but is this really the intended
behaviour?
I am thinking about what situations will wcid now be non zero at this
(HERE below) position in the code.
It will be non-zero as a result of a previous round of the loop,
instead of given a value on this round of the loop.
There are no comments in the code, so I don't know if the intention is
to use the wcid from previous rounds of the loop or not.
My guess is we should raise the initialisation of wcid =
rcu_dereference(dev->mt76.wcid
[idx]);   to somewhere higher up, before the previous if...else scope.

  1127                 } else if (info & MT_TXFREE_INFO_HEADER) {
    1128                         u32 tx_retries = 0, tx_failed = 0;
    1129
--> 1130                         if (!wcid)        <--- HERE

Uninitialized on first iteration

    1131                                 continue;
    1132
    1133                         tx_retries =
    1134                                 FIELD_GET(MT_TXFREE_INFO_COUN
T, info) - 1;
    1135                         tx_failed = tx_retries +
    1136                                 !!FIELD_GET(MT_TXFREE_INFO_STAT, info);
    1137
    1138                         wcid->stats.tx_retries += tx_retries;
    1139                         wcid->stats.tx_failed += tx_failed;
    1140                         continue;
    1141                 }
diff mbox series

Patch

diff --git a/drivers/net/wireless/mediatek/mt76/mt7996/mac.c b/drivers/net/wireless/mediatek/mt76/mt7996/mac.c
index 04540833485f..59ab07b89087 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7996/mac.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7996/mac.c
@@ -1074,7 +1074,7 @@  mt7996_mac_tx_free(struct mt7996_dev *dev, void *data, int len)
 	struct mt76_phy *phy3 = mdev->phys[MT_BAND2];
 	struct mt76_txwi_cache *txwi;
 	struct ieee80211_sta *sta = NULL;
-	struct mt76_wcid *wcid;
+	struct mt76_wcid *wcid = NULL;
 	LIST_HEAD(free_list);
 	struct sk_buff *skb, *tmp;
 	void *end = data + len;