Re: [PATCH 1/1 v3] cfg80211: wext: fix cfg80211's SIOCSIWAP function

["John W. Linville" <linville@tuxdriver.com>]

On Sat, Jan 30, 2010 at 09:35:45AM +0100, Johannes Berg wrote:
> 
> > @@ -779,10 +779,15 @@ int __cfg80211_connect(struct cfg80211_registered_device *rdev,
> >  			wdev->conn->auto_auth = false;
> >  		}
> >  
> > -		memcpy(wdev->ssid, connect->ssid, connect->ssid_len);
> > +		/* make sure it can connect if only bssid is provided */
> >  		wdev->ssid_len = connect->ssid_len;
> > -		wdev->conn->params.ssid = wdev->ssid;
> >  		wdev->conn->params.ssid_len = connect->ssid_len;
> > +		if (connect->ssid_len) {
> > +			memcpy(wdev->ssid, connect->ssid, connect->ssid_len);
> > +			wdev->conn->params.ssid = wdev->ssid;
> > +		}
> > +		else
> > +			wdev->conn->params.ssid = NULL;
> 
> I don't think I understand this. The wdev->conn->params are initialised
> from "connect", and now you're copying the data back again? That's not
> making sense to me.
> 
> Also, please say what exactly are you trying to fix.
> 
> Also, cfg80211_mgd_wext_siwap() certainly is _not_ expected to clear the
> SSID, since we want to connect to the previously set SSID. Same with
> cfg80211_mgd_wext_siwessid(), it should _not_ clear the BSSID.
> 
> Right now, it looks to me that your patch makes it impossible to specify
> both the SSID and the BSSID to connect to, which is definitely wrong.
> 
> John, you mentioned you had applied this, can you revert this patch for
> now?

Yeah, it's coming out...  Clearly I didn't understand what it was doing.

John
-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.