linux-bluetooth.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] android/tester: Add Socket test close and listen
@ 2013-12-18 15:15 Andrei Emeltchenko
  2013-12-18 15:15 ` [PATCH 2/2] android/socket: Handle Android events for server socket Andrei Emeltchenko
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Andrei Emeltchenko @ 2013-12-18 15:15 UTC (permalink / raw)
  To: linux-bluetooth

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

This test the situation when Android close file descriptor we passed to
it and try to listen() again.
---
 android/android-tester.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 55 insertions(+)

diff --git a/android/android-tester.c b/android/android-tester.c
index 8a2861e..cb8fb4e 100644
--- a/android/android-tester.c
+++ b/android/android-tester.c
@@ -1141,6 +1141,57 @@ clean:
 		close(sock_fd);
 }
 
+static void test_listen_close(const void *test_data)
+{
+	struct test_data *data = tester_get_data();
+	const struct socket_data *test = data->test_data;
+	bt_status_t status;
+	int sock_fd = -1;
+
+	status = data->if_sock->listen(test->sock_type,
+					test->service_name, test->service_uuid,
+					test->channel, &sock_fd, test->flags);
+	if (status != test->expected_status) {
+		tester_test_failed();
+		goto clean;
+	}
+
+	/* Check that file descriptor is valid */
+	if (status == BT_STATUS_SUCCESS && fcntl(sock_fd, F_GETFD) == -1) {
+		tester_test_failed();
+		return;
+	}
+
+	tester_print("sock_fd: %d", sock_fd);
+
+	/* Now close sock_fd */
+	close(sock_fd);
+
+	/* Try to listen again */
+	status = data->if_sock->listen(test->sock_type,
+					test->service_name, test->service_uuid,
+					test->channel, &sock_fd, test->flags);
+	if (status != test->expected_status) {
+		tester_test_failed();
+		goto clean;
+	}
+
+	/* Check that file descriptor is valid */
+	if (status == BT_STATUS_SUCCESS && fcntl(sock_fd, F_GETFD) == -1) {
+		tester_test_failed();
+		return;
+	}
+
+	tester_print("sock_fd: %d", sock_fd);
+
+	tester_test_passed();
+
+clean:
+	if (sock_fd >= 0)
+		close(sock_fd);
+
+}
+
 static void test_generic_connect(const void *test_data)
 {
 	struct test_data *data = tester_get_data();
@@ -1363,6 +1414,10 @@ int main(int argc, char *argv[])
 			&btsock_success_check_chan,
 			setup_socket_interface, test_generic_listen, teardown);
 
+	test_bredrle("Socket Listen - Close and Listen again",
+			&btsock_success_check_chan,
+			setup_socket_interface, test_listen_close, teardown);
+
 	test_bredrle("Socket Connect - Invalid: sock_type 0",
 			&btsock_inv_param_socktype, setup_socket_interface,
 			test_generic_connect, teardown);
-- 
1.8.3.2


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/2] android/socket: Handle Android events for server socket
  2013-12-18 15:15 [PATCH 1/2] android/tester: Add Socket test close and listen Andrei Emeltchenko
@ 2013-12-18 15:15 ` Andrei Emeltchenko
  2013-12-19  8:18 ` [PATCH 1/2] android/tester: Add Socket test close and listen Johan Hedberg
  2013-12-19  8:51 ` [PATCHv2 " Andrei Emeltchenko
  2 siblings, 0 replies; 6+ messages in thread
From: Andrei Emeltchenko @ 2013-12-18 15:15 UTC (permalink / raw)
  To: linux-bluetooth

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

Add watch for tracking events from Android framework for server socket.
Android might want to close server connection, in this case we close
our listening socket and cleaning up rfsock structure. Glib watch is
added with high priority to avoid races.
---
 android/socket.c | 32 +++++++++++++++++++++++++++++++-
 1 file changed, 31 insertions(+), 1 deletion(-)

diff --git a/android/socket.c b/android/socket.c
index 7bc5b0b..de98ae4 100644
--- a/android/socket.c
+++ b/android/socket.c
@@ -622,6 +622,24 @@ static bool sock_send_accept(struct rfcomm_sock *rfsock, bdaddr_t *bdaddr,
 	return true;
 }
 
+static gboolean sock_server_stack_event_cb(GIOChannel *io, GIOCondition cond,
+								gpointer data)
+{
+	struct rfcomm_sock *rfsock = data;
+
+	DBG("sock %d cond %d", g_io_channel_unix_get_fd(io), cond);
+
+	if (cond & G_IO_NVAL)
+		return FALSE;
+
+	if (cond & (G_IO_ERR | G_IO_HUP )) {
+		servers = g_list_remove(servers, rfsock);
+		cleanup_rfsock(rfsock);
+	}
+
+	return FALSE;
+}
+
 static void accept_cb(GIOChannel *io, GError *err, gpointer user_data)
 {
 	struct rfcomm_sock *rfsock = user_data;
@@ -697,10 +715,12 @@ static void handle_listen(const void *buf, uint16_t len)
 	const struct profile_info *profile;
 	struct rfcomm_sock *rfsock = NULL;
 	BtIOSecLevel sec_level;
-	GIOChannel *io;
+	GIOChannel *io, *io_stack;
+	GIOCondition cond;
 	GError *err = NULL;
 	int hal_fd = -1;
 	int chan;
+	guint id;
 
 	DBG("");
 
@@ -738,6 +758,16 @@ static void handle_listen(const void *buf, uint16_t len)
 	g_io_channel_set_close_on_unref(io, FALSE);
 	g_io_channel_unref(io);
 
+	/* Handle events from Android */
+	cond = G_IO_HUP | G_IO_ERR | G_IO_NVAL;
+	io_stack = g_io_channel_unix_new(rfsock->fd);
+	id = g_io_add_watch_full(io_stack, G_PRIORITY_HIGH, cond,
+					sock_server_stack_event_cb, rfsock,
+					NULL);
+	g_io_channel_unref(io_stack);
+
+	rfsock->stack_watch = id;
+
 	DBG("real_sock %d fd %d hal_fd %d", rfsock->real_sock, rfsock->fd,
 								hal_fd);
 
-- 
1.8.3.2


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] android/tester: Add Socket test close and listen
  2013-12-18 15:15 [PATCH 1/2] android/tester: Add Socket test close and listen Andrei Emeltchenko
  2013-12-18 15:15 ` [PATCH 2/2] android/socket: Handle Android events for server socket Andrei Emeltchenko
@ 2013-12-19  8:18 ` Johan Hedberg
  2013-12-19  8:51 ` [PATCHv2 " Andrei Emeltchenko
  2 siblings, 0 replies; 6+ messages in thread
From: Johan Hedberg @ 2013-12-19  8:18 UTC (permalink / raw)
  To: Andrei Emeltchenko; +Cc: linux-bluetooth

Hi Andrei,

On Wed, Dec 18, 2013, Andrei Emeltchenko wrote:
> This test the situation when Android close file descriptor we passed to
> it and try to listen() again.
> ---
>  android/android-tester.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 55 insertions(+)
> 
> diff --git a/android/android-tester.c b/android/android-tester.c
> index 8a2861e..cb8fb4e 100644
> --- a/android/android-tester.c
> +++ b/android/android-tester.c
> @@ -1141,6 +1141,57 @@ clean:
>  		close(sock_fd);
>  }
>  
> +static void test_listen_close(const void *test_data)
> +{
> +	struct test_data *data = tester_get_data();
> +	const struct socket_data *test = data->test_data;
> +	bt_status_t status;
> +	int sock_fd = -1;
> +
> +	status = data->if_sock->listen(test->sock_type,
> +					test->service_name, test->service_uuid,
> +					test->channel, &sock_fd, test->flags);
> +	if (status != test->expected_status) {
> +		tester_test_failed();
> +		goto clean;
> +	}
> +
> +	/* Check that file descriptor is valid */
> +	if (status == BT_STATUS_SUCCESS && fcntl(sock_fd, F_GETFD) == -1) {

We usually check for failures like this with < 0 instead of == -1.

> +	status = data->if_sock->listen(test->sock_type,
> +					test->service_name, test->service_uuid,
> +					test->channel, &sock_fd, test->flags);
> +	if (status != test->expected_status) {
> +		tester_test_failed();
> +		goto clean;
> +	}

You've got many different conditions that can trigger
tester_test_failed but no tester_warn() logs for each one to make
debugging easy if these new tests start failing. Please add those.
Otherwise once there's a regression it's going to be a real pain for you
to track down exactly what's broken.

Johan

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCHv2 1/2] android/tester: Add Socket test close and listen
  2013-12-18 15:15 [PATCH 1/2] android/tester: Add Socket test close and listen Andrei Emeltchenko
  2013-12-18 15:15 ` [PATCH 2/2] android/socket: Handle Android events for server socket Andrei Emeltchenko
  2013-12-19  8:18 ` [PATCH 1/2] android/tester: Add Socket test close and listen Johan Hedberg
@ 2013-12-19  8:51 ` Andrei Emeltchenko
  2013-12-19  8:51   ` [PATCHv2 2/2] android/socket: Handle Android events for server socket Andrei Emeltchenko
  2013-12-19  9:09   ` [PATCHv2 1/2] android/tester: Add Socket test close and listen Johan Hedberg
  2 siblings, 2 replies; 6+ messages in thread
From: Andrei Emeltchenko @ 2013-12-19  8:51 UTC (permalink / raw)
  To: linux-bluetooth

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

This test the situation when Android close file descriptor we passed to
it and try to listen() again.
---
 android/android-tester.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 60 insertions(+)

diff --git a/android/android-tester.c b/android/android-tester.c
index 8a2861e..c24f5a6 100644
--- a/android/android-tester.c
+++ b/android/android-tester.c
@@ -1141,6 +1141,62 @@ clean:
 		close(sock_fd);
 }
 
