* re: carl9170: improve unicast PS buffering
@ 2012-07-19 11:36 Dan Carpenter
2012-07-19 12:29 ` Christian Lamparter
0 siblings, 1 reply; 4+ messages in thread
From: Dan Carpenter @ 2012-07-19 11:36 UTC (permalink / raw)
To: chunkeey; +Cc: linux-wireless
Hello Christian Lamparter,
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?
regards,
dan carpenter
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: carl9170: improve unicast PS buffering
2012-07-19 11:36 carl9170: improve unicast PS buffering Dan Carpenter
@ 2012-07-19 12:29 ` Christian Lamparter
2012-07-20 7:23 ` Dan Carpenter
0 siblings, 1 reply; 4+ messages in thread
From: Christian Lamparter @ 2012-07-19 12:29 UTC (permalink / raw)
To: Dan Carpenter; +Cc: linux-wireless
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) {
....
}
}
Regards,
Chr
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: carl9170: improve unicast PS buffering
2012-07-19 12:29 ` Christian Lamparter
@ 2012-07-20 7:23 ` Dan Carpenter
2012-07-20 19:01 ` Christian Lamparter
0 siblings, 1 reply; 4+ messages in thread
From: Dan Carpenter @ 2012-07-20 7:23 UTC (permalink / raw)
To: Christian Lamparter; +Cc: linux-wireless
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
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: carl9170: improve unicast PS buffering
2012-07-20 7:23 ` Dan Carpenter
@ 2012-07-20 19:01 ` Christian Lamparter
0 siblings, 0 replies; 4+ messages in thread
From: Christian Lamparter @ 2012-07-20 19:01 UTC (permalink / raw)
To: Dan Carpenter; +Cc: linux-wireless
On Friday 20 July 2012 09:23:15 Dan Carpenter wrote:
> 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 ?know? if it's possible
> for "sta" to be NULL at this pointer or not.
no, it is not possible.
In fact, in order to set up a ampdu session, the stack would
call the driver's op_ampdu_action callback which always needs
a station.
Regards,
Chr
(So, where do we go from here - do you still want a patch?)
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2012-07-20 19:01 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-07-19 11:36 carl9170: improve unicast PS buffering Dan Carpenter
2012-07-19 12:29 ` Christian Lamparter
2012-07-20 7:23 ` Dan Carpenter
2012-07-20 19:01 ` Christian Lamparter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).