On Thu, 2014-06-05 at 15:38 +0300, Jukka Rissanen wrote:
> User is able to change autoconnect value for a provisioned service.
> This user modification must be preserved across ConnMan restarts.
> This is done by loading the necessary minimal service settings
> when provisioning a service via .config file. We also do not load
> the service settings if we could provision the service so that
> the options in .config file are not overwritten.
> ---
> src/config.c | 59
> ++++++++++++++++++++++++++++++++++++-----------------------
> src/service.c | 10 +++++++---
> 2 files changed, 43 insertions(+), 26 deletions(-)
>
> diff --git a/src/config.c b/src/config.c
> index d7f98f0..a74409d 100644
> --- a/src/config.c
> +++ b/src/config.c
> @@ -1055,11 +1055,10 @@ static gboolean remove_virtual_config(gpointer
> user_data)
> return FALSE;
> }
>
> -static void provision_service(gpointer key, gpointer value,
> - gpointer user_data)
> +static int try_provision_service(gpointer key,
> + struct connman_config_service *config,
> + struct connman_service *service)
> {
> - struct connman_service *service = user_data;
> - struct connman_config_service *config = value;
> struct connman_network *network;
> const void *service_id;
> enum connman_service_type type;
> @@ -1069,15 +1068,15 @@ static void provision_service(gpointer key, gpointer
> value,
> type = connman_service_get_type(service);
> if (type == CONNMAN_SERVICE_TYPE_WIFI &&
> g_strcmp0(config->type, "wifi") != 0)
> - return;
> + return -ENOENT;
>
> if (type == CONNMAN_SERVICE_TYPE_ETHERNET &&
> g_strcmp0(config->type, "ethernet") != 0)
> - return;
> + return -ENOENT;
>
> if (type == CONNMAN_SERVICE_TYPE_GADGET &&
> g_strcmp0(config->type, "gadget") != 0)
> - return;
> + return -ENOENT;
>
> DBG("service %p ident %s", service,
> __connman_service_get_ident(service));
> @@ -1085,7 +1084,7 @@ static void provision_service(gpointer key, gpointer
> value,
> network = __connman_service_get_network(service);
> if (!network) {
> connman_error("Service has no network set");
> - return;
> + return -EINVAL;
> }
>
> DBG("network %p ident %s", network,
> @@ -1098,7 +1097,7 @@ static void provision_service(gpointer key, gpointer
> value,
> device = connman_network_get_device(network);
> if (!device) {
> connman_error("Network device is missing");
> - return;
> + return -ENODEV;
> }
>
> device_addr = connman_device_get_string(device, "Address");
> @@ -1106,7 +1105,7 @@ static void provision_service(gpointer key, gpointer
> value,
> DBG("wants %s has %s", config->mac, device_addr);
>
> if (g_ascii_strcasecmp(device_addr, config->mac) != 0)
> - return;
> + return -ENOENT;
> }
>
> if (g_strcmp0(config->type, "wifi") == 0 &&
> @@ -1115,14 +1114,14 @@ static void provision_service(gpointer key, gpointer
> value,
> &ssid_len);
> if (!ssid) {
> connman_error("Network SSID not set");
> - return;
> + return -EINVAL;
> }
>
> if (!config->ssid || ssid_len != config->ssid_len)
> - return;
> + return -ENOENT;
>
> if (memcmp(config->ssid, ssid, ssid_len) != 0)
> - return;
> + return -ENOENT;
> }
>
> if (!config->ipv6_address) {
> @@ -1140,12 +1139,12 @@ static void provision_service(gpointer key, gpointer
> value,
>
> if (config->ipv6_prefix_length == 0) {
> DBG("IPv6 prefix missing");
> - return;
> + return -EINVAL;
> }
>
> address = connman_ipaddress_alloc(AF_INET6);
> if (!address)
> - return;
> + return -ENOENT;
>
> connman_ipaddress_set_ipv6(address, config->ipv6_address,
> config->ipv6_prefix_length,
> @@ -1185,12 +1184,12 @@ static void provision_service(gpointer key, gpointer
> value,
>
> if (!config->ipv4_netmask) {
> DBG("IPv4 netmask missing");
> - return;
> + return -EINVAL;
> }
>
> address = connman_ipaddress_alloc(AF_INET);
> if (!address)
> - return;
> + return -ENOENT;
>
> connman_ipaddress_set_ipv4(address, config->ipv4_address,
> config->ipv4_netmask,
> @@ -1253,7 +1252,7 @@ static void provision_service(gpointer key, gpointer
> value,
>
> __connman_service_mark_dirty();
>
> - __connman_service_save(service);
> + __connman_service_load_minimal(service);
>
> if (config->virtual) {
> struct connect_virtual *virtual;
> @@ -1265,13 +1264,21 @@ static void provision_service(gpointer key, gpointer
> value,
> g_timeout_add(0, remove_virtual_config, virtual);
> } else
>
> __connman_service_auto_connect(CONNMAN_SERVICE_CONNECT_REASON_AUTO);
> +
> + return 0;
> +}
> +
> +static void provision_service(gpointer key, gpointer value, gpointer
> user_data)
> +{
> + try_provision_service(key, value, user_data);
> }
Do we need this function or can the other uses of this function also be
done with a while loop?
> int __connman_config_provision_service(struct connman_service *service)
> {
> enum connman_service_type type;
> - GHashTableIter iter;
> - gpointer value, key;
> + GHashTableIter iter, iter_service;
> + gpointer value, key, value_service, key_service;
> + int ret = -ENOENT;
>
> /* For now only WiFi, Gadget and Ethernet services are supported */
> type = connman_service_get_type(service);
> @@ -1288,11 +1295,17 @@ int __connman_config_provision_service(struct
> connman_service *service)
> while (g_hash_table_iter_next(&iter, &key, &value)) {
> struct connman_config *config = value;
>
> - g_hash_table_foreach(config->service_table,
> - provision_service, service);
> + g_hash_table_iter_init(&iter_service, config->service_table);
> + while (g_hash_table_iter_next(&iter_service, &key_service,
> + &value_service)) {
> + if (!(ret = try_provision_service(key_service,
> + value_service, service)))
> + goto out;
> + }
> }
>
> - return 0;
> +out:
> + return ret;
> }
>
> int __connman_config_provision_service_ident(struct connman_service *service,
> diff --git a/src/service.c b/src/service.c
> index 48b0609..c871234 100644
> --- a/src/service.c
> +++ b/src/service.c
> @@ -6241,9 +6241,13 @@ static int service_register(struct connman_service
> *service)
>
> DBG("path %s", service->path);
>
> - __connman_config_provision_service(service);
> -
> - service_load(service);
> + /*
> + * We only load the service data if we could not provision
> + * the service. This way the provisioned service is not overwritten
> + * when loading the service from the settings file.
> + */
The above comment should go to the commit messge, to me it is clear from
the code that the service loading part is done only if the service was
not provisioned.
> + if (__connman_config_provision_service(service) < 0)
> + service_load(service);
>
> g_dbus_register_interface(connection, service->path,
> CONNMAN_SERVICE_INTERFACE,
Cheers,
Patrik
_______________________________________________
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman
