>>> 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?
> Nah, I didn't read it correctly when giving a quick glance. I suggest to move the whole functionality into a function and just call it from the loop. If it returns success leave the loop otherwise continue looping. This is just a bit too easy to get it wrong as it stands now.

You are right. It has become a complex code even before this patch. To do what you say will make it more clear/readable but unfortunately I am currently quite busy to modify and retest all again.

On the other hand, I found that it is required to initialize the variable berr (previously bridge_error) because it could be evaluated without being assigned in case of one of these two actions fail:

info->ssid = ssid_ap_init(identifier, passphrase);
if (!info->ssid)
goto failed;

or ...

wifi->tethering_param->ssid = ssid_ap_init(identifier, passphrase);
if (!wifi->tethering_param->ssid)
goto failed;

Please check the PATCH v2. Do you agree?


Jose Blanquicet