[PATCH 1/2] P2P: improve deinit for go neg non-zero status

[Eduardo Abinader]

On Mon, May 12, 2014 at 11:18 AM, Jouni Malinen <j at w1.fi> wrote:
>
> On Sun, May 11, 2014 at 12:54:43PM -0400, Eduardo Abinader wrote:
> > Added cancel timeout and issuing formation failed, in order
> > to improve detection for group formation failure, upon
> > non-zero status.
>
> > diff --git a/wpa_supplicant/p2p_supplicant.c b/wpa_supplicant/p2p_supplicant.c
> > @@ -1590,6 +1590,8 @@ static void wpas_go_neg_completed(void *ctx, struct p2p_go_neg_results *res)
> >                              res->status);
> >               wpas_notify_p2p_go_neg_completed(wpa_s, res);
> >               wpas_p2p_remove_pending_group_interface(wpa_s);
> > +             eloop_cancel_timeout(wpas_p2p_long_listen_timeout, wpa_s, NULL);
> > +             wpas_p2p_group_formation_failed(wpa_s);
> >               return;
> >       }
>
> I can understand the wpas_p2p_group_formation_failed() call, but what is
> the purpose of canceling wpas_p2p_long_listen_timeout here? Actually,
> that same question would really apply for the earlier commit
> 5eae87a7d61b053fb315c667f97954d004c195a5 ('P2P: Fix GO failed interface
> init') as well.. That was the only place where that
> eloop_cancel_timeout() was introduced without clearing
> wpa_s->p2p_long_listen to zero.

You're right. We must ensure p2p_long_listen=0.

I think we could keep this cancel_timeout, in order to earlier free
the timeout queue and set this p2p_long_listen=0 in patch 2/2 (just
moving after the formation_failed). This would comply with
p2p_stop_find_oper and p2p_listen, that do the same.

Either way, keeping the timeout everything would be fine also.


>
> The p2p_long_listen and its timeout mechanism are used to stop long
> listen operation so that it does not trigger during group formation
> steps. Doing that in failure cases looks a bit odd. I think I'd rather
> remove that previously added eloop_cancel_timeout() and this one here as
> well, unless there is a good reason for these that I cannot currently
> think of. These changes would also make patch 2/2 less useful (and well,
> if it should be added, I'd say it should also do wpa_s->p2p_long_listen
> = 0).
>
> --
> Jouni Malinen                                            PGP id EFC895FA
> _______________________________________________
> HostAP mailing list
> HostAP at lists.shmoo.com
> http://lists.shmoo.com/mailman/listinfo/hostap