Send connman mailing list submissions to
[email protected]
To subscribe or unsubscribe via the World Wide Web, visit
https://lists.01.org/mailman/listinfo/connman
or, via email, send a message with subject or body 'help' to
[email protected]
You can reach the person managing the list at
[email protected]
When replying, please edit your Subject line so it is more specific
than "Re: Contents of connman digest..."
Today's Topics:
1. Re: [PATCHv2 4/4] service: Fixed Memory Leak (Patrik Flykt)
2. Re: [PATCH 0/2] Set search domains on connect (Patrik Flykt)
3. Re: [PATCH] agent: avoid recursion in connman_agent_cancel()
(Patrik Flykt)
4. Re: [PATCH] wifi: set interface when added (Patrik Flykt)
5. Re: [PATCH v2] gdhcp: don't call the lease added callback for
OFFER (Patrik Flykt)
6. [PATCH v3 4/4] service: Fixed Memory Leak (Saurav Babu)
7. Re: [PATCH 1/4] gdbus: Fix Memory Leak (Patrik Flykt)
8. Re: [PATCH 2/4] gweb: Properly handle memory failure
scenarios (Patrik Flykt)
9. Re: [PATCH 3/4] dhcp: Fixed Memory Leak (Patrik Flykt)
10. [PATCH v2 1/4] gdbus: Fix Memory Leak (Saurav Babu)
----------------------------------------------------------------------
Message: 1
Date: Tue, 01 Dec 2015 11:19:33 +0200
From: Patrik Flykt <[email protected]>
To: Saurav Babu <[email protected]>
Cc: [email protected], [email protected]
Subject: Re: [PATCHv2 4/4] service: Fixed Memory Leak
Message-ID: <[email protected]>
Content-Type: text/plain; charset="UTF-8"
Hi,
On Thu, 2015-11-26 at 16:21 +0530, Saurav Babu wrote:
> When memory allocation fails for any value of j for servers[j] then all
> the memory allocated for 0 to j-1 should be freed alongwith servers
> ---
> src/service.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/src/service.c b/src/service.c
> index e3317dd..1aced50 100644
> --- a/src/service.c
> +++ b/src/service.c
> @@ -2732,8 +2732,12 @@ int __connman_service_timeserver_remove(struct
> connman_service *service,
> for (i = 0, j = 0; i < len; i++) {
> if (g_strcmp0(service->timeservers[i], timeserver) != 0) {
> servers[j] = g_strdup(service->timeservers[i]);
> - if (!servers[j])
> + if (!servers[j]) {
> + while (j >= 0)
> + g_free(servers[--j]);
if j is zero, g_free will free index -1, right? So the while loop is
about j > 0?
> + g_free(servers);
> return -ENOMEM;
> + }
> j++;
> }
> }
Patrik
------------------------------
Message: 2
Date: Tue, 01 Dec 2015 11:24:18 +0200
From: Patrik Flykt <[email protected]>
To: [email protected]
Subject: Re: [PATCH 0/2] Set search domains on connect
Message-ID: <[email protected]>
Content-Type: text/plain; charset="UTF-8"
On Thu, 2015-11-26 at 16:09 +0200, Patrik Flykt wrote:
> Hi,
>
> Patch 1/2 obsoletes the previous patch called "service: Set search domains
> regardless of connected state". Instead, ensure that search domains are
> added at least once after a successful connection. The previously committed
> patches ensure that search domains are updated properly when new nameservers
> are added.
>
> Patch 2/2 trivially moves code around to get rid of a somewhat useless
> function declaration.
Both patches applied.
Patrik
------------------------------
Message: 3
Date: Tue, 01 Dec 2015 11:33:16 +0200
From: Patrik Flykt <[email protected]>
To: Michael Olbrich <[email protected]>
Cc: [email protected]
Subject: Re: [PATCH] agent: avoid recursion in connman_agent_cancel()
Message-ID: <[email protected]>
Content-Type: text/plain; charset="UTF-8"
On Mon, 2015-11-30 at 13:07 +0100, Michael Olbrich wrote:
> It is possible that connman_agent_cancel() from within
> connman_agent_cancel():
>
> -> request->callback() == request_peer_authorization_reply()
> -> auth_reply->peer_callback() == request_authorization_cb()
> -> peer_driver->connect() returns -EBUSY (wifi->p2p_connecting == TRUE)
> -> connman_peer_set_state()
> with old_state == 2 and new_state == 1
> -> peer_disconnect()
> -> connman_agent_cancel()
>
> Break the loop by removing the request from the list before calling the
> callback funtion.
Applied, good catch!
Patrik
------------------------------
Message: 4
Date: Tue, 01 Dec 2015 11:33:32 +0200
From: Patrik Flykt <[email protected]>
To: Michael Olbrich <[email protected]>
Cc: [email protected]
Subject: Re: [PATCH] wifi: set interface when added
Message-ID: <[email protected]>
Content-Type: text/plain; charset="UTF-8"
On Mon, 2015-11-30 at 13:08 +0100, Michael Olbrich wrote:
> Otherwise g_supplicant_interface_set_data(interface, NULL) is not called
> for the g_suplicant interface when the wifi object is removed. As a result,
> the g_suplicant interface may access the wifi object when it is already
> deleted.
> This is rarely a problem, because the g_suplicant interface is deleted
> shortly afterwards, but occasionally a D-Bus signal arrives at the wrong
> time and triggers this use after free issue.
Applied, thanks!
Patrik
------------------------------
Message: 5
Date: Tue, 01 Dec 2015 11:32:58 +0200
From: Patrik Flykt <[email protected]>
To: Michael Olbrich <[email protected]>
Cc: [email protected]
Subject: Re: [PATCH v2] gdhcp: don't call the lease added callback for
OFFER
Message-ID: <[email protected]>
Content-Type: text/plain; charset="UTF-8"
On Mon, 2015-11-30 at 13:00 +0100, Michael Olbrich wrote:
> The lease added callback only used to determine and announce the peer
> address. Without this, the peer address is announced via D-Bus before the
> peer actually ueses the address.
> If the first OFFER is lost, then the difference can actually be several
> seconds.
>
> ---
>
> Hi,
>
> it took a while for me to get back to this, but here is the updated patch.
Thanks! Applied!
Patrik
------------------------------
Message: 6
Date: Tue, 01 Dec 2015 15:06:17 +0530
From: Saurav Babu <[email protected]>
To: [email protected]
Cc: [email protected]
Subject: [PATCH v3 4/4] service: Fixed Memory Leak
Message-ID:
<[email protected]>
When memory allocation fails for any value of j for servers[j] then all
the memory allocated for 0 to j-1 should be freed alongwith servers
---
src/service.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/src/service.c b/src/service.c
index e3317dd..1aced50 100644
--- a/src/service.c
+++ b/src/service.c
@@ -2732,8 +2732,12 @@ int __connman_service_timeserver_remove(struct
connman_service *service,
for (i = 0, j = 0; i < len; i++) {
if (g_strcmp0(service->timeservers[i], timeserver) != 0) {
servers[j] = g_strdup(service->timeservers[i]);
- if (!servers[j])
+ if (!servers[j]) {
+ while (j > 0)
+ g_free(servers[--j]);
+ g_free(servers);
return -ENOMEM;
+ }
j++;
}
}
--
1.9.1
------------------------------
Message: 7
Date: Tue, 01 Dec 2015 13:23:04 +0200
From: Patrik Flykt <[email protected]>
To: Saurav Babu <[email protected]>
Cc: [email protected], [email protected]
Subject: Re: [PATCH 1/4] gdbus: Fix Memory Leak
Message-ID: <[email protected]>
Content-Type: text/plain; charset="UTF-8"
On Thu, 2015-11-26 at 14:52 +0530, Saurav Babu wrote:
> Members of data are allocated memory but not freed only data is freed.
> This patch frees other members of data also.
> ---
> gdbus/watch.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/gdbus/watch.c b/gdbus/watch.c
> index b60f650..8ab9aae 100644
> --- a/gdbus/watch.c
> +++ b/gdbus/watch.c
> @@ -248,6 +248,12 @@ proceed:
> data->argument = g_strdup(argument);
>
> if (!add_match(data, filter)) {
> + g_free(data->name);
> + g_free(data->owner);
> + g_free(data->path);
> + g_free(data->interface);
> + g_free(data->member);
> + g_free(data->argument);
> g_free(data);
> return NULL;
> }
Please use filter_data_free() here. And send the resulting patch also to
[email protected].
Cheers,
Patrik
------------------------------
Message: 8
Date: Tue, 01 Dec 2015 13:59:24 +0200
From: Patrik Flykt <[email protected]>
To: Saurav Babu <[email protected]>
Cc: [email protected], [email protected]
Subject: Re: [PATCH 2/4] gweb: Properly handle memory failure
scenarios
Message-ID: <[email protected]>
Content-Type: text/plain; charset="UTF-8"
On Thu, 2015-11-26 at 14:52 +0530, Saurav Babu wrote:
> ---
> gweb/gweb.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/gweb/gweb.c b/gweb/gweb.c
> index 5f18a0e..f6e819f 100644
> --- a/gweb/gweb.c
> +++ b/gweb/gweb.c
> @@ -1472,13 +1472,20 @@ GWebParser *g_web_parser_new(const char *begin, const
> char *end,
> parser->ref_count = 1;
>
> parser->begin_token = g_strdup(begin);
> - parser->end_token = g_strdup(end);
The implementation of g_strdup() is such that it always allocates memory
or terminates the program, as it in that case does a memcpy to the
destination address NULL.
>
> if (!parser->begin_token) {
> g_free(parser);
> return NULL;
> }
If there is no begin_token, this is to be checked in the beginning of
the function. Apparently the code works even if end_token is NULL, but
that seems not to be the desired behavior. Thus begin_token and
end_token need to be checked in the beginning of this function only.
Cheers,
Patrik
------------------------------
Message: 9
Date: Tue, 01 Dec 2015 14:04:11 +0200
From: Patrik Flykt <[email protected]>
To: Saurav Babu <[email protected]>
Cc: [email protected], [email protected]
Subject: Re: [PATCH 3/4] dhcp: Fixed Memory Leak
Message-ID: <[email protected]>
Content-Type: text/plain; charset="UTF-8"
Hi,
On Thu, 2015-11-26 at 14:52 +0530, Saurav Babu wrote:
> Memory allocated should be feed when apply_lease_available_on_network()
> fails
> ---
> src/dhcp.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/src/dhcp.c b/src/dhcp.c
> index 9a74362..371983b 100644
> --- a/src/dhcp.c
> +++ b/src/dhcp.c
> @@ -461,8 +461,12 @@ static void lease_available_cb(GDHCPClient *dhcp_client,
> gpointer user_data)
> __connman_ipconfig_set_gateway(dhcp->ipconfig, gateway);
> }
>
> - if (!apply_lease_available_on_network(dhcp_client, dhcp))
> + if (!apply_lease_available_on_network(dhcp_client, dhcp)) {
> + g_free(address);
> + g_free(netmask);
> + g_free(gateway);
> return;
> + }
Here it's clearer to either add a goto past the next two lines into the
end of the function where these three variables are g_free()'d or turn
around the if() to run the next two lines if
apply_lease_available_on_network() succeeds.
> if (ip_change)
> dhcp_valid(dhcp);
Cheers,
Patrik
------------------------------
Message: 10
Date: Tue, 01 Dec 2015 17:34:29 +0530
From: Saurav Babu <[email protected]>
To: [email protected]
Cc: [email protected]
Subject: [PATCH v2 1/4] gdbus: Fix Memory Leak
Message-ID:
<[email protected]>
Members of data are allocated memory but not freed only data is freed
---
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
------------------------------
Subject: Digest Footer
_______________________________________________
connman mailing list
[email protected]
https://lists.01.org/mailman/listinfo/connman
------------------------------
End of connman Digest, Vol 2, Issue 1
*************************************
