Hi Mohamed,
On Wed, Dec 08, 2010 at 04:37:13PM -0800, Mohamed Abbas wrote:
> Make sure disconnect is completed before start new connection
> otherwise we will disconnect the new network when we receive
> DISCONNECT signal from supplicant.
The patch makes sense to me, although I wish wpa_supplicant would somehow take
care of those issues.
Before applying the patch, I have a few comments:
> @@ -508,19 +522,37 @@ static void connect_callback(int result,
> GSupplicantInterface *interface,
> connman_error("%s", __func__);
> }
>
> +static int network_connect(struct connman_network *network);
> +
By moving disconnect_callback() between network_connect(0 and
network_disconnect(), you wouldn't need this forward declaration.
> static void disconnect_callback(int result, GSupplicantInterface *interface,
> void *user_data)
> {
> struct wifi_data *wifi = user_data;
>
> - if (result < 0) {
> - connman_error("%s", __func__);
> - return;
> + if (wifi->network != NULL) {
So here I'm wondering how we could end up getting a disconnect_callback() call
with wifi->network being NULL. With your new network_disconnect()
implementation, this shouldn't happen.
> @@ -619,8 +658,15 @@ static int network_disconnect(struct connman_network
> *network)
>
> connman_network_set_associating(network, FALSE);
>
> - return g_supplicant_interface_disconnect(wifi->interface,
> + if (wifi->disconnecting == TRUE)
> + return -EALREADY;
> +
> + err = g_supplicant_interface_disconnect(wifi->interface,
> disconnect_callback, wifi);
> + if (err >= 0)
> + wifi->disconnecting = TRUE;
> +
> + return err;
I'd do things slightly differently:
if (wifi->disconnecting == TRUE)
return -EALREADY;
wifi->disconnecting = TRUE;
err = g_supplicant_interface_disconnect(wifi->interface,
disconnect_callback, wifi);
if (err < 0)
wifi->disconnecting = FALSE;
return err;
Cheers,
Samuel.
--
Intel Open Source Technology Centre
http://oss.intel.com/
_______________________________________________
connman mailing list
[email protected]
http://lists.connman.net/listinfo/connman
