> -----Original Message----- > From: Patrik Flykt [mailto:[email protected]] > Sent: Tuesday, January 21, 2014 8:27 PM > To: Zhang, Zhengguang > Cc: [email protected] > Subject: Re: [PATCH] ipconfig: Fix dhcp last address is cleared unreasonably > > > Hi, > > On Mon, 2014-01-13 at 14:51 +0800, [email protected] wrote: > > From: Zhang zhengguang <[email protected]> > > > > In __connman_ipconfig_save(), it will delete last address from the > > config file if it can't get it, which will lead to dhcp last address > > will be always NULL after a service is provisioned, and dhcp last > > address can't be passed to dhcp client. > > If DHCP has been successfully used, there will always be a last known address. > > > > > Signed-off-by: Zhang zhengguang <[email protected]> > > We haven't had the habit of doing Signed-off-bys in ConnMan. Got it, thanks! > > > --- > > src/ipconfig.c | 4 ---- > > 1 file changed, 4 deletions(-) > > > > diff --git a/src/ipconfig.c b/src/ipconfig.c index 9452125..b3232ab > > 100644 > > --- a/src/ipconfig.c > > +++ b/src/ipconfig.c > > @@ -2305,8 +2305,6 @@ int __connman_ipconfig_save(struct > connman_ipconfig *ipconfig, > > strlen(ipconfig->last_dhcp_address) > 0) > > g_key_file_set_string(keyfile, identifier, key, > > ipconfig->last_dhcp_address); > > - else > > - g_key_file_remove_key(keyfile, identifier, key, NULL); > > g_free(key); > > > > key = g_strdup_printf("%sDHCP.LastPrefixes", prefix); @@ -2333,8 > > +2331,6 @@ int __connman_ipconfig_save(struct connman_ipconfig > *ipconfig, > > strlen(ipconfig->last_dhcp_address) > 0) > > g_key_file_set_string(keyfile, identifier, key, > > ipconfig->last_dhcp_address); > > - else > > - g_key_file_remove_key(keyfile, identifier, key, NULL); > > g_free(key); > > /* fall through */ > > case CONNMAN_IPCONFIG_METHOD_UNKNOWN: > > This patch doesn't seem to do as advertised. As the last DHCP address should > always be available, its not possible that it is NULL, at least not with the > explanation given in the commit message. > > There is actually a bug in service_register() which causes a service always > to be > loaded from disk, also if the service is provisioned. The proper thing to do > is to > load and save _only_ DHCP last address from/to disk if the service is > immutable. > The last used time should also be loaded/saved from/to disk. > In the current ConnMan implementation, in service_register(), last dhcp address will be cleared when the service is provisioned(in __connman_ipconfig_save), which leads to last dhcp address will be always NULL when passing it to dhcp client, I agree with you that when service is provisioned, it should not be loaded from disk, I will investigate it when I have time, so just drop this patch at this moment.
> Cheers, > > Patrik Regards Zhang Zhengguang _______________________________________________ connman mailing list [email protected] https://lists.connman.net/mailman/listinfo/connman
