2014-10-28 09:55:19

by Marcin Kraglak

[permalink] [raw]
Subject: [PATCH 1/7] shared/io: Remove disconnect handler in io_destroy

Diconnect callback may be called after destroying IO in next
mainloop iteration (it leada to reading freed data). Removing
disconnect handler prevents that situation.
---
src/shared/io-glib.c | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/src/shared/io-glib.c b/src/shared/io-glib.c
index a2ada66..5ebde3d 100644
--- a/src/shared/io-glib.c
+++ b/src/shared/io-glib.c
@@ -115,6 +115,11 @@ void io_destroy(struct io *io)
io->write_watch = 0;
}

+ if (io->disconnect_watch > 0) {
+ g_source_remove(io->disconnect_watch);
+ io->disconnect_watch = 0;
+ }
+
g_io_channel_unref(io->channel);
io->channel = NULL;

--
1.9.3



2014-10-28 11:41:05

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH 2/7] unit/test-gatt: Add search all primary services test case

Hi Marcin,

On Tue, Oct 28, 2014 at 11:55 AM, Marcin Kraglak
<[email protected]> wrote:
> ---
> unit/test-gatt.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 78 insertions(+)
>
> diff --git a/unit/test-gatt.c b/unit/test-gatt.c
> index bbbf9a5..57cb109 100644
> --- a/unit/test-gatt.c
> +++ b/unit/test-gatt.c
> @@ -35,8 +35,10 @@
>
> #include <glib.h>
>
> +#include "lib/uuid.h"
> #include "src/shared/util.h"
> #include "src/shared/att.h"
> +#include "src/shared/gatt-helpers.h"
> #include "src/shared/gatt-client.h"
>
> struct test_pdu {
> @@ -53,6 +55,7 @@ struct test_data {
> struct context {
> GMainLoop *main_loop;
> struct bt_gatt_client *client;
> + struct bt_att *att;
> guint source;
> guint process;
> int fd;
> @@ -222,6 +225,53 @@ static struct context *create_context(uint16_t mtu, gconstpointer data)
> return context;
> }
>
> +static void primary_cb(bool success, uint8_t att_ecode,
> + struct bt_gatt_result *result,
> + void *user_data)
> +{
> + struct context *context = user_data;
> +
> + g_assert(success);
> +
> + context_quit(context);
> +}
> +
> +static struct context *create_helpers_context(uint16_t mtu, gconstpointer data)
> +{
> + struct context *context = g_new0(struct context, 1);
> + GIOChannel *channel;
> + int err, sv[2];
> +
> + context->main_loop = g_main_loop_new(NULL, FALSE);
> + g_assert(context->main_loop);
> +
> + err = socketpair(AF_UNIX, SOCK_SEQPACKET | SOCK_CLOEXEC, 0, sv);
> + g_assert(err == 0);
> +
> + context->att = bt_att_new(sv[0]);
> + g_assert(context->att);
> +
> + channel = g_io_channel_unix_new(sv[1]);
> +
> + g_io_channel_set_close_on_unref(channel, TRUE);
> + g_io_channel_set_encoding(channel, NULL, NULL);
> + g_io_channel_set_buffered(channel, FALSE);
> +
> + context->source = g_io_add_watch(channel,
> + G_IO_IN | G_IO_HUP | G_IO_ERR | G_IO_NVAL,
> + test_handler, context);
> + g_assert(context->source > 0);
> +
> + g_io_channel_unref(channel);
> +
> + context->fd = sv[1];
> + context->data = data;
> +
> + bt_gatt_exchange_mtu(context->att, mtu, NULL, NULL, NULL);
> +
> + return context;
> +}

Another function to create context? I looks awful similar to
create_context, perhaps you want a create_client_context and
create_server_context wrappers that calls create_context or even
better add a parameter to create_context to say what role we will act.

> static void destroy_context(struct context *context)
> {
> if (context->source > 0)
> @@ -229,6 +279,8 @@ static void destroy_context(struct context *context)
>
> bt_gatt_client_unref(context->client);
>
> + bt_att_unref(context->att);
> +
> g_main_loop_unref(context->main_loop);
>
> test_free(context->data);
> @@ -249,6 +301,16 @@ static void test_client(gconstpointer data)
> execute_context(context);
> }
>
> +static void test_search_primary(gconstpointer data)
> +{
> + struct context *context = create_helpers_context(512, data);
> +
> + bt_gatt_discover_all_primary_services(context->att, NULL, primary_cb,
> + context, NULL);
> +
> + execute_context(context);
> +}
> +
> int main(int argc, char *argv[])
> {
> g_test_init(&argc, &argv, NULL);
> @@ -262,5 +324,21 @@ int main(int argc, char *argv[])
> define_test("/TP/GAC/CL/BV-01-C", test_client,
> raw_pdu(0x02, 0x00, 0x02));
>
> + /* Discover All Primary Services */
> + define_test("/TP/GAD/CL/BV-01-C", test_search_primary,
> + raw_pdu(0x02, 0x00, 0x02),
> + raw_pdu(0x03, 0x00, 0x02),
> + raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28),
> + raw_pdu(0x11, 0x06, 0x10, 0x00, 0x13, 0x00, 0x00, 0x18,
> + 0x20, 0x00, 0x29, 0x00, 0xb0, 0x68,
> + 0x30, 0x00, 0x32, 0x00, 0x19, 0x18),
> + raw_pdu(0x10, 0x33, 0x00, 0xff, 0xff, 0x00, 0x28),
> + raw_pdu(0x11, 0x14, 0x90, 0x00, 0x96, 0x00, 0xef, 0xcd,
> + 0xab, 0x89, 0x67, 0x45, 0x23, 0x01,
> + 0x00, 0x00, 0x00, 0x00, 0x85, 0x60,
> + 0x00, 0x00),
> + raw_pdu(0x10, 0x97, 0x00, 0xff, 0xff, 0x00, 0x28),
> + raw_pdu(0x01, 0x10, 0x97, 0x00, 0x0a));
> +
> return g_test_run();
> }
> --
> 1.9.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html



--
Luiz Augusto von Dentz

2014-10-28 09:55:25

by Marcin Kraglak

[permalink] [raw]
Subject: [PATCH 7/7] unit/test-gatt: Add search characteristics test case

---
unit/test-gatt.c | 37 +++++++++++++++++++++++++++++++++++++
1 file changed, 37 insertions(+)

diff --git a/unit/test-gatt.c b/unit/test-gatt.c
index c744f2d..70ae54b 100644
--- a/unit/test-gatt.c
+++ b/unit/test-gatt.c
@@ -236,6 +236,17 @@ static void primary_cb(bool success, uint8_t att_ecode,
context_quit(context);
}

+static void characteristic_cb(bool success, uint8_t att_ecode,
+ struct bt_gatt_result *result,
+ void *user_data)
+{
+ struct context *context = user_data;
+
+ g_assert(success);
+
+ context_quit(context);
+}
+
static void included_cb(bool success, uint8_t att_ecode,
struct bt_gatt_result *result,
void *user_data)
@@ -358,6 +369,17 @@ static void test_search_included(gconstpointer data)
execute_context(context);
}

