| Message ID | 20111129010938.E3CF93835E@ushik.mtv.corp.google.com |
|---|---|
| State | Accepted |
| Headers | show |
On Mon, Nov 28, 2011 at 05:07:26PM -0800, Dmitry Shmidt wrote: > diff --git a/wpa_supplicant/ctrl_iface.c b/wpa_supplicant/ctrl_iface.c > i = 0; > + ssid = wpa_s->conf->ssid; > while (ssid) { Thanks! Applied this part. > @@ -3377,10 +3378,12 @@ static int wpa_supplicant_driver_cmd(struct wpa_supplicant *wpa_s, char *cmd, > > #ifdef ANDROID > if (os_strcasecmp(cmd, "BGSCAN-START") == 0) > - return pno_start(wpa_s); > + ret = pno_start(wpa_s); > + > + else if (os_strcasecmp(cmd, "BGSCAN-STOP") == 0) > + ret = pno_stop(wpa_s); > > - if (os_strcasecmp(cmd, "BGSCAN-STOP") == 0) > - return pno_stop(wpa_s); > + else > #endif /* ANDROID */ > > ret = wpa_drv_driver_cmd(wpa_s, cmd, buf, buflen); This does not look correct. The reason for the special handlers for BGSCAN-START/STOP here are for the explicit purpose of avoiding that wpa_drv_driver_cmd() call and as such, those returns are there for purpose. pno_start/stop end up calling wpa_drv_sched_scan/wpa_drv_stop_sched_scan and the driver wrappers can use those to implement whatever is needed for this. In the android branch, driver_wext.c converts this to what driver_cmd() does in wpa_supplicant_8.git (and the private wpa_supplicant_8_lib) and driver_nl80211.c uses nl80211 sched_scan commands if the driver supports them and if not, does what wpa_supplicant_8.git tree is doing with the private ioctl.
On Tue, Nov 29, 2011 at 3:03 AM, Jouni Malinen <j@w1.fi> wrote: > On Mon, Nov 28, 2011 at 05:07:26PM -0800, Dmitry Shmidt wrote: >> diff --git a/wpa_supplicant/ctrl_iface.c b/wpa_supplicant/ctrl_iface.c >> i = 0; >> + ssid = wpa_s->conf->ssid; >> while (ssid) { > > Thanks! Applied this part. > >> @@ -3377,10 +3378,12 @@ static int wpa_supplicant_driver_cmd(struct wpa_supplicant *wpa_s, char *cmd, >> >> #ifdef ANDROID >> if (os_strcasecmp(cmd, "BGSCAN-START") == 0) >> - return pno_start(wpa_s); >> + ret = pno_start(wpa_s); >> + >> + else if (os_strcasecmp(cmd, "BGSCAN-STOP") == 0) >> + ret = pno_stop(wpa_s); >> >> - if (os_strcasecmp(cmd, "BGSCAN-STOP") == 0) >> - return pno_stop(wpa_s); >> + else >> #endif /* ANDROID */ >> >> ret = wpa_drv_driver_cmd(wpa_s, cmd, buf, buflen); > > This does not look correct. The reason for the special handlers for > BGSCAN-START/STOP here are for the explicit purpose of avoiding that > wpa_drv_driver_cmd() call and as such, those returns are there for > purpose. pno_start/stop end up calling > wpa_drv_sched_scan/wpa_drv_stop_sched_scan and the driver wrappers can > use those to implement whatever is needed for this. In the android > branch, driver_wext.c converts this to what driver_cmd() does in > wpa_supplicant_8.git (and the private wpa_supplicant_8_lib) and > driver_nl80211.c uses nl80211 sched_scan commands if the driver supports > them and if not, does what wpa_supplicant_8.git tree is doing with the > private ioctl. The point here to return proper status in the return buffer. ("Ok" in this case or "Fail"). Thanks, Dmitry > > -- > Jouni Malinen PGP id EFC895FA > _______________________________________________ > HostAP mailing list > HostAP@lists.shmoo.com > http://lists.shmoo.com/mailman/listinfo/hostap >
On Tue, Nov 29, 2011 at 10:26:27AM -0800, Dmitry Shmidt wrote: > On Tue, Nov 29, 2011 at 3:03 AM, Jouni Malinen <j@w1.fi> wrote: > > On Mon, Nov 28, 2011 at 05:07:26PM -0800, Dmitry Shmidt wrote: > >> @@ -3377,10 +3378,12 @@ static int wpa_supplicant_driver_cmd(struct wpa_supplicant *wpa_s, char *cmd, > >> > >> #ifdef ANDROID > >> if (os_strcasecmp(cmd, "BGSCAN-START") == 0) > >> - return pno_start(wpa_s); > >> + ret = pno_start(wpa_s); > >> + > >> + else if (os_strcasecmp(cmd, "BGSCAN-STOP") == 0) > >> + ret = pno_stop(wpa_s); > >> > >> - if (os_strcasecmp(cmd, "BGSCAN-STOP") == 0) > >> - return pno_stop(wpa_s); > >> + else > >> #endif /* ANDROID */ > >> > >> ret = wpa_drv_driver_cmd(wpa_s, cmd, buf, buflen); > The point here to return proper status in the return buffer. ("Ok" in > this case or "Fail"). Ah. Sorry, obviously I cannot read anymore.. ;-) I missed the last "else" there going across #endif and the context in the patch ended just before the key functionality. I applied this now with the ifdef block removed so that this is easier to read.
diff --git a/wpa_supplicant/ctrl_iface.c b/wpa_supplicant/ctrl_iface.c index 8a6d00c..38538c9 100644 --- a/wpa_supplicant/ctrl_iface.c +++ b/wpa_supplicant/ctrl_iface.c @@ -85,6 +85,7 @@ static int pno_start(struct wpa_supplicant *wpa_s) if (params.filter_ssids == NULL) return -1; i = 0; + ssid = wpa_s->conf->ssid; while (ssid) { if (!ssid->disabled) { params.ssids[i].ssid = ssid->ssid; @@ -3377,10 +3378,12 @@ static int wpa_supplicant_driver_cmd(struct wpa_supplicant *wpa_s, char *cmd, #ifdef ANDROID if (os_strcasecmp(cmd, "BGSCAN-START") == 0) - return pno_start(wpa_s); + ret = pno_start(wpa_s); + + else if (os_strcasecmp(cmd, "BGSCAN-STOP") == 0) + ret = pno_stop(wpa_s); - if (os_strcasecmp(cmd, "BGSCAN-STOP") == 0) - return pno_stop(wpa_s); + else #endif /* ANDROID */ ret = wpa_drv_driver_cmd(wpa_s, cmd, buf, buflen);
Signed-off-by: Dmitry Shmidt <dimitrysh@google.com> --- wpa_supplicant/ctrl_iface.c | 9 ++++++--- 1 files changed, 6 insertions(+), 3 deletions(-)