Hi,

On Wed, 2012-10-17 at 14:42 +0200, Daniel Wagner wrote:
> From: Daniel Wagner <daniel.wag...@bmw-carit.de>
> 
> Give the policy plugin a way to inform the session core that
> some of the config values have changed.
> 
> This could be done in a more clever way, e.g. figure out only
> to update the necessary info entries.
> 
> It is not expected that there are many updates so let's keep it
> simple for the time beeing.
> ---
>  include/session.h |  2 ++
>  src/session.c     | 37 +++++++++++++++++++++++++++++++++++++
>  2 files changed, 39 insertions(+)
> 
> diff --git a/include/session.h b/include/session.h
> index 9b6428f..086c0fc 100644
> --- a/include/session.h
> +++ b/include/session.h
> @@ -81,6 +81,8 @@ struct connman_session_policy {
>  int connman_session_policy_register(struct connman_session_policy *config);
>  void connman_session_policy_unregister(struct connman_session_policy 
> *config);
>  
> +void connman_session_config_update(struct connman_session *session);
> +
>  GSList *connman_session_allowed_bearers_any(void);
>  void connman_session_free_bearers(GSList *bearers);
>  
> diff --git a/src/session.c b/src/session.c
> index a5fa801..4778b76 100644
> --- a/src/session.c
> +++ b/src/session.c
> @@ -1403,6 +1403,43 @@ static void session_changed(struct connman_session 
> *session,
>       session_notify(session);
>  }
>  
> +void connman_session_config_update(struct connman_session *session)
> +{
> +     struct session_info *info = session->info;
> +     GSList *allowed_bearers;
> +     int err;
> +
> +     DBG("session %p", session);
> +
> +     /*
> +      * We update all configuration even though only one entry
> +      * might have changed. We can still optimize this later.
> +      */
> +
> +     err = apply_policy_on_bearers(
> +             session->policy_config->allowed_bearers,
> +             info->config.allowed_bearers,
> +             &allowed_bearers);
> +     if (err < 0)
> +             return;
> +     connman_session_free_bearers(info->config.allowed_bearers);
> +     info->config.allowed_bearers = allowed_bearers;
> +
> +     info->config.type = apply_policy_on_type(
> +                             session->policy_config->type,
> +                             info->config.type);
> +
> +     info->config.roaming_policy = session->policy_config->roaming_policy;
> +
> +     info->config.ecall = session->policy_config->ecall;
> +     if (info->config.ecall == TRUE)
> +             ecall_session = session;
> +
> +     info->config.priority = session->policy_config->priority;
> +
> +     session_changed(session, CONNMAN_SESSION_TRIGGER_SETTING);
> +}
> +
>  static DBusMessage *connect_session(DBusConnection *conn,
>                                       DBusMessage *msg, void *user_data)
>  {

In this patch it starts to become really confusing what an 'info' is. It
would be great if it could be given a more obvious name. Also, it would
help if either 'session->info' were to be used instead of 'info' here,
or that 'session->policy_config' were to be accessed as 'allowed_policy'
or something. Now there is an "imbalance", which makes the source of all
the structures hard to grasp. So that it is absolutely clear which one
is carved in stone, which one is the requested part and where the end
result goes. Just my $0.02 here, let's continue with this patch set
before hastily renaming things.

Cheers,

        Patrik


_______________________________________________
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

Reply via email to