+static void test_search_characteristics(gconstpointer data)
+{
+ struct context *context = create_helpers_context(512, data);
+
+ g_assert(bt_gatt_discover_characteristics(context->att, 0x0010, 0x0020,
+ characteristic_cb, context,
+ NULL));
+
+ execute_context(context);
+}
+
int main(int argc, char *argv[])
{
g_test_init(&argc, &argv, NULL);
@@ -437,5 +459,20 @@ int main(int argc, char *argv[])
raw_pdu(0x08, 0x06, 0x00, 0xff, 0xff, 0x02, 0x28),
raw_pdu(0x01, 0x08, 0x06, 0x00, 0x0a));

+ /* Discover all Characteristics in given range */
+ define_test("/TP/GAD/CL/BV-04-C", test_search_characteristics,
+ raw_pdu(0x02, 0x00, 0x02),
+ raw_pdu(0x03, 0x00, 0x02),
+ raw_pdu(0x08, 0x10, 0x00, 0x20, 0x00, 0x03, 0x28),
+ raw_pdu(0x09, 0x07, 0x11, 0x00, 02, 0x12, 0x00, 0x25,
+ 0x2a),
+ raw_pdu(0x08, 0x12, 0x00, 0x20, 0x00, 0x03, 0x28),
+ raw_pdu(0x09, 0x15, 0x13, 0x00, 0x02, 0x14, 0x00, 0x85,
+ 0x00, 0xef, 0xcd, 0xab, 0x89, 0x67,
+ 0x45, 0x23, 0x01, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00),
+ raw_pdu(0x08, 0x14, 0x00, 0x20, 0x00, 0x03, 0x28),
+ raw_pdu(0x01, 0x08, 0x12, 0x00, 0x0a));
+
return g_test_run();
}
--
1.9.3


2014-10-28 09:55:24

by Marcin Kraglak

