2015-12-01 13:11:32

by Patrik Flykt

Subject: [PATCH v2 1/4] gdbus: Fix Memory Leak

From: Saurav Babu <[email protected]>

Members of data are allocated memory but not freed only data is freed
---

Hi,

This patch came up on the ConnMan mailing list, applies to Bluez too.

Patrik


gdbus/watch.c | 50 +++++++++++++++++++++++++-------------------------
1 file changed, 25 insertions(+), 25 deletions(-)

diff --git a/gdbus/watch.c b/gdbus/watch.c
index b60f650..447e486 100644
--- a/gdbus/watch.c
+++ b/gdbus/watch.c
@@ -204,6 +204,30 @@ static gboolean remove_match(struct filter_data *data)
return TRUE;
}

+static void filter_data_free(struct filter_data *data)
+{
+ GSList *l;
+
+ /* Remove filter if there are no listeners left for the connection */
+ if (filter_data_find(data->connection) == NULL)
+ dbus_connection_remove_filter(data->connection, message_filter,
+ NULL);
+
+ for (l = data->callbacks; l != NULL; l = l->next)
+ g_free(l->data);
+
+ g_slist_free(data->callbacks);
+ g_dbus_remove_watch(data->connection, data->name_watch);
+ g_free(data->name);
+ g_free(data->owner);
+ g_free(data->path);
+ g_free(data->interface);
+ g_free(data->member);
+ g_free(data->argument);
+ dbus_connection_unref(data->connection);
+ g_free(data);
+}
+
static struct filter_data *filter_data_get(DBusConnection *connection,
DBusHandleMessageFunction filter,
const char *sender,
@@ -248,7 +272,7 @@ proceed:
data->argument = g_strdup(argument);

if (!add_match(data, filter)) {
- g_free(data);
+ filter_data_free(data);
return NULL;
}

@@ -277,30 +301,6 @@ static struct filter_callback *filter_data_find_callback(
return NULL;
}

-static void filter_data_free(struct filter_data *data)
-{
- GSList *l;
-
- /* Remove filter if there are no listeners left for the connection */
- if (filter_data_find(data->connection) == NULL)
- dbus_connection_remove_filter(data->connection, message_filter,
- NULL);
-
- for (l = data->callbacks; l != NULL; l = l->next)
- g_free(l->data);
-
- g_slist_free(data->callbacks);
- g_dbus_remove_watch(data->connection, data->name_watch);
- g_free(data->name);
- g_free(data->owner);
- g_free(data->path);
- g_free(data->interface);
- g_free(data->member);
- g_free(data->argument);
- dbus_connection_unref(data->connection);
- g_free(data);
-}
-
static void filter_data_call_and_free(struct filter_data *data)
{
GSList *l;
--
1.9.1

_______________________________________________
connman mailing list
[email protected]
https://lists.01.org/mailman/listinfo/connman

2015-12-04 12:31:12

by Von Dentz, Luiz

Subject: Re: [PATCH v2 1/4] gdbus: Fix Memory Leak

Hi,

On Tue, Dec 1, 2015 at 3:11 PM, Patrik Flykt
<[email protected]> wrote:
> From: Saurav Babu <[email protected]>
>
> Members of data are allocated memory but not freed only data is freed
> ---
>
> Hi,
>
> This patch came up on the ConnMan mailing list, applies to Bluez too.
>
> Patrik
>
>
> gdbus/watch.c | 50 +++++++++++++++++++++++++-------------------------
> 1 file changed, 25 insertions(+), 25 deletions(-)
>
> diff --git a/gdbus/watch.c b/gdbus/watch.c
> index b60f650..447e486 100644
> --- a/gdbus/watch.c
> +++ b/gdbus/watch.c
> @@ -204,6 +204,30 @@ static gboolean remove_match(struct filter_data *data)
> return TRUE;
> }
>
> +static void filter_data_free(struct filter_data *data)
> +{
> + GSList *l;
> +
> + /* Remove filter if there are no listeners left for the connection */
> + if (filter_data_find(data->connection) == NULL)
> + dbus_connection_remove_filter(data->connection, message_filter,
> + NULL);
> +
> + for (l = data->callbacks; l != NULL; l = l->next)
> + g_free(l->data);
> +
> + g_slist_free(data->callbacks);
> + g_dbus_remove_watch(data->connection, data->name_watch);
> + g_free(data->name);
> + g_free(data->owner);
> + g_free(data->path);
> + g_free(data->interface);
> + g_free(data->member);
> + g_free(data->argument);
> + dbus_connection_unref(data->connection);
> + g_free(data);
> +}
> +
> static struct filter_data *filter_data_get(DBusConnection *connection,
> DBusHandleMessageFunction filter,
> const char *sender,
> @@ -248,7 +272,7 @@ proceed:
> data->argument = g_strdup(argument);
>
> if (!add_match(data, filter)) {
> - g_free(data);
> + filter_data_free(data);
> return NULL;
> }
>
> @@ -277,30 +301,6 @@ static struct filter_callback *filter_data_find_callback(
> return NULL;
> }
>
> -static void filter_data_free(struct filter_data *data)
> -{
> - GSList *l;
> -
> - /* Remove filter if there are no listeners left for the connection */
> - if (filter_data_find(data->connection) == NULL)
> - dbus_connection_remove_filter(data->connection, message_filter,
> - NULL);
> -
> - for (l = data->callbacks; l != NULL; l = l->next)
> - g_free(l->data);
> -
> - g_slist_free(data->callbacks);
> - g_dbus_remove_watch(data->connection, data->name_watch);
> - g_free(data->name);
> - g_free(data->owner);
> - g_free(data->path);
> - g_free(data->interface);
> - g_free(data->member);
> - g_free(data->argument);
> - dbus_connection_unref(data->connection);
> - g_free(data);
> -}
> -
> static void filter_data_call_and_free(struct filter_data *data)
> {
> GSList *l;
> --
> 1.9.1

This has been applied to BlueZ.