[PATCH 1/2] service: Implement ServicesAdded and ServicesRemoved signals

Jukka Rissanen jukka.rissanen at linux.intel.com
Tue Feb 7 02:23:14 PST 2012


Hi,

few comments below:

On 02/07/2012 11:09 AM, patrik.flykt at linux.intel.com wrote:
> From: Patrik Flykt<patrik.flykt at linux.intel.com>
>
> When services are added or removed, collect them into separate
> lists in order to send as many as possible in one go and with
> then removed ones sent first. Add a wait of 100ms to collect
> more services in one batch. When removing a service check
> whether it was also on the added list; if so, remove it from
> that list.
> ---
>   src/manager.c |    2 +
>   src/service.c |  144 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 146 insertions(+), 0 deletions(-)
>
> diff --git a/src/manager.c b/src/manager.c
> index cf2c624..7cf1eec 100644
> --- a/src/manager.c
> +++ b/src/manager.c
> @@ -439,6 +439,8 @@ static GDBusSignalTable manager_signals[] = {
>   	{ "PropertyChanged", "sv" },
>   	{ "TechnologyAdded", "a{sv}" },
>   	{ "TechnologyRemoved", "o" },
> +	{ "ServicesAdded",   "a(oa{sv})" },
> +	{ "ServicesRemoved", "a{o}" },
>   	{ },
>   };
>
> diff --git a/src/service.c b/src/service.c
> index b0acd5f..d9bb8bc 100644
> --- a/src/service.c
> +++ b/src/service.c
> @@ -3318,6 +3318,143 @@ static DBusMessage *reset_counters(DBusConnection *conn,
>   	return g_dbus_create_reply(msg, DBUS_TYPE_INVALID);
>   }
>
> +static struct _services_notify {
> +	int id;
> +	GSList *added;
> +	GSList *removed;
> +} *services_notify;

This could also be a normal variable and not a pointer. So there would 
be no need to allocate/deallocate it in init/cleanup.

> @@ -5403,6 +5543,8 @@ int __connman_service_init(void)
>
>   	service_list = g_sequence_new(service_free);
>
> +	services_notify = g_new0(struct _services_notify, 1);
> +
>   	return 0;
>   }
>
> @@ -5422,5 +5564,7 @@ void __connman_service_cleanup(void)
>   	g_slist_free(counter_list);
>   	counter_list = NULL;
>
> +	g_free(services_notify);
> +

Minor issue but it would be nice to deallocate the added/removed list so 
that it is easier to read valgrind logs and notice real memory leaks 
more easily.

We should also remove the timer before we remove services_notify.

>   	dbus_connection_unref(connection);
>   }



Cheers,
Jukka



More information about the connman mailing list