Hi Daniel,

After several tests, I can not reproduce this issue on v1.38. I presume it has been fixed after v1.37.


Best Regards

Emmanuel


De : Daniel Wagner <wagi@monom.org>
Envoyé : vendredi 4 décembre 2020 13:53
À : VAUTRIN Emmanuel (Canal Plus Prestataire) <Emmanuel.VAUTRIN@cpexterne.org>
Cc : connman@lists.01.org <connman@lists.01.org>
Objet : Re: [PATCH] Fix hang up when connecting a secure network with an empty passphrase
 
Hi Emmanuel,

On Wed, Dec 02, 2020 at 09:17:17AM -0000, Emmanuel Vautrin wrote:
> Hello everyone,
>
> Please find below the patch fixing this issue:

I need a commit message explaining what you are fixing. The 'why' is
important. Also I can't really understand what do. Maybe it gets more
clear if you describe where the problems are.

> diff --git a/src/service.c b/src/service.c
> index 8d236749..629db995 100644
> --- a/src/service.c
> +++ b/src/service.c
> @@ -4021,6 +4021,7 @@ static void disconnect_on_last_session(enum connman_service_type type)
>  static int active_sessions[MAX_CONNMAN_SERVICE_TYPES] = {};
>  static int always_connect[MAX_CONNMAN_SERVICE_TYPES] = {};
>  static int active_count = 0;
> +static bool passphrase_requested = false;

No, this can't be a global variable. You need to track it in the service
object I guess.


>  void __connman_service_set_active_session(bool enable, GSList *list)
>  {
> @@ -4187,6 +4188,9 @@ static bool auto_connect_service(GList *services,
>                        continue;
>                }

> +             if (passphrase_requested)
> +                             return true;

This looks very suspiciously. The auto_connect should not be involved in
anyway how a service is configured.

>                if (service->pending ||
>                                is_connecting(service->state) ||
>                                is_connected(service->state)) {
> @@ -5721,6 +5725,7 @@ static void request_input_cb(struct connman_service *service,
>                err = __connman_service_set_passphrase(service, passphrase);

>   done:
> +     passphrase_requested = false;
>        if (err >= 0) {
>                /* We forget any previous error. */
>                set_error(service, CONNMAN_SERVICE_ERROR_UNKNOWN);
> @@ -6664,6 +6669,9 @@ int __connman_service_connect(struct connman_service *service,
>                        if (service->hidden && err != -EINPROGRESS)
>                                service->pending = pending;

> +                     if (err == -EINPROGRESS)
> +                             passphrase_requested = true;

If this is needed I think it should go inside the connman agent code. I
rather not have yet another state variable inside service.

Thanks,
Daniel