[permalink] [raw]
Subject: [PATCH 6/7] unit/test-gatt: Add search included services test case

---
unit/test-gatt.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 45 insertions(+)

diff --git a/unit/test-gatt.c b/unit/test-gatt.c
index fef0757..c744f2d 100644
--- a/unit/test-gatt.c
+++ b/unit/test-gatt.c
@@ -236,6 +236,17 @@ static void primary_cb(bool success, uint8_t att_ecode,
context_quit(context);
}

+static void included_cb(bool success, uint8_t att_ecode,
+ struct bt_gatt_result *result,
+ void *user_data)
+{
+ struct context *context = user_data;
+
+ g_assert(success);
+
+ context_quit(context);
+}
+
static struct context *create_helpers_context(uint16_t mtu, gconstpointer data)
{
struct context *context = g_new0(struct context, 1);
@@ -337,6 +348,16 @@ static void test_search_primary_by_uuid_128(gconstpointer data)
execute_context(context);
}

+static void test_search_included(gconstpointer data)
+{
+ struct context *context = create_helpers_context(512, data);
+
+ bt_gatt_discover_included_services(context->att, 0x0001, 0xffff,
+ included_cb, context, NULL);
+
+ execute_context(context);
+}
+
int main(int argc, char *argv[])
{
g_test_init(&argc, &argv, NULL);
@@ -392,5 +413,29 @@ int main(int argc, char *argv[])
0x18, 0x00, 0x00),
raw_pdu(0x01, 0x06, 0x08, 0x00, 0x0a));

+ /* Discover All Include Services */
+ define_test("/TP/GAD/CL/BV-03-C", test_search_included,
+ raw_pdu(0x02, 0x00, 0x02),
+ raw_pdu(0x03, 0x00, 0x02),
+ raw_pdu(0x08, 0x01, 0x00, 0xff, 0xff, 0x02, 0x28),
+ raw_pdu(0x09, 0x08, 0x02, 0x00, 0x10, 0x00, 0x1f, 0x00,
+ 0x0f, 0x18),
+ raw_pdu(0x08, 0x03, 0x00, 0xff, 0xff, 0x02, 0x28),
+ raw_pdu(0x09, 0x06, 0x03, 0x00, 0x20, 0x00, 0x2f, 0x00,
+ 0x04, 0x00, 0x30, 0x00, 0x3f, 0x00),
+ raw_pdu(0x0a, 0x20, 0x00),
+ raw_pdu(0x0b, 0x00, 0x00, 0x3e, 0x39, 0x00, 0x00, 0x00,
+ 0x00, 0x01, 0x23, 0x45, 0x67, 0x89,
+ 0xab, 0xcd, 0xef),
+ raw_pdu(0x0a, 0x30, 0x00),
+ raw_pdu(0x0b, 0x00, 0x00, 0x3b, 0x39, 0x00, 0x00, 0x00,
+ 0x00, 0x01, 0x23, 0x45, 0x67, 0x89,
+ 0xab, 0xcd, 0xef),
+ raw_pdu(0x08, 0x05, 0x00, 0xff, 0xff, 0x02, 0x28),
+ raw_pdu(0x09, 0x08, 0x05, 0x00, 0x40, 0x00, 0x4f, 0x00,
+ 0x0a, 0x18),
+ raw_pdu(0x08, 0x06, 0x00, 0xff, 0xff, 0x02, 0x28),
+ raw_pdu(0x01, 0x08, 0x06, 0x00, 0x0a));
+
return g_test_run();
}
--
1.9.3


2014-10-28 09:55:22

by Marcin Kraglak

[permalink] [raw]
Subject: [PATCH 4/7] shared/gatt: Fix incorrect data read

---
src/shared/gatt-helpers.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/src/shared/gatt-helpers.c b/src/shared/gatt-helpers.c
index d751d5a..6e19066 100644
--- a/src/shared/gatt-helpers.c
+++ b/src/shared/gatt-helpers.c
@@ -686,7 +686,13 @@ static void find_by_type_val_cb(uint8_t opcode, const void *pdu,
op->result_tail = cur_result;
}