+static void test_listen_close(const void *test_data)
+{
+	struct test_data *data = tester_get_data();
+	const struct socket_data *test = data->test_data;
+	bt_status_t status;
+	int sock_fd = -1;
+
+	status = data->if_sock->listen(test->sock_type,
+					test->service_name, test->service_uuid,
+					test->channel, &sock_fd, test->flags);
+	if (status != test->expected_status) {
+		tester_warn("sock->listen() failed");
+		tester_test_failed();
+		goto clean;
+	}
+
+	/* Check that file descriptor is valid */
+	if (status == BT_STATUS_SUCCESS && fcntl(sock_fd, F_GETFD) < 0) {
+		tester_warn("sock_fd %d is not valid", sock_fd);
+		tester_test_failed();
+		return;
+	}
+
+	tester_print("Got valid sock_fd: %d", sock_fd);
+
+	/* Now close sock_fd */
+	close(sock_fd);
+	sock_fd = -1;
+
+	/* Try to listen again */
+	status = data->if_sock->listen(test->sock_type,
+					test->service_name, test->service_uuid,
+					test->channel, &sock_fd, test->flags);
+	if (status != test->expected_status) {
+		tester_warn("sock->listen() failed");
+		tester_test_failed();
+		goto clean;
+	}
+
+	/* Check that file descriptor is valid */
+	if (status == BT_STATUS_SUCCESS && fcntl(sock_fd, F_GETFD) < 0) {
+		tester_warn("sock_fd %d is not valid", sock_fd);
+		tester_test_failed();
+		return;
+	}
+
+	tester_print("Got valid sock_fd: %d", sock_fd);
+
+	tester_test_passed();
+
+clean:
+	if (sock_fd >= 0)
+		close(sock_fd);
+
+}
+
 static void test_generic_connect(const void *test_data)
 {
 	struct test_data *data = tester_get_data();
@@ -1363,6 +1419,10 @@ int main(int argc, char *argv[])
 			&btsock_success_check_chan,
 			setup_socket_interface, test_generic_listen, teardown);
 
