2013-11-28 14:37:57

by Andrei Emeltchenko

[permalink] [raw]
Subject: [PATCH 01/10] android: Avoid memory leak warnings for event_loop

From: Andrei Emeltchenko <[email protected]>

Move creation of event_loop closer to g_main_loop_run. This avoids
calling g_main_loop_unref too many times in initialization error paths.
This is safe since g_main_loop_quit eval to NOOP if parameter == NULL.
---
android/main.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/android/main.c b/android/main.c
index bfd2a87..830eef2 100644
--- a/android/main.c
+++ b/android/main.c
@@ -559,7 +559,6 @@ int main(int argc, char *argv[])
exit(EXIT_SUCCESS);
}

- event_loop = g_main_loop_new(NULL, FALSE);
signal = setup_signalfd();
if (!signal)
return EXIT_FAILURE;
@@ -584,6 +583,8 @@ int main(int argc, char *argv[])

DBG("Entering main loop");

+ event_loop = g_main_loop_new(NULL, FALSE);
+
g_main_loop_run(event_loop);

g_source_remove(signal);
--
1.8.3.2



2013-11-29 08:31:08

by Johan Hedberg

[permalink] [raw]
Subject: Re: [PATCH 10/10] android/socket: Remove unneeded code

Hi Andrei,

On Thu, Nov 28, 2013, Andrei Emeltchenko wrote:
> From: Andrei Emeltchenko <[email protected]>
>
> The flag is already set in bt_io_listen.
> ---
> android/socket.c | 2 --
> 1 file changed, 2 deletions(-)

Patches 6-10 have been applied. Thanks.

Johan

2013-11-29 08:29:43

by Johan Hedberg

[permalink] [raw]
Subject: Re: [PATCH 04/10] android/main: Free enabled string on exit

Hi Andrei,

On Thu, Nov 28, 2013, Andrei Emeltchenko wrote:
> From: Andrei Emeltchenko <[email protected]>
>
> ---
> android/main.c | 3 +++
> 1 file changed, 3 insertions(+)

Patches 1-4 have been applied. Thanks.

Johan

2013-11-29 08:26:48

by Johan Hedberg

[permalink] [raw]
Subject: Re: [PATCH 05/10] android/socket: Cleanup sockets on unregister

Hi Andrei,

On Thu, Nov 28, 2013, Andrei Emeltchenko wrote:
> This cleans up rfsock structures closing all sockets and making general cleanup
> for servers and for connections. This will be called form socket unregister.
> ---
> android/socket.c | 20 ++++++++++++++++++++
> 1 file changed, 20 insertions(+)
>
> diff --git a/android/socket.c b/android/socket.c
> index 1fb154d..d55db54 100644
> --- a/android/socket.c
> +++ b/android/socket.c
> @@ -943,7 +943,27 @@ bool bt_socket_register(int sk, const bdaddr_t *addr)
> return true;
> }
>
> +static void free_connection(gpointer data, gpointer user_data)
> +{
> + struct rfcomm_sock *rfsock = data;
> +
> + connections = g_list_remove(connections, rfsock);
> + cleanup_rfsock(rfsock);
> +}
> +
> +static void free_server(gpointer data, gpointer user_data)
> +{
> + struct rfcomm_sock *rfsock = data;
> +
> + servers = g_list_remove(servers, rfsock);
> + cleanup_rfsock(rfsock);
> +}
> +
> void bt_socket_unregister(void)
> {
> DBG("");
> +
> + g_list_foreach(connections, free_connection, NULL);
> +
> + g_list_foreach(servers, free_server, NULL);
> }

I think what you're looking for here is g_slist_free_full instead of
g_slist_foreach. That way you won't need to have any helper functions at
all but you can directly pass cleanup_rfsock to it.

Johan

2013-11-28 14:37:58

by Andrei Emeltchenko

[permalink] [raw]
Subject: [PATCH 02/10] android/main: Remove timeout source on exit

From: Andrei Emeltchenko <[email protected]>

This fixes memory leak types of warnings from some tools.
---
android/main.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/android/main.c b/android/main.c
index 830eef2..dd5c622 100644
--- a/android/main.c
+++ b/android/main.c
@@ -575,8 +575,10 @@ int main(int argc, char *argv[])
return EXIT_FAILURE;
}

