* re: mac80211: support P2P Device abstraction
@ 2012-09-04 16:13 Dan Carpenter
2012-09-04 16:18 ` Johannes Berg
0 siblings, 1 reply; 4+ messages in thread
From: Dan Carpenter @ 2012-09-04 16:13 UTC (permalink / raw)
To: johannes.berg; +Cc: linux-wireless
Hello Johannes Berg,
This is a semi-automatic email about new static checker warnings.
The patch f142c6b906da: "mac80211: support P2P Device abstraction"
from Jun 18, 2012, leads to the following Smatch complaint:
net/mac80211/iface.c:1168 ieee80211_setup_sdata()
error: we previously assumed 'sdata->dev' could be null (see line 1134)
net/mac80211/iface.c
1133 /* only monitor/p2p-device differ */
1134 if (sdata->dev) {
^^^^^^^^^^
New test.
1135 sdata->dev->netdev_ops = &ieee80211_dataif_ops;
1136 sdata->dev->type = ARPHRD_ETHER;
1137 }
[snip]
1167 case NL80211_IFTYPE_MONITOR:
1168 sdata->dev->type = ARPHRD_IEEE80211_RADIOTAP;
^^^^^^^^^^^^
Old dereference.
1169 sdata->dev->netdev_ops = &ieee80211_monitorif_ops;
1170 sdata->u.mntr_flags = MONITOR_FLAG_CONTROL |
regards,
dan carpenter
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: mac80211: support P2P Device abstraction
2012-09-04 16:13 mac80211: support P2P Device abstraction Dan Carpenter
@ 2012-09-04 16:18 ` Johannes Berg
2012-09-04 17:20 ` Dan Carpenter
0 siblings, 1 reply; 4+ messages in thread
From: Johannes Berg @ 2012-09-04 16:18 UTC (permalink / raw)
To: Dan Carpenter; +Cc: linux-wireless
Hi Dan,
> The patch f142c6b906da: "mac80211: support P2P Device abstraction"
> from Jun 18, 2012, leads to the following Smatch complaint:
>
> net/mac80211/iface.c:1168 ieee80211_setup_sdata()
> error: we previously assumed 'sdata->dev' could be null (see line 1134)
>
> net/mac80211/iface.c
> 1133 /* only monitor/p2p-device differ */
> 1134 if (sdata->dev) {
> ^^^^^^^^^^
> New test.
>
> 1135 sdata->dev->netdev_ops = &ieee80211_dataif_ops;
> 1136 sdata->dev->type = ARPHRD_ETHER;
> 1137 }
>
> [snip]
>
> 1167 case NL80211_IFTYPE_MONITOR:
> 1168 sdata->dev->type = ARPHRD_IEEE80211_RADIOTAP;
> ^^^^^^^^^^^^
> Old dereference.
Thanks. I'm aware of the warning (I run smatch), but sdata->dev can only
be NULL in "case NL80211_IFTYPE_P2P_DEVICE", so adding an extra check in
the monitor case didn't seem worthwhile.
It might have been better to use sdata->vif.type != P2P_DEVICE in the
new test I guess, but then it wouldn't be extensible to new interface
types like Bluetooth AMP.
johannes
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: mac80211: support P2P Device abstraction
2012-09-04 16:18 ` Johannes Berg
@ 2012-09-04 17:20 ` Dan Carpenter
2012-09-04 17:27 ` Johannes Berg
0 siblings, 1 reply; 4+ messages in thread
From: Dan Carpenter @ 2012-09-04 17:20 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless
On Tue, Sep 04, 2012 at 06:18:45PM +0200, Johannes Berg wrote:
> Hi Dan,
>
> > The patch f142c6b906da: "mac80211: support P2P Device abstraction"
> > from Jun 18, 2012, leads to the following Smatch complaint:
> >
> > net/mac80211/iface.c:1168 ieee80211_setup_sdata()
> > error: we previously assumed 'sdata->dev' could be null (see line 1134)
> >
> > net/mac80211/iface.c
> > 1133 /* only monitor/p2p-device differ */
> > 1134 if (sdata->dev) {
> > ^^^^^^^^^^
> > New test.
> >
> > 1135 sdata->dev->netdev_ops = &ieee80211_dataif_ops;
> > 1136 sdata->dev->type = ARPHRD_ETHER;
> > 1137 }
> >
> > [snip]
> >
> > 1167 case NL80211_IFTYPE_MONITOR:
> > 1168 sdata->dev->type = ARPHRD_IEEE80211_RADIOTAP;
> > ^^^^^^^^^^^^
> > Old dereference.
>
> Thanks. I'm aware of the warning (I run smatch), but sdata->dev can only
> be NULL in "case NL80211_IFTYPE_P2P_DEVICE", so adding an extra check in
> the monitor case didn't seem worthwhile.
>
> It might have been better to use sdata->vif.type != P2P_DEVICE in the
> new test I guess, but then it wouldn't be extensible to new interface
> types like Bluetooth AMP.
Gar. This was my bad. I didn't realize the warning was so old. I
try to only send the automatic emails for new warnings. I have been
meaning to fix the bug in my script for a while now but this time
I will really do that.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: mac80211: support P2P Device abstraction
2012-09-04 17:20 ` Dan Carpenter
@ 2012-09-04 17:27 ` Johannes Berg
0 siblings, 0 replies; 4+ messages in thread
From: Johannes Berg @ 2012-09-04 17:27 UTC (permalink / raw)
To: Dan Carpenter; +Cc: linux-wireless
Hi Dan,
> > > The patch f142c6b906da: "mac80211: support P2P Device abstraction"
> > > from Jun 18, 2012, leads to the following Smatch complaint:
> > >
> > > net/mac80211/iface.c:1168 ieee80211_setup_sdata()
> > > error: we previously assumed 'sdata->dev' could be null (see line 1134)
> > >
> > > net/mac80211/iface.c
> > > 1133 /* only monitor/p2p-device differ */
> > > 1134 if (sdata->dev) {
> > > ^^^^^^^^^^
> > > New test.
> > >
> > > 1135 sdata->dev->netdev_ops = &ieee80211_dataif_ops;
> > > 1136 sdata->dev->type = ARPHRD_ETHER;
> > > 1137 }
> > >
> > > [snip]
> > >
> > > 1167 case NL80211_IFTYPE_MONITOR:
> > > 1168 sdata->dev->type = ARPHRD_IEEE80211_RADIOTAP;
> > > ^^^^^^^^^^^^
> > > Old dereference.
> >
> > Thanks. I'm aware of the warning (I run smatch), but sdata->dev can only
> > be NULL in "case NL80211_IFTYPE_P2P_DEVICE", so adding an extra check in
> > the monitor case didn't seem worthwhile.
> >
> > It might have been better to use sdata->vif.type != P2P_DEVICE in the
> > new test I guess, but then it wouldn't be extensible to new interface
> > types like Bluetooth AMP.
>
> Gar. This was my bad. I didn't realize the warning was so old. I
> try to only send the automatic emails for new warnings. I have been
> meaning to fix the bug in my script for a while now but this time
> I will really do that.
No no, the warning was introduced in that patch on June 18th, so in that
sense the email was correct. Before that patch, the test didn't exist,
so the warning didn't show up, because the function *always* assumed
sdata->dev was set. Now there's one case where this isn't the case any
more, for vif.type == NL80211_IFTYPE_P2P_DEVICE, but the monitor case
can still assume sdata->dev is set...
It's really just a false positive since sdata->dev and sdata->vif.type
are correlated, but smatch can't know that.
johannes
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2012-09-04 17:27 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-09-04 16:13 mac80211: support P2P Device abstraction Dan Carpenter
2012-09-04 16:18 ` Johannes Berg
2012-09-04 17:20 ` Dan Carpenter
2012-09-04 17:27 ` Johannes Berg
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox