Dmitry Antipov <dmantipov@xxxxxxxxx> writes: > Since 'skb', 'tx_info' and 'fi' are actually used within > buffers processing loop only, move them inside the latter > and avoid some redundant initialization at the beginning > of 'ath_tx_complete_aggr()'. Compile tested only. > > Found by Linux Verification Center (linuxtesting.org) with SVACE. > > Signed-off-by: Dmitry Antipov <dmantipov@xxxxxxxxx> > --- > drivers/net/wireless/ath/ath9k/xmit.c | 13 +++---------- > 1 file changed, 3 insertions(+), 10 deletions(-) > > diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c > index d519b676a109..657862be45e5 100644 > --- a/drivers/net/wireless/ath/ath9k/xmit.c > +++ b/drivers/net/wireless/ath/ath9k/xmit.c > @@ -491,8 +491,6 @@ static void ath_tx_complete_aggr(struct ath_softc *sc, struct ath_txq *txq, > struct ath_tx_status *ts, int txok) > { > struct ath_node *an = NULL; > - struct sk_buff *skb; > - struct ieee80211_tx_info *tx_info; > struct ath_buf *bf_next, *bf_last = bf->bf_lastbf; > struct list_head bf_head; > struct sk_buff_head bf_pending; > @@ -501,15 +499,11 @@ static void ath_tx_complete_aggr(struct ath_softc *sc, struct ath_txq *txq, > int isaggr, txfail, txpending, sendbar = 0, needreset = 0, nbad = 0; > bool rc_update = true, isba; > struct ieee80211_tx_rate rates[4]; > - struct ath_frame_info *fi; > int nframes; > bool flush = !!(ts->ts_status & ATH9K_TX_FLUSH); > int i, retries; > int bar_index = -1; > > - skb = bf->bf_mpdu; > - tx_info = IEEE80211_SKB_CB(skb); > - > memcpy(rates, bf->rates, sizeof(rates)); > > retries = ts->ts_longretry + 1; > @@ -571,14 +565,13 @@ static void ath_tx_complete_aggr(struct ath_softc *sc, struct ath_txq *txq, > ath_tx_count_frames(sc, bf, ts, txok, &nframes, &nbad); > while (bf) { > u16 seqno = bf->bf_state.seqno; > + struct sk_buff *skb = bf->bf_mpdu; > + struct ath_frame_info *fi = get_frame_info(skb); > + struct ieee80211_tx_info *tx_info = IEEE80211_SKB_CB(skb); > > txfail = txpending = sendbar = 0; > bf_next = bf->bf_next; > > - skb = bf->bf_mpdu; > - tx_info = IEEE80211_SKB_CB(skb); > - fi = get_frame_info(skb); > - I'm OK with moving the variable declarations inside the loop body, but please keep the assignment here as separate statements (and maintain reverse x-mas tree ordering in the declarations at the top of the loop). -Toke