Pkshih <pkshih@xxxxxxxxxxx> writes: >> >> > I check the size of object files before/after this patch, and >> >> > the original one is smaller. >> >> > >> >> > text data bss dec hex filename >> >> > 16781 3392 1 20174 4ece core-0.o // original >> >> > 16819 3392 1 20212 4ef4 core-1.o // after this patch >> >> > >> >> > Do you think it is worth to apply this patch? >> >> >> >> I think that we should apply the patch. Even though the compiler _may_ >> >> reorder the code, it might choose not to do that. >> > >> > Understand. >> > >> > I have another way to fix this coverity warning, like: >> > >> > @@ -1617,7 +1617,7 @@ static bool rtw89_core_txq_agg_wait(struct rtw89_dev *rtwdev, >> > { >> > struct rtw89_txq *rtwtxq = (struct rtw89_txq *)txq->drv_priv; >> > struct ieee80211_sta *sta = txq->sta; >> > - struct rtw89_sta *rtwsta = (struct rtw89_sta *)sta->drv_priv; >> > + struct rtw89_sta *rtwsta = sta ? (struct rtw89_sta *)sta->drv_priv : NULL; >> > >> > if (!sta || rtwsta->max_agg_wait <= 0) >> > return false; >> > >> > Is this acceptable? >> > It has a little redundant checking of 'sta', but the code looks clean. >> >> I feel that Colin's fix is more readable, but this is just matter of >> taste. You can choose. > > I would like my version. > > There are three similar warnings reported by smatch, so I will fix them by > myself. Please drop this patch. Ok, dropped. > But, still thank Colin to point out this issue. Indeed, thanks Colin. A good way to thank is to add Reported-by to the commit log. -- https://patchwork.kernel.org/project/linux-wireless/list/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches