Re: carl9170: improve unicast PS buffering

[Dan Carpenter <dan.carpenter@oracle.com>]

On Thu, Jul 19, 2012 at 02:29:41PM +0200, Christian Lamparter wrote:
> Hello Dan,
> 
> On Thu, Jul 19, 2012 at 1:36 PM, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> > Sorry this is so old.  I was going through some old Smatch warnings.
> >
> > This is a semi-automatic email about new static checker warnings.
> >
> > The patch caf1eae20668: "carl9170: improve unicast PS buffering" from
> > Apr 24, 2011, leads to the following Smatch complaint:
> >
> > drivers/net/wireless/ath/carl9170/tx.c:1488 carl9170_op_tx()
> >          error: we previously assumed 'sta' could be null (see line 1482)
> >
> > drivers/net/wireless/ath/carl9170/tx.c
> >   1481
> >   1482          if (sta) {
> >                    ^^^^^
> > New check.
> >
> >   1483                  struct carl9170_sta_info *stai = (void *) sta->drv_priv;
> >   1484                  atomic_inc(&stai->pending_frames);
> >   1485          }
> >   1486
> >   1487          if (info->flags & IEEE80211_TX_CTL_AMPDU) {
> >   1488                  run = carl9170_tx_ampdu_queue(ar, sta, skb);
> >                               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> > Old dereference of "sta" inside the call to carl9170_tx_ampdu_queue().
> >
> >   1489                  if (run)
> >   1490                          carl9170_tx_ampdu(ar);
> >
> > Probably we can remove the check?
> What check do you want to remove? The check in smatch
> which produces the warning/error. Or the "if (sta) {" in line
> 1482?
> 
> Or do you mean I should extend the check in 1487 to:
> if (sta) {
>      ...
>      if (info->flags & IEEE80211_TX_CTL_AMPDU) {
>      ....
>      }
> }

What I'm saying is that I don't if it's possible for "sta" to be
NULL at this pointer or not.

regards,
dan carpenter