Hi Samuel
>
> 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.
The problem still exist because if race comdition in connman. The
problem we issue disconnect to supplican then once return we can start
new coneection, so when we receive G_SUPPLICANT_STATE_DISCONNECTED from
supplicant wifi->network now point to a new network we want to
associate with causing to disconnect with the new network we want
to connect to. This patch will defer connection of the new network
until supplicant totally disconnected.
> 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.
I see it happen a couple of times but I could not root cause it I guess
it could happen if we call network_disconnect twice, which could happen
if we disconnect a network while still associating.
>
>
> > @@ -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:
I will do that.
>
> 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;
>
I will resubmit the patch a gain.
Mohamed
_______________________________________________
connman mailing list
[email protected]
http://lists.connman.net/listinfo/connman
