2014-11-24 23:03:50

by Arman Uguray

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

*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

2014-11-25 21:51:05

by Luiz Augusto von Dentz

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

Hi Arman,

On Tue, Nov 25, 2014 at 1:03 AM, Arman Uguray <[email protected]> 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

2014-11-24 23:03:52

by Arman Uguray

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

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

2014-11-24 23:03:51

by Arman Uguray

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

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