[PATCH BlueZ v1 0/2] shared/gatt: Bug fixes.

[PATCH BlueZ v1 0/2] shared/gatt: Bug fixes.

[Arman Uguray]

*v1: Call io_shutdown instead of close(att->fd).

Arman Uguray (2):
  shared/att: Call io_shutdown on ATT violations.
  shared/gatt-client: Call ready_handler only in init.

 src/shared/att.c         | 21 +++++++++++++--------
 src/shared/gatt-client.c |  3 ++-
 2 files changed, 15 insertions(+), 9 deletions(-)

-- 
2.1.0.rc2.206.gedb03e5

[PATCH BlueZ v1 1/2] shared/att: Call io_shutdown on ATT violations.

[Arman Uguray]

The existing code handles ATT protocol request timeouts and invalid
incoming request PDUs by cleaning up the io structure via io_destroy.
This doesn't always work, since this only terminates the connection if
io_set_close_on_destroy has been set. Calling io_destroy to force a
disconnect also has the added side-effect of not invoking the disconnect
callback registered with struct io.

This patch fixes these by calling io_shutdown on the io instead when
required by ATT protocol specification. This both cleans up the
connection regardless of whether or not close_on_unref has been set, and
it triggers the disconnect callback via an EPOLLHUP event.
---
 src/shared/att.c | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/src/shared/att.c b/src/shared/att.c
index 2bc7682..bc01827 100644
--- a/src/shared/att.c
+++ b/src/shared/att.c
@@ -416,9 +416,6 @@ static bool timeout_cb(void *user_data)
 	if (!op)
 		return false;
 
-	io_destroy(att->io);
-	att->io = NULL;
-
 	util_debug(att->debug_callback, att->debug_data,
 				"Operation timed out: 0x%02x", op->opcode);
 
@@ -428,6 +425,13 @@ static bool timeout_cb(void *user_data)
 	op->timeout_id = 0;
 	destroy_att_send_op(op);
 
+	/*
+	 * Directly terminate the connection as required by the ATT protocol.
+	 * This should trigger an io disconnect event which will clean up the
+	 * io and notify the upper layer.
+	 */
+	io_shutdown(att->io);
+
 	return false;
 }
 
@@ -579,7 +583,7 @@ static void handle_rsp(struct bt_att *att, uint8_t opcode, uint8_t *pdu,
 	if (!op) {
 		util_debug(att->debug_callback, att->debug_data,
 					"Received unexpected ATT response");
-		disconnect_cb(att->io, att);
+		io_shutdown(att->io);
 		return;
 	}
 
@@ -634,7 +638,7 @@ static void handle_conf(struct bt_att *att, uint8_t *pdu, ssize_t pdu_len)
 	if (!op || pdu_len) {
 		util_debug(att->debug_callback, att->debug_data,
 				"Received unexpected/invalid ATT confirmation");
-		disconnect_cb(att->io, att);
+		io_shutdown(att->io);
 		return;
 	}
 
@@ -765,14 +769,15 @@ static bool can_read_data(struct io *io, void *user_data)
 	case ATT_OP_TYPE_REQ:
 		/*
 		 * If a request is currently pending, then the sequential
-		 * protocol was violated. Disconnect the bearer and notify the
-		 * upper-layer.
+		 * protocol was violated. Disconnect the bearer, which will
+		 * promptly notify the upper layer via disconnect handlers.
 		 */
 		if (att->in_req) {
 			util_debug(att->debug_callback, att->debug_data,
 					"Received request while another is "
 					"pending: 0x%02x", opcode);
-			disconnect_cb(att->io, att);
+			io_shutdown(att->io);
+
 			return false;
 		}
 
-- 
2.1.0.rc2.206.gedb03e5

[PATCH BlueZ v1 2/2] shared/gatt-client: Call ready_handler only in init.

[Arman Uguray]

Currently disconnect_cb calls ready_handler to notify the upper layer
if a disconnect occurred during initialization. This patch fixes it so
that the handler is only called if in_init is set.
---
 src/shared/gatt-client.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c
index 4a645eb..033cba1 100644
--- a/src/shared/gatt-client.c
+++ b/src/shared/gatt-client.c
@@ -1455,6 +1455,7 @@ static void bt_gatt_client_free(struct bt_gatt_client *client)
 static void att_disconnect_cb(void *user_data)
 {
 	struct bt_gatt_client *client = user_data;
+	bool in_init = client->in_init;
 
 	client->disc_id = 0;
 
@@ -1464,7 +1465,7 @@ static void att_disconnect_cb(void *user_data)
 	client->in_init = false;
 	client->ready = false;
 
-	if (client->ready_callback)
+	if (in_init && client->ready_callback)
 		client->ready_callback(false, 0, client->ready_data);
 }
 
-- 
2.1.0.rc2.206.gedb03e5

Re: [PATCH BlueZ v1 0/2] shared/gatt: Bug fixes.

[Luiz Augusto von Dentz]

Hi Arman,

On Tue, Nov 25, 2014 at 1:03 AM, Arman Uguray <armansito@chromium.org> wrote:
> *v1: Call io_shutdown instead of close(att->fd).
>
> Arman Uguray (2):
>   shared/att: Call io_shutdown on ATT violations.
>   shared/gatt-client: Call ready_handler only in init.
>
>  src/shared/att.c         | 21 +++++++++++++--------
>  src/shared/gatt-client.c |  3 ++-
>  2 files changed, 15 insertions(+), 9 deletions(-)
>
> --
> 2.1.0.rc2.206.gedb03e5

Applied, thanks.


-- 
Luiz Augusto von Dentz