On Wed, Dec 07, 2011 at 04:17:50PM +0100, Johannes Berg wrote:
> On Wed, 2011-12-07 at 20:41 +0530, Vasanthakumar Thiagarajan wrote:
>
> > + spin_lock_bh(&dev->bss_lock);
> > + memcpy(bssid, bss->bssid, ETH_ALEN);
> > + spin_unlock_bh(&dev->bss_lock);
>
> I don't think this is necessary.
Right, i don't see any race either.
>
> > nl80211_send_roamed(wiphy_to_dev(wdev->wiphy), wdev->netdev, bssid,
> > req_ie, req_ie_len, resp_ie, resp_ie_len,
> > GFP_KERNEL);
> > @@ -615,40 +612,65 @@ void __cfg80211_roamed(struct wireless_dev *wdev,
> > wdev->wext.prev_bssid_valid = true;
> > wireless_send_event(wdev->netdev, SIOCGIWAP, &wrqu, NULL);
> > #endif
> > +
> > + return;
> > +out:
> > + if (bss)
> > + cfg80211_put_bss(bss);
> > }
>
> Doesn't that leak the reference if you return? It'll also give you an
> smatch warning since the function assumes the "bss" pointer that was
> passed in is not NULL, no?
Oops, sorry, i'll fix it. I may need to run smatch as well.
>
> > +static void cfg80211_roamed_bss(struct net_device *dev,
> > + struct cfg80211_bss *bss, const u8 *req_ie,
> > + size_t req_ie_len, const u8 *resp_ie,
> > + size_t resp_ie_len, gfp_t gfp)
> > {
> > struct wireless_dev *wdev = dev->ieee80211_ptr;
> > struct cfg80211_registered_device *rdev = wiphy_to_dev(wdev->wiphy);
> > struct cfg80211_event *ev;
> > unsigned long flags;
> >
> > - CFG80211_DEV_WARN_ON(wdev->sme_state != CFG80211_SME_CONNECTED);
> >
> > ev = kzalloc(sizeof(*ev) + req_ie_len + resp_ie_len, gfp);
>
> Why remove that warning? Also maybe do something like
>
> if (WARN_ON(!bss))
> return;
>
> (before allocating memory)
These warnings are added in cfg80211_roamed().
Thanks!
Vasanth
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