- if (!bt_bluetooth_start(option_index, adapter_ready))
+ if (!bt_bluetooth_start(option_index, adapter_ready)) {
+ g_source_remove(bluetooth_start_timeout);
return EXIT_FAILURE;
+ }

/* Use params: mtu = 0, flags = 0 */
start_sdp_server(0, 0);
@@ -589,6 +591,9 @@ int main(int argc, char *argv[])

g_source_remove(signal);

+ if (bluetooth_start_timeout > 0)
+ g_source_remove(bluetooth_start_timeout);
+
cleanup_hal_connection();
stop_sdp_server();
bt_bluetooth_cleanup();
--
1.8.3.2


2013-11-28 14:37:59

by Andrei Emeltchenko

[permalink] [raw]
Subject: [PATCH 03/10] android/main: Remove signal source on exit

From: Andrei Emeltchenko <[email protected]>

Remove signal source on exit and move check capability function in order
to avoid extra check.
---
android/main.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/android/main.c b/android/main.c
index dd5c622..4dc3061 100644
--- a/android/main.c
+++ b/android/main.c
@@ -565,18 +565,22 @@ int main(int argc, char *argv[])

__btd_log_init("*", 0);

- if (!set_capabilities())
+ if (!set_capabilities()) {
+ g_source_remove(signal);
return EXIT_FAILURE;
+ }

bluetooth_start_timeout = g_timeout_add_seconds(STARTUP_GRACE_SECONDS,
quit_eventloop, NULL);
if (bluetooth_start_timeout == 0) {
error("Failed to init startup timeout");
+ g_source_remove(signal);
return EXIT_FAILURE;
}

if (!bt_bluetooth_start(option_index, adapter_ready)) {
g_source_remove(bluetooth_start_timeout);
+ g_source_remove(signal);
return EXIT_FAILURE;
}

--
1.8.3.2


2013-11-28 14:38:00

by Andrei Emeltchenko

[permalink] [raw]
Subject: [PATCH 04/10] android/main: Free enabled string on exit

From: Andrei Emeltchenko <[email protected]>

---
android/main.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/android/main.c b/android/main.c
index 4dc3061..30792c5 100644
--- a/android/main.c
+++ b/android/main.c
@@ -566,6 +566,7 @@ int main(int argc, char *argv[])
__btd_log_init("*", 0);

if (!set_capabilities()) {
+ __btd_log_cleanup();
g_source_remove(signal);
return EXIT_FAILURE;
}
@@ -574,11 +575,13 @@ int main(int argc, char *argv[])
quit_eventloop, NULL);
if (bluetooth_start_timeout == 0) {
error("Failed to init startup timeout");
+ __btd_log_cleanup();
g_source_remove(signal);
return EXIT_FAILURE;
}

if (!bt_bluetooth_start(option_index, adapter_ready)) {
+ __btd_log_cleanup();
g_source_remove(bluetooth_start_timeout);
g_source_remove(signal);
return EXIT_FAILURE;
--
1.8.3.2


2013-11-28 14:38:01

by Andrei Emeltchenko

[permalink] [raw]
Subject: [PATCH 05/10] android/socket: Cleanup sockets on unregister

From: Andrei Emeltchenko <[email protected]>

This cleans up rfsock structures closing all sockets and making general cleanup
for servers and for connections. This will be called form socket unregister.
---
android/socket.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)

diff --git a/android/socket.c b/android/socket.c
index 1fb154d..d55db54 100644
--- a/android/socket.c
+++ b/android/socket.c
@@ -943,7 +943,27 @@ bool bt_socket_register(int sk, const bdaddr_t *addr)
return true;
}

+static void free_connection(gpointer data, gpointer user_data)
+{
+ struct rfcomm_sock *rfsock = data;
+
+ connections = g_list_remove(connections, rfsock);
+ cleanup_rfsock(rfsock);
+}
+
+static void free_server(gpointer data, gpointer user_data)
+{
+ struct rfcomm_sock *rfsock = data;
+
+ servers = g_list_remove(servers, rfsock);
+ cleanup_rfsock(rfsock);
+}
+
void bt_socket_unregister(void)
{
DBG("");
+
+ g_list_foreach(connections, free_connection, NULL);
+
+ g_list_foreach(servers, free_server, NULL);
}
--
1.8.3.2


2013-11-28 14:38:04

by Andrei Emeltchenko

[permalink] [raw]
Subject: [PATCH 08/10] android/socket: Strip extra log messages

