From: Johannes Berg <johannes@sipsolutions.net>
To: Dan Carpenter <dan.carpenter@oracle.com>
Cc: linux-wireless@vger.kernel.org
Subject: Re: mac80211: support P2P Device abstraction
Date: Tue, 04 Sep 2012 19:27:39 +0200 [thread overview]
Message-ID: <1346779659.3737.41.camel@jlt4.sipsolutions.net> (raw)
In-Reply-To: <20120904172057.GB19410@mwanda>
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
prev parent reply other threads:[~2012-09-04 17:27 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
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 message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1346779659.3737.41.camel@jlt4.sipsolutions.net \
--to=johannes@sipsolutions.net \
--cc=dan.carpenter@oracle.com \
--cc=linux-wireless@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox