Re: [MM] [PATCH] bearer: call set_signal_handlers after setting 'config' property

[Aleksander Morgado <aleksander lanedo com>]

On 09/06/2012 03:31 AM, Ben Chan wrote:
> Modem plugins set the 'modem' property before the 'config' property when
> creating a bearer. set_signal_handlers() should thus be called after both
> properties are set such that modem_{3gpp,cdma}_registration_state_changed
> checks roaming allowance correctly when launching a connection.

We shouldn't depend on the order in which the properties are set; I
updated your patch to handle that, so that set_signal_handlers() is
called when we have both the 'modem' propery and the 'config' property,
regardless of the order.

Pushed it already; thanks!


> ---
>  src/mm-bearer.c |    8 +++++---
>  1 files changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/src/mm-bearer.c b/src/mm-bearer.c
> index d9a7e62..41cf3bb 100644
> --- a/src/mm-bearer.c
> +++ b/src/mm-bearer.c
> @@ -911,9 +911,6 @@ set_property (GObject *object,
>              g_object_bind_property (self->priv->modem, MM_BASE_MODEM_CONNECTION,
>                                      self, MM_BEARER_CONNECTION,
>                                      G_BINDING_DEFAULT | G_BINDING_SYNC_CREATE);
> -
> -            /* Listen to 3GPP/CDMA registration state changes */
> -            set_signal_handlers (self);
>          }
>          break;
>      case PROP_STATUS:
> @@ -925,6 +922,11 @@ set_property (GObject *object,
>  
>          g_clear_object (&self->priv->config);
>          self->priv->config = g_value_dup_object (value);
> +        if (self->priv->modem) {
> +            /* Listen to 3GPP/CDMA registration state changes.
> +             * The 'modem' property is expected to be set before the 'config' property. */
> +            set_signal_handlers (self);
> +        }
>          /* Also expose the properties */
>          dictionary = mm_bearer_properties_get_dictionary (self->priv->config);
>          mm_gdbus_bearer_set_properties (MM_GDBUS_BEARER (self), dictionary);
> 


-- 
Aleksander