- last_end = get_le16(pdu + length - 6);
+ /*
+ * Each data set contains:
+ * 2 octets with start handle
+ * 2 octets with end handle
+ * last_end is end handle of last data set
+ */
+ last_end = get_le16(pdu + length - 2);
if (last_end < op->end_handle) {
uint8_t pdu[6 + get_uuid_len(&op->uuid)];

--
1.9.3


2014-10-28 09:55:23

by Marcin Kraglak

[permalink] [raw]
Subject: [PATCH 5/7] unit/test-gatt: Add search primary by 128 bit UUID test case

---
unit/test-gatt.c | 28 ++++++++++++++++++++++++++++
1 file changed, 28 insertions(+)

diff --git a/unit/test-gatt.c b/unit/test-gatt.c
index cc9f34e..fef0757 100644
--- a/unit/test-gatt.c
+++ b/unit/test-gatt.c
@@ -324,6 +324,19 @@ static void test_search_primary_by_uuid_16(gconstpointer data)
execute_context(context);
}

+static void test_search_primary_by_uuid_128(gconstpointer data)
+{
+ struct context *context = create_helpers_context(512, data);
+ bt_uuid_t uuid;
+
+ bt_string_to_uuid(&uuid, HEART_RATE_UUID);
+
+ bt_gatt_discover_all_primary_services(context->att, &uuid, primary_cb,
+ context, NULL);
+
+ execute_context(context);
+}
+
int main(int argc, char *argv[])
{
g_test_init(&argc, &argv, NULL);
@@ -364,5 +377,20 @@ int main(int argc, char *argv[])
0x18),
raw_pdu(0x01, 0x06, 0x08, 0x00, 0x0a));

+ /* Discover Primary Services By 128 bit UUID*/
+ define_test("/TP/GAD/CL/BV-02-C-2", test_search_primary_by_uuid_128,
+ raw_pdu(0x02, 0x00, 0x02),
+ raw_pdu(0x03, 0x00, 0x02),
+ raw_pdu(06, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28, 0xfb,
+ 0x34, 0x9b, 0x5f, 0x80, 0x00, 0x00,
+ 0x80, 0x00, 0x10, 0x00, 0x00, 0x0d,
+ 0x18, 0x00, 0x00),
+ raw_pdu(0x07, 0x10, 0x00, 0x17, 0x00),
+ raw_pdu(06, 0x18, 0x00, 0xff, 0xff, 0x00, 0x28, 0xfb,
+ 0x34, 0x9b, 0x5f, 0x80, 0x00, 0x00,
+ 0x80, 0x00, 0x10, 0x00, 0x00, 0x0d,
+ 0x18, 0x00, 0x00),
+ raw_pdu(0x01, 0x06, 0x08, 0x00, 0x0a));
+
return g_test_run();
}
--
1.9.3


2014-10-28 09:55:21

by Marcin Kraglak

[permalink] [raw]
Subject: [PATCH 3/7] unit/test-gatt: Add search primary by 16 but UUID test case

---
unit/test-gatt.c | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)

diff --git a/unit/test-gatt.c b/unit/test-gatt.c
index 57cb109..cc9f34e 100644
--- a/unit/test-gatt.c
+++ b/unit/test-gatt.c
@@ -311,6 +311,19 @@ static void test_search_primary(gconstpointer data)
execute_context(context);
}

+static void test_search_primary_by_uuid_16(gconstpointer data)
+{
+ struct context *context = create_helpers_context(512, data);
+ bt_uuid_t uuid;
+
+ bt_uuid16_create(&uuid, 0x1800);
+
+ bt_gatt_discover_all_primary_services(context->att, &uuid, primary_cb,
+ context, NULL);
+
+ execute_context(context);
+}
+
int main(int argc, char *argv[])
{
g_test_init(&argc, &argv, NULL);
@@ -340,5 +353,16 @@ int main(int argc, char *argv[])
raw_pdu(0x10, 0x97, 0x00, 0xff, 0xff, 0x00, 0x28),
raw_pdu(0x01, 0x10, 0x97, 0x00, 0x0a));

+ /* Discover Primary Services By 16 bit UUID*/
+ define_test("/TP/GAD/CL/BV-02-C-1", test_search_primary_by_uuid_16,
+ raw_pdu(0x02, 0x00, 0x02),
+ raw_pdu(0x03, 0x00, 0x02),
+ raw_pdu(0x06, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28, 0x00,
+ 0x18),
+ raw_pdu(0x07, 0x01, 0x00, 0x07, 0x00),
+ raw_pdu(0x06, 0x08, 0x00, 0xff, 0xff, 0x00, 0x28, 0x00,
+ 0x18),
+ raw_pdu(0x01, 0x06, 0x08, 0x00, 0x0a));
+
return g_test_run();
}
--
1.9.3


2014-10-28 09:55:20

by Marcin Kraglak

[permalink] [raw]
Subject: [PATCH 2/7] unit/test-gatt: Add search all primary services test case