+	test_bredrle("Socket Listen - Close and Listen again",
+			&btsock_success_check_chan,
+			setup_socket_interface, test_listen_close, teardown);
+
 	test_bredrle("Socket Connect - Invalid: sock_type 0",
 			&btsock_inv_param_socktype, setup_socket_interface,
 			test_generic_connect, teardown);
-- 
1.8.3.2


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCHv2 2/2] android/socket: Handle Android events for server socket
  2013-12-19  8:51 ` [PATCHv2 " Andrei Emeltchenko
@ 2013-12-19  8:51   ` Andrei Emeltchenko
  2013-12-19  9:09   ` [PATCHv2 1/2] android/tester: Add Socket test close and listen Johan Hedberg
  1 sibling, 0 replies; 6+ messages in thread
From: Andrei Emeltchenko @ 2013-12-19  8:51 UTC (permalink / raw)
  To: linux-bluetooth

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

Add watch for tracking events from Android framework for server socket.
Android might want to close server connection, in this case we close
our listening socket and cleaning up rfsock structure. Glib watch is
added with high priority to avoid races.
---
 android/socket.c | 32 +++++++++++++++++++++++++++++++-
 1 file changed, 31 insertions(+), 1 deletion(-)

diff --git a/android/socket.c b/android/socket.c
index 94a44fc..5a03a80 100644
--- a/android/socket.c
+++ b/android/socket.c
@@ -622,6 +622,24 @@ static bool sock_send_accept(struct rfcomm_sock *rfsock, bdaddr_t *bdaddr,
 	return true;
 }
 
+static gboolean sock_server_stack_event_cb(GIOChannel *io, GIOCondition cond,
+								gpointer data)
+{
+	struct rfcomm_sock *rfsock = data;
+
+	DBG("sock %d cond %d", g_io_channel_unix_get_fd(io), cond);
+
+	if (cond & G_IO_NVAL)
+		return FALSE;
+
+	if (cond & (G_IO_ERR | G_IO_HUP )) {
+		servers = g_list_remove(servers, rfsock);
+		cleanup_rfsock(rfsock);
+	}
+
+	return FALSE;
+}
+
 static void accept_cb(GIOChannel *io, GError *err, gpointer user_data)
 {
 	struct rfcomm_sock *rfsock = user_data;
@@ -697,10 +715,12 @@ static void handle_listen(const void *buf, uint16_t len)
 	const struct profile_info *profile;
 	struct rfcomm_sock *rfsock = NULL;
 	BtIOSecLevel sec_level;
-	GIOChannel *io;
+	GIOChannel *io, *io_stack;
+	GIOCondition cond;
 	GError *err = NULL;
 	int hal_fd = -1;
 	int chan;
+	guint id;
 
 	DBG("");
 
@@ -738,6 +758,16 @@ static void handle_listen(const void *buf, uint16_t len)
 	g_io_channel_set_close_on_unref(io, FALSE);
 	g_io_channel_unref(io);
 
+	/* Handle events from Android */
+	cond = G_IO_HUP | G_IO_ERR | G_IO_NVAL;
+	io_stack = g_io_channel_unix_new(rfsock->fd);
+	id = g_io_add_watch_full(io_stack, G_PRIORITY_HIGH, cond,
+					sock_server_stack_event_cb, rfsock,
+					NULL);
+	g_io_channel_unref(io_stack);
+
+	rfsock->stack_watch = id;
+
 	DBG("real_sock %d fd %d hal_fd %d", rfsock->real_sock, rfsock->fd,
 								hal_fd);
 
-- 
1.8.3.2


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCHv2 1/2] android/tester: Add Socket test close and listen
  2013-12-19  8:51 ` [PATCHv2 " Andrei Emeltchenko
  2013-12-19  8:51   ` [PATCHv2 2/2] android/socket: Handle Android events for server socket Andrei Emeltchenko
@ 2013-12-19  9:09   ` Johan Hedberg
  1 sibling, 0 replies; 6+ messages in thread
From: Johan Hedberg @ 2013-12-19  9:09 UTC (permalink / raw)
  To: Andrei Emeltchenko; +Cc: linux-bluetooth

Hi Andrei,

On Thu, Dec 19, 2013, Andrei Emeltchenko wrote:
> This test the situation when Android close file descriptor we passed to
> it and try to listen() again.
> ---
>  android/android-tester.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 60 insertions(+)

Both patches have been applied. Thanks.

Johan

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2013-12-19  9:09 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-12-18 15:15 [PATCH 1/2] android/tester: Add Socket test close and listen Andrei Emeltchenko
2013-12-18 15:15 ` [PATCH 2/2] android/socket: Handle Android events for server socket Andrei Emeltchenko
2013-12-19  8:18 ` [PATCH 1/2] android/tester: Add Socket test close and listen Johan Hedberg
2013-12-19  8:51 ` [PATCHv2 " Andrei Emeltchenko
2013-12-19  8:51   ` [PATCHv2 2/2] android/socket: Handle Android events for server socket Andrei Emeltchenko
2013-12-19  9:09   ` [PATCHv2 1/2] android/tester: Add Socket test close and listen Johan Hedberg

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).