Hi Daniel,

>>> No I am wondering why we should call connman_technology_tethering_notify() several times? Wouldn't it make sense to do it only once?
>>
>>
>> This function further than notify Tethering property has changed, it
>> also creates/configures the bridge which we need to be ready in
>> sta_remove_callback() because in this function we recreate the same
>> wpa_s interface but bridged. That is the reason this function is
>> called from the very beginning and needs to be called each time we
>> found there was an error to remove the bridge. I think what you mean
>> is why we just enable Tethering property once at the end of the whole
>> process. If so, you are right but then we would need to split this
>> function into two where one of them just enable Tethering property and
>> the other handle the bridge. However, the function that handle the
>> bridge should anyway be called each time we found there was an error.
>
>
> Let me show you why calling connnman_technology_tethering_notify()
> in the loop is not useful:
>
> int connman_technology_tethering_notify(struct connman_technology *technology,
>                                                         bool enabled)
> {
>         int err;
>
>         DBG("technology %p enabled %u", technology, enabled);
>
>         if (technology->tethering == enabled)
>                 return -EALREADY;
>
>         if (enabled) {
>                 err = __connman_tethering_set_enabled();
>                 if (err < 0)
>                         return err;
>         } else
>                 __connman_tethering_set_disabled();
>
>         technology->tethering = enabled;
>         tethering_changed(technology);
>
>         return 0;
> }
>
> After the first successful call with enabled=true it is a no op (return -EALREDAY). Hmm, even worse, since you will break out with
>
>         if (bridge_result < 0)
>                 break
>
> you will only enable the first interface. I suggest you call
> connman_technology_tethering_notify() before the while loop
> or after. Depending what is simpler to do (error handling).

Now I got your point. However, I do not know why you continue iterating in the loop after the first successful call, we only want to enable tethering on one interface (The first one that supports tethering). That's the reason loop return when everything went well: 

err = g_supplicant_interface_remove(interface,
sta_remove_callback,
info);
if (err >= 0) {
DBG("tethering wifi %p ifname %s", wifi, ifname);
return 0;
}

Therefore, connman_technology_tethering_notify() will get called only once. However, what you made me notice is that I have to remove the bridge in case g_supplicant_interface_remove() fails.

Is there something I am missing?

Regards,

Jose Blanquicet