---
unit/test-gatt.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 78 insertions(+)

diff --git a/unit/test-gatt.c b/unit/test-gatt.c
index bbbf9a5..57cb109 100644
--- a/unit/test-gatt.c
+++ b/unit/test-gatt.c
@@ -35,8 +35,10 @@

#include <glib.h>

+#include "lib/uuid.h"
#include "src/shared/util.h"
#include "src/shared/att.h"
+#include "src/shared/gatt-helpers.h"
#include "src/shared/gatt-client.h"

struct test_pdu {
@@ -53,6 +55,7 @@ struct test_data {
struct context {
GMainLoop *main_loop;
struct bt_gatt_client *client;
+ struct bt_att *att;
guint source;
guint process;
int fd;
@@ -222,6 +225,53 @@ static struct context *create_context(uint16_t mtu, gconstpointer data)
return context;
}

+static void primary_cb(bool success, uint8_t att_ecode,
+ struct bt_gatt_result *result,
+ void *user_data)
+{
+ struct context *context = user_data;
+
+ g_assert(success);
+
+ context_quit(context);
+}
+
+static struct context *create_helpers_context(uint16_t mtu, gconstpointer data)
+{
+ struct context *context = g_new0(struct context, 1);
+ GIOChannel *channel;
+ int err, sv[2];
+
+ context->main_loop = g_main_loop_new(NULL, FALSE);
+ g_assert(context->main_loop);
+
+ err = socketpair(AF_UNIX, SOCK_SEQPACKET | SOCK_CLOEXEC, 0, sv);
+ g_assert(err == 0);
+
+ context->att = bt_att_new(sv[0]);
+ g_assert(context->att);
+
+ channel = g_io_channel_unix_new(sv[1]);
+
+ g_io_channel_set_close_on_unref(channel, TRUE);
+ g_io_channel_set_encoding(channel, NULL, NULL);
+ g_io_channel_set_buffered(channel, FALSE);
+
+ context->source = g_io_add_watch(channel,
+ G_IO_IN | G_IO_HUP | G_IO_ERR | G_IO_NVAL,
+ test_handler, context);
+ g_assert(context->source > 0);
+
+ g_io_channel_unref(channel);
+
+ context->fd = sv[1];
+ context->data = data;
+
+ bt_gatt_exchange_mtu(context->att, mtu, NULL, NULL, NULL);
+
+ return context;
+}
+
static void destroy_context(struct context *context)
{
if (context->source > 0)
@@ -229,6 +279,8 @@ static void destroy_context(struct context *context)

bt_gatt_client_unref(context->client);

+ bt_att_unref(context->att);
+
g_main_loop_unref(context->main_loop);

test_free(context->data);
@@ -249,6 +301,16 @@ static void test_client(gconstpointer data)
execute_context(context);
}

+static void test_search_primary(gconstpointer data)
+{
+ struct context *context = create_helpers_context(512, data);
+
+ bt_gatt_discover_all_primary_services(context->att, NULL, primary_cb,
+ context, NULL);
+
+ execute_context(context);
+}
+
int main(int argc, char *argv[])
{
g_test_init(&argc, &argv, NULL);
@@ -262,5 +324,21 @@ int main(int argc, char *argv[])
define_test("/TP/GAC/CL/BV-01-C", test_client,
raw_pdu(0x02, 0x00, 0x02));

+ /* Discover All Primary Services */
+ define_test("/TP/GAD/CL/BV-01-C", test_search_primary,
+ raw_pdu(0x02, 0x00, 0x02),
+ raw_pdu(0x03, 0x00, 0x02),
+ raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28),
+ raw_pdu(0x11, 0x06, 0x10, 0x00, 0x13, 0x00, 0x00, 0x18,
+ 0x20, 0x00, 0x29, 0x00, 0xb0, 0x68,
+ 0x30, 0x00, 0x32, 0x00, 0x19, 0x18),
+ raw_pdu(0x10, 0x33, 0x00, 0xff, 0xff, 0x00, 0x28),
+ raw_pdu(0x11, 0x14, 0x90, 0x00, 0x96, 0x00, 0xef, 0xcd,
+ 0xab, 0x89, 0x67, 0x45, 0x23, 0x01,
+ 0x00, 0x00, 0x00, 0x00, 0x85, 0x60,
+ 0x00, 0x00),
+ raw_pdu(0x10, 0x97, 0x00, 0xff, 0xff, 0x00, 0x28),
+ raw_pdu(0x01, 0x10, 0x97, 0x00, 0x0a));
+
return g_test_run();
}
--
1.9.3