From: Andrei Emeltchenko <[email protected]>

Remove debug messages when sending data, debug still exist for connection
establishment. Do not print error when connection hang up, print debug
instead.
---
android/socket.c | 26 ++++++++++----------------
1 file changed, 10 insertions(+), 16 deletions(-)

diff --git a/android/socket.c b/android/socket.c
index 3f07dc6..45ff90e 100644
--- a/android/socket.c
+++ b/android/socket.c
@@ -488,11 +488,12 @@ static gboolean sock_stack_event_cb(GIOChannel *io, GIOCondition cond,
unsigned char buf[1024];
int len, sent;

- DBG("rfsock: fd %d real_sock %d chan %u sock %d",
- rfsock->fd, rfsock->real_sock, rfsock->channel,
- g_io_channel_unix_get_fd(io));
+ if (cond & G_IO_HUP) {
+ DBG("Socket %d hang up", g_io_channel_unix_get_fd(io));
+ goto fail;
+ }

- if (cond & (G_IO_ERR | G_IO_HUP | G_IO_NVAL)) {
+ if (cond & (G_IO_ERR | G_IO_NVAL)) {
error("Socket error: sock %d cond %d",
g_io_channel_unix_get_fd(io), cond);
goto fail;
@@ -505,16 +506,12 @@ static gboolean sock_stack_event_cb(GIOChannel *io, GIOCondition cond,
return TRUE;
}

- DBG("read %d bytes write to %d", len, rfsock->real_sock);
-
sent = try_write_all(rfsock->real_sock, buf, len);
if (sent < 0) {
error("write(): %s", strerror(errno));
goto fail;
}

- DBG("Written %d bytes", sent);
-
return TRUE;
fail:
connections = g_list_remove(connections, rfsock);
@@ -530,11 +527,12 @@ static gboolean sock_rfcomm_event_cb(GIOChannel *io, GIOCondition cond,
unsigned char buf[1024];
int len, sent;

- DBG("rfsock: fd %d real_sock %d chan %u sock %d",
- rfsock->fd, rfsock->real_sock, rfsock->channel,
- g_io_channel_unix_get_fd(io));
+ if (cond & G_IO_HUP) {
+ DBG("Socket %d hang up", g_io_channel_unix_get_fd(io));
+ goto fail;
+ }

- if (cond & (G_IO_ERR | G_IO_HUP | G_IO_NVAL)) {
+ if (cond & (G_IO_ERR | G_IO_NVAL)) {
error("Socket error: sock %d cond %d",
g_io_channel_unix_get_fd(io), cond);
goto fail;
@@ -547,16 +545,12 @@ static gboolean sock_rfcomm_event_cb(GIOChannel *io, GIOCondition cond,
return TRUE;
}

- DBG("read %d bytes, write to fd %d", len, rfsock->fd);
-
sent = try_write_all(rfsock->fd, buf, len);
if (sent < 0) {
error("write(): %s", strerror(errno));
goto fail;
}

- DBG("Written %d bytes", sent);
-
return TRUE;
fail:
connections = g_list_remove(connections, rfsock);
--
1.8.3.2


2013-11-28 14:38:02

by Andrei Emeltchenko

[permalink] [raw]
Subject: [PATCH 06/10] android/socket: Fix rfsock lists

From: Andrei Emeltchenko <[email protected]>

This fixes several places where rfsock structure were not removed
from the list due to connection errors.
---
android/socket.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/android/socket.c b/android/socket.c
index d55db54..3f07dc6 100644
--- a/android/socket.c
+++ b/android/socket.c
@@ -625,8 +625,6 @@ static void accept_cb(GIOChannel *io, GError *err, gpointer user_data)
return;
}

- connections = g_list_append(connections, rfsock_acc);
-
DBG("rfsock: fd %d real_sock %d chan %u sock %d",
rfsock->fd, rfsock->real_sock, rfsock->channel,
sock_acc);
@@ -636,6 +634,8 @@ static void accept_cb(GIOChannel *io, GError *err, gpointer user_data)
return;
}

+ connections = g_list_append(connections, rfsock_acc);
+
/* Handle events from Android */
cond = G_IO_IN | G_IO_HUP | G_IO_ERR | G_IO_NVAL;
io_stack = g_io_channel_unix_new(rfsock_acc->fd);
@@ -700,7 +700,6 @@ static int handle_listen(void *buf)
}

rfsock->real_sock = g_io_channel_unix_get_fd(io);
- servers = g_list_append(servers, rfsock);

/* TODO: Add server watch */
g_io_channel_set_close_on_unref(io, TRUE);
@@ -717,6 +716,8 @@ static int handle_listen(void *buf)

rfsock->service_handle = sdp_service_register(profile, cmd->name);

+ servers = g_list_append(servers, rfsock);
+
return hal_fd;
}

@@ -787,6 +788,7 @@ static void connect_cb(GIOChannel *io, GError *err, gpointer user_data)

return;
fail:
+ connections = g_list_remove(connections, rfsock);
cleanup_rfsock(rfsock);
}

@@ -865,6 +867,7 @@ static void sdp_search_cb(sdp_list_t *recs, int err, gpointer data)

return;
fail:
+ connections = g_list_remove(connections, rfsock);
cleanup_rfsock(rfsock);
}

--
1.8.3.2


2013-11-28 14:38:06

by Andrei Emeltchenko

[permalink] [raw]
Subject: [PATCH 10/10] android/socket: Remove unneeded code

From: Andrei Emeltchenko <[email protected]>

The flag is already set in bt_io_listen.
---
android/socket.c | 2 --
1 file changed, 2 deletions(-)

diff --git a/android/socket.c b/android/socket.c
index 8df6e01..ea88712 100644
--- a/android/socket.c
+++ b/android/socket.c
@@ -696,8 +696,6 @@ static int handle_listen(void *buf)

rfsock->real_sock = g_io_channel_unix_get_fd(io);

- /* TODO: Add server watch */
- g_io_channel_set_close_on_unref(io, TRUE);
g_io_channel_unref(io);

DBG("real_sock %d fd %d hal_fd %d", rfsock->real_sock, rfsock->fd,
--
1.8.3.2


2013-11-28 14:38:05

by Andrei Emeltchenko

[permalink] [raw]
Subject: [PATCH 09/10] android/socket: Avoid double close of file descriptor

From: Andrei Emeltchenko <[email protected]>

Since we close all file descriptors in cleanup_rfsock do not close it also
during iochannel cleaning up. The flag was setup in bt_io_listen and
bt_io_connect calls.
---
android/socket.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/android/socket.c b/android/socket.c
index 45ff90e..8df6e01 100644
--- a/android/socket.c
+++ b/android/socket.c
@@ -641,6 +641,7 @@ static void accept_cb(GIOChannel *io, GError *err, gpointer user_data)
/* Handle rfcomm events */
cond = G_IO_IN | G_IO_HUP | G_IO_ERR | G_IO_NVAL;
id = g_io_add_watch(io, cond, sock_rfcomm_event_cb, rfsock_acc);
+ g_io_channel_set_close_on_unref(io, FALSE);

rfsock_acc->rfcomm_watch = id;

@@ -777,6 +778,7 @@ static void connect_cb(GIOChannel *io, GError *err, gpointer user_data)
/* Handle rfcomm events */
cond = G_IO_IN | G_IO_ERR | G_IO_HUP | G_IO_NVAL;
id = g_io_add_watch(io, cond, sock_rfcomm_event_cb, rfsock);
+ g_io_channel_set_close_on_unref(io, FALSE);

rfsock->rfcomm_watch = id;

--
1.8.3.2


2013-11-28 14:38:03

by Andrei Emeltchenko

[permalink] [raw]
Subject: [PATCH 07/10] android/hidhost: Shutdown ctrl_io channel if intr_io fails

From: Andrei Emeltchenko <[email protected]>

This fix possible memory leak.
---
android/hidhost.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/android/hidhost.c b/android/hidhost.c
index d50c5b8..1f5aa5d 100644
--- a/android/hidhost.c
+++ b/android/hidhost.c
@@ -1219,8 +1219,12 @@ bool bt_hid_register(int sk, const bdaddr_t *addr)
BT_IO_OPT_INVALID);
if (!intr_io) {
error("Failed to listen on intr channel: %s", err->message);
- g_io_channel_unref(ctrl_io);
g_error_free(err);
+
+ g_io_channel_shutdown(ctrl_io, TRUE, NULL);
+ g_io_channel_unref(ctrl_io);
+ ctrl_io = NULL;
+
return false;
}

--
1.8.3.2