Hi,

Thanks for these patches. I'm going to add text from the cover letter to
this patch and nitpick on clearing AutoConnect below.

On Thu, 2015-10-15 at 10:41 +0300, Jaakko Hannikainen wrote:
> The previous implementation only cleared Error, which as per
> documentation is wrong since Error is readonly. Add sane
> defaults to all readwrite fields.
> 
> Reported by Francesco Bruno.
> ---
>  src/service.c | 31 ++++++++++++++++++++++++++-----
>  1 file changed, 26 insertions(+), 5 deletions(-)
> 
> diff --git a/src/service.c b/src/service.c
> index e9107c2..fe20763 100644
> --- a/src/service.c
> +++ b/src/service.c
> @@ -3582,11 +3582,32 @@ static DBusMessage *clear_property(DBusConnection 
> *conn,
>       dbus_message_get_args(msg, NULL, DBUS_TYPE_STRING, &name,
>                                                       DBUS_TYPE_INVALID);
>  
> -     if (g_str_equal(name, "Error")) {
> -             set_error(service, CONNMAN_SERVICE_ERROR_UNKNOWN);
> -
> -             g_get_current_time(&service->modified);
> -             service_save(service);
> +     if (service->hidden) {
> +             return __connman_error_not_supported(msg);
> +     } else if (g_str_equal(name, "AutoConnect")) {
> +             __connman_service_set_autoconnect(service, true);

Here I'd say clearing means to unset the value 'true'. I.e. the result
is not that one gets a default value but the cleared one. Nitpicking on
this means that the default value of AutoConnect is really false, it's
only the DefaultAutoConnectTechnologies ones that are set to true.

I'll change this to avoid an extra round-trip.

Cheers,

        Patrik

> +     } else if (service->immutable) {
> +             return __connman_error_not_supported(msg);
> +     } else if (g_str_equal(name, "Nameservers.Configuration")) {
> +             __connman_service_set_nameservers_conf(service, NULL);
> +     } else if (g_str_equal(name, "Timeservers.Configuration")) {
> +             __connman_service_set_timeservers_conf(service, NULL);
> +     } else if (g_str_equal(name, "Domains.Configuration")) {
> +             __connman_service_set_domains_conf(service, NULL);
> +     } else if (g_str_equal(name, "Proxy.Configuration")) {
> +             __connman_service_set_proxy_conf(service,
> +                                     CONNMAN_SERVICE_PROXY_METHOD_AUTO,
> +                                     NULL, NULL, NULL);
> +     } else if (g_str_equal(name, "IPv4.Configuration")) {
> +             __connman_service_reset_ipconfig(service,
> +                             CONNMAN_IPCONFIG_TYPE_IPV4, NULL, NULL);
> +             __connman_ipconfig_set_method(service->ipconfig_ipv4,
> +                                             CONNMAN_IPCONFIG_METHOD_DHCP);
> +             __connman_network_enable_ipconfig(service->network,
> +                             service->ipconfig_ipv4);
> +     } else if (g_str_equal(name, "IPv6.Configuration")) {
> +             __connman_service_reset_ipconfig(service,
> +                             CONNMAN_IPCONFIG_TYPE_IPV6, NULL, NULL);
>       } else
>               return __connman_error_invalid_property(msg);
>  


_______________________________________________
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

Reply via email to