Hi Marcel, On 19.04.2012 13:10, Marcel Holtmann wrote: >>>>>> This fixes a small regression with the AutoConnect changes on ethernet. >>>>>> If the user has used ConnMan in the past and updates to the recent >>>>>> version >>>>>> the settings files wont contain the AutoConnect key value pair. Then >>>>>> we do not enable the auto connect. >>>>>> --- >>>>>> src/service.c | 2 ++ >>>>>> 1 file changed, 2 insertions(+) >>>>>> >>>>>> diff --git a/src/service.c b/src/service.c >>>>>> index 8f97c09..ed64133 100644 >>>>>> --- a/src/service.c >>>>>> +++ b/src/service.c >>>>>> @@ -407,6 +407,8 @@ static int service_load(struct connman_service >>>>>> *service) >>>>>> service->identifier, "AutoConnect", >>>>>> &error); >>>>>> if (error == NULL) >>>>>> service->autoconnect = autoconnect; >>>>>> + else >>>>>> + service->autoconnect = TRUE; >>>>> >>>>> shouldn't this be using the default value or the settings from main.conf >>>>> depending on the technology? >>>> >>>> service_load() is called after the update from the main.conf. So it >>>> overwrites the main.conf settings. Sounds reasonable for me, that the >>>> per service settings have higher precedence then the main.conf ones. >>> >>> that is correct, but you are currently overwriting it with TRUE if it is >>> not found in the storage file. >>> >>> Are you actually sure that your fix is doing the right thing. If by >>> accident the storage has AutoConnect=false, we can do nothing to fix >>> this here. Bad luck. >> >> First, this patch should actually only be needed for those, who have >> used ConnMan in the past and upgrade now. When service_save() is called >> the AutoConnect key will be written. A missing key should only happen >> once, when ConnMan loads the setting file after the upgrade. In this >> case, the key is missing and we should get an error != NULL, right? In >> all other cases the key will be present. At least that is what I think >> will happen. Not 100% sure. If we really want to be on the safe side, we >> just document it as known upgrade issue. What do you think? > > then your patch is wrong. Since if it does not find the key, it sets it > to true and ignores the main.conf setting. So I think the original code > was correct and by accident it was set to false from a previous version > and we just happen to ignore it before. > > That said, there is no real fix we can do here. So lets just not touch > it and help people to upgrade.
Yes, let's keep it as it is. This should really only happen the first time you start ConnMan after an upgrade. cheers, daniel _______________________________________________ connman mailing list [email protected] http://lists.connman.net/listinfo/connman
