All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH BlueZ v5 0/8] Unit tests for gattrib API
@ 2014-10-31 18:14 Michael Janssen
  2014-10-31 18:14 ` [PATCH BlueZ v5 1/8] attrib: Add mtu argument to g_attrib_new Michael Janssen
                   ` (8 more replies)
  0 siblings, 9 replies; 10+ messages in thread
From: Michael Janssen @ 2014-10-31 18:14 UTC (permalink / raw
  To: linux-bluetooth; +Cc: Michael Janssen

Since we want to replace GAttrib with a bt_att shim for a smooth transition 
to bt_att profiles, this patch cleans up the API and adds unit tests for
GAttrib functions.

This adds tests for all but the register / unregister functions, which
I will send in a separate patch since they are slightly more complicated due
to catchalls.

v1 -> v2:
  * Add g_attrib_register tests
  * Cleanup of unit/test-gattrib.c
  * Bugfix for GATTRIB_ALL_REQS

v2 -> v3:
  * Just move g_attrib_is_encrypted to static (Luiz)
  * Check that registered-for PDUs were delivered
  * Respond to Request PDU when registered for it.
  * Copy result PDUs since they aren't guaranteed to live beyond callback.

v3 -> v4:
  * Rework to use g_main_loop_run() for running
  * Reuse pdu_data struct in expect_response

v4 -> v5:
  * Rebase, fix compilation problems
  * Only init log on --verbose

Michael Janssen (8):
  attrib: Add mtu argument to g_attrib_new
  attrib: remove g_attrib_is_encrypted
  Add unit tests for gattrib
  attrib: Add unit tests for g_attrib_register
  attrib: fix GATTRIB_ALL_REQS behavior
  unit/gattrib: Check expected PDUs were delivered
  unit/gattrib: Respond to the request PDU.
  unit/gattrib: Only init log on --verbose

 .gitignore           |   1 +
 Makefile.am          |   7 +
 attrib/gattrib.c     |  44 ++--
 attrib/gattrib.h     |   4 +-
 attrib/gatttool.c    |  17 +-
 attrib/interactive.c |  17 +-
 src/attrib-server.c  |  12 ++
 src/device.c         |  11 +-
 unit/test-gattrib.c  | 576 +++++++++++++++++++++++++++++++++++++++++++++++++++
 9 files changed, 664 insertions(+), 25 deletions(-)
 create mode 100644 unit/test-gattrib.c

-- 
2.1.0.rc2.206.gedb03e5


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

* [PATCH BlueZ v5 1/8] attrib: Add mtu argument to g_attrib_new
  2014-10-31 18:14 [PATCH BlueZ v5 0/8] Unit tests for gattrib API Michael Janssen
@ 2014-10-31 18:14 ` Michael Janssen
  2014-10-31 18:14 ` [PATCH BlueZ v5 2/8] attrib: remove g_attrib_is_encrypted Michael Janssen
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Michael Janssen @ 2014-10-31 18:14 UTC (permalink / raw
  To: linux-bluetooth; +Cc: Michael Janssen

Instead of using the default MTU, use one passed
in by the user, and detect it from the channel when
it is created.
---
 attrib/gattrib.c     |  9 +++------
 attrib/gattrib.h     |  2 +-
 attrib/gatttool.c    | 17 ++++++++++++++++-
 attrib/interactive.c | 17 ++++++++++++++++-
 src/device.c         | 11 ++++++++++-
 5 files changed, 46 insertions(+), 10 deletions(-)

diff --git a/attrib/gattrib.c b/attrib/gattrib.c
index ba2f6cc..a6511a2 100644
--- a/attrib/gattrib.c
+++ b/attrib/gattrib.c
@@ -473,10 +473,9 @@ done:
 	return TRUE;
 }
 
-GAttrib *g_attrib_new(GIOChannel *io)
+GAttrib *g_attrib_new(GIOChannel *io, guint16 mtu)
 {
 	struct _GAttrib *attrib;
-	uint16_t att_mtu;
 
 	g_io_channel_set_encoding(io, NULL, NULL);
 	g_io_channel_set_buffered(io, FALSE);
@@ -485,10 +484,8 @@ GAttrib *g_attrib_new(GIOChannel *io)
 	if (attrib == NULL)
 		return NULL;
 
-	att_mtu = ATT_DEFAULT_LE_MTU;
-
-	attrib->buf = g_malloc0(att_mtu);
-	attrib->buflen = att_mtu;
+	attrib->buf = g_malloc0(mtu);
+	attrib->buflen = mtu;
 
 	attrib->io = g_io_channel_ref(io);
 	attrib->requests = g_queue_new();
diff --git a/attrib/gattrib.h b/attrib/gattrib.h
index 18df2ad..99b8b37 100644
--- a/attrib/gattrib.h
+++ b/attrib/gattrib.h
@@ -42,7 +42,7 @@ typedef void (*GAttribDebugFunc)(const char *str, gpointer user_data);
 typedef void (*GAttribNotifyFunc)(const guint8 *pdu, guint16 len,
 							gpointer user_data);
 
-GAttrib *g_attrib_new(GIOChannel *io);
+GAttrib *g_attrib_new(GIOChannel *io, guint16 mtu);
 GAttrib *g_attrib_ref(GAttrib *attrib);
 void g_attrib_unref(GAttrib *attrib);
 
diff --git a/attrib/gatttool.c b/attrib/gatttool.c
index db5da62..8f92d62 100644
--- a/attrib/gatttool.c
+++ b/attrib/gatttool.c
@@ -123,6 +123,9 @@ static gboolean listen_start(gpointer user_data)
 static void connect_cb(GIOChannel *io, GError *err, gpointer user_data)
 {
 	GAttrib *attrib;
+	uint16_t mtu;
+	uint16_t cid;
+	GError *gerr = NULL;
 
 	if (err) {
 		g_printerr("%s\n", err->message);
@@ -130,7 +133,19 @@ static void connect_cb(GIOChannel *io, GError *err, gpointer user_data)
 		g_main_loop_quit(event_loop);
 	}
 
-	attrib = g_attrib_new(io);
+	bt_io_get(io, &gerr, BT_IO_OPT_IMTU, &mtu,
+				BT_IO_OPT_CID, &cid, BT_IO_OPT_INVALID);
+
+	if (gerr) {
+		g_printerr("Can't detect MTU, using default: %s", gerr->message);
+		g_error_free(gerr);
+		mtu = ATT_DEFAULT_LE_MTU;
+	}
+
+	if (cid == ATT_CID)
+		mtu = ATT_DEFAULT_LE_MTU;
+
+	attrib = g_attrib_new(io, mtu);
 
 	if (opt_listen)
 		g_idle_add(listen_start, attrib);
diff --git a/attrib/interactive.c b/attrib/interactive.c
index 08f39f7..7911ba5 100644
--- a/attrib/interactive.c
+++ b/attrib/interactive.c
@@ -150,13 +150,28 @@ static void events_handler(const uint8_t *pdu, uint16_t len, gpointer user_data)
 
 static void connect_cb(GIOChannel *io, GError *err, gpointer user_data)
 {
+	uint16_t mtu;
+	uint16_t cid;
+
 	if (err) {
 		set_state(STATE_DISCONNECTED);
 		error("%s\n", err->message);
 		return;
 	}
 
-	attrib = g_attrib_new(iochannel);
+	bt_io_get(io, &err, BT_IO_OPT_IMTU, &mtu,
+				BT_IO_OPT_CID, &cid, BT_IO_OPT_INVALID);
+
+	if (err) {
+		g_printerr("Can't detect MTU, using default: %s", err->message);
+		g_error_free(err);
+		mtu = ATT_DEFAULT_LE_MTU;
+	}
+
+	if (cid == ATT_CID)
+		mtu = ATT_DEFAULT_LE_MTU;
+
+	attrib = g_attrib_new(iochannel, mtu);
 	g_attrib_register(attrib, ATT_OP_HANDLE_NOTIFY, GATTRIB_ALL_HANDLES,
 						events_handler, attrib, NULL);
 	g_attrib_register(attrib, ATT_OP_HANDLE_IND, GATTRIB_ALL_HANDLES,
diff --git a/src/device.c b/src/device.c
index db6c4f8..6e1f8ea 100644
--- a/src/device.c
+++ b/src/device.c
@@ -3628,11 +3628,17 @@ bool device_attach_attrib(struct btd_device *dev, GIOChannel *io)
 	GError *gerr = NULL;
 	GAttrib *attrib;
 	BtIOSecLevel sec_level;
+	uint16_t mtu;
+	uint16_t cid;
 
 	bt_io_get(io, &gerr, BT_IO_OPT_SEC_LEVEL, &sec_level,
+						BT_IO_OPT_IMTU, &mtu,
+						BT_IO_OPT_CID, &cid,
 						BT_IO_OPT_INVALID);
+
 	if (gerr) {
 		error("bt_io_get: %s", gerr->message);
+		mtu = ATT_DEFAULT_LE_MTU;
 		g_error_free(gerr);
 		return false;
 	}
@@ -3650,7 +3656,10 @@ bool device_attach_attrib(struct btd_device *dev, GIOChannel *io)
 		}
 	}
 
-	attrib = g_attrib_new(io);
+	if (cid == ATT_CID)
+		mtu = ATT_DEFAULT_LE_MTU;
+
+	attrib = g_attrib_new(io, mtu);
 	if (!attrib) {
 		error("Unable to create new GAttrib instance");
 		return false;
-- 
2.1.0.rc2.206.gedb03e5


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

* [PATCH BlueZ v5 2/8] attrib: remove g_attrib_is_encrypted
  2014-10-31 18:14 [PATCH BlueZ v5 0/8] Unit tests for gattrib API Michael Janssen
  2014-10-31 18:14 ` [PATCH BlueZ v5 1/8] attrib: Add mtu argument to g_attrib_new Michael Janssen
@ 2014-10-31 18:14 ` Michael Janssen
  2014-10-31 18:14 ` [PATCH BlueZ v5 3/8] Add unit tests for gattrib Michael Janssen
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Michael Janssen @ 2014-10-31 18:14 UTC (permalink / raw
  To: linux-bluetooth; +Cc: Michael Janssen

This function is only used in one place and encryption is the
responsibility of the channel, not the attribute.
---
 attrib/gattrib.c    | 12 ------------
 attrib/gattrib.h    |  2 --
 src/attrib-server.c | 12 ++++++++++++
 3 files changed, 12 insertions(+), 14 deletions(-)

diff --git a/attrib/gattrib.c b/attrib/gattrib.c
index a6511a2..a65d1ca 100644
--- a/attrib/gattrib.c
+++ b/attrib/gattrib.c
@@ -690,18 +690,6 @@ static int event_cmp_by_id(gconstpointer a, gconstpointer b)
 	return evt->id - id;
 }
 
-gboolean g_attrib_is_encrypted(GAttrib *attrib)
-{
-	BtIOSecLevel sec_level;
-
-	if (!bt_io_get(attrib->io, NULL,
-			BT_IO_OPT_SEC_LEVEL, &sec_level,
-			BT_IO_OPT_INVALID))
-		return FALSE;
-
-	return sec_level > BT_IO_SEC_LOW;
-}
-
 gboolean g_attrib_unregister(GAttrib *attrib, guint id)
 {
 	struct event *evt;
diff --git a/attrib/gattrib.h b/attrib/gattrib.h
index 99b8b37..1557b99 100644
--- a/attrib/gattrib.h
+++ b/attrib/gattrib.h
@@ -62,8 +62,6 @@ guint g_attrib_register(GAttrib *attrib, guint8 opcode, guint16 handle,
 				GAttribNotifyFunc func, gpointer user_data,
 				GDestroyNotify notify);
 
-gboolean g_attrib_is_encrypted(GAttrib *attrib);
-
 uint8_t *g_attrib_get_buffer(GAttrib *attrib, size_t *len);
 gboolean g_attrib_set_mtu(GAttrib *attrib, int mtu);
 
diff --git a/src/attrib-server.c b/src/attrib-server.c
index e65fff2..6571577 100644
--- a/src/attrib-server.c
+++ b/src/attrib-server.c
@@ -372,6 +372,18 @@ static struct attribute *attrib_db_add_new(struct gatt_server *server,
 	return a;
 }
 
+static bool g_attrib_is_encrypted(GAttrib *attrib)
+{
+	BtIOSecLevel sec_level;
+	GIOChannel *io = g_attrib_get_channel(attrib);
+
+	if (!bt_io_get(io, NULL, BT_IO_OPT_SEC_LEVEL, &sec_level,
+							     BT_IO_OPT_INVALID))
+		return FALSE;
+
+	return sec_level > BT_IO_SEC_LOW;
+}
+
 static uint8_t att_check_reqs(struct gatt_channel *channel, uint8_t opcode,
 								int reqs)
 {
-- 
2.1.0.rc2.206.gedb03e5


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

* [PATCH BlueZ v5 3/8] Add unit tests for gattrib
  2014-10-31 18:14 [PATCH BlueZ v5 0/8] Unit tests for gattrib API Michael Janssen
  2014-10-31 18:14 ` [PATCH BlueZ v5 1/8] attrib: Add mtu argument to g_attrib_new Michael Janssen
  2014-10-31 18:14 ` [PATCH BlueZ v5 2/8] attrib: remove g_attrib_is_encrypted Michael Janssen
@ 2014-10-31 18:14 ` Michael Janssen
  2014-10-31 18:14 ` [PATCH BlueZ v5 4/8] attrib: Add unit tests for g_attrib_register Michael Janssen
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Michael Janssen @ 2014-10-31 18:14 UTC (permalink / raw
  To: linux-bluetooth; +Cc: Michael Janssen

This will ensure that the API behavior of gattrib is preserved.
---
 .gitignore          |   1 +
 Makefile.am         |   7 +
 unit/test-gattrib.c | 395 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 403 insertions(+)
 create mode 100644 unit/test-gattrib.c

diff --git a/.gitignore b/.gitignore
index 97daa9f..164cc97 100644
--- a/.gitignore
+++ b/.gitignore
@@ -121,6 +121,7 @@ unit/test-avdtp
 unit/test-avctp
 unit/test-avrcp
 unit/test-gatt
+unit/test-gattrib
 unit/test-*.log
 unit/test-*.trs
 
diff --git a/Makefile.am b/Makefile.am
index 2dfea28..3fddb80 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -377,6 +377,13 @@ unit_test_gatt_SOURCES = unit/test-gatt.c
 unit_test_gatt_LDADD = src/libshared-glib.la \
 				lib/libbluetooth-internal.la @GLIB_LIBS@
 
+unit_tests += unit/test-gattrib
+
+unit_test_gattrib_SOURCES = unit/test-gattrib.c attrib/gattrib.c $(btio_sources) src/log.h src/log.c
+unit_test_gattrib_LDADD = lib/libbluetooth-internal.la \
+			src/libshared-glib.la \
+			@GLIB_LIBS@ @DBUS_LIBS@ -ldl -lrt
+
 if MAINTAINER_MODE
 noinst_PROGRAMS += $(unit_tests)
 endif
diff --git a/unit/test-gattrib.c b/unit/test-gattrib.c
new file mode 100644
index 0000000..1f5494f
--- /dev/null
+++ b/unit/test-gattrib.c
@@ -0,0 +1,395 @@
+/*
+ *
+ *  BlueZ - Bluetooth protocol stack for Linux
+ *
+ *  Copyright (C) 2014  Google, Inc.
+ *
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <unistd.h>
+#include <stdlib.h>
+#include <stdbool.h>
+#include <inttypes.h>
+#include <string.h>
+#include <fcntl.h>
+#include <sys/socket.h>
+
+#include <glib.h>
+
+#include "src/shared/util.h"
+#include "lib/uuid.h"
+#include "attrib/att.h"
+#include "attrib/gattrib.h"
+#include "src/log.h"
+
+#define DEFAULT_MTU 23
+
+#define data(args...) ((const unsigned char[]) { args })
+
+struct test_pdu {
+	bool valid;
+	bool sent;
+	bool received;
+	const uint8_t *data;
+	size_t size;
+};
+
+#define pdu(args...)				\
+	{					\
+		.valid = true,			\
+		.sent = false,			\
+		.received = false,		\
+		.data = data(args),		\
+		.size = sizeof(data(args)),	\
+	}
+
+struct context {
+	GMainLoop *main_loop;
+	GIOChannel *att_io;
+	GIOChannel *server_io;
+	GAttrib *att;
+};
+
+static void setup_context(struct context *cxt, gconstpointer data)
+{
+	int err, sv[2];
+
+	cxt->main_loop = g_main_loop_new(NULL, FALSE);
+	g_assert(cxt->main_loop != NULL);
+
+	err = socketpair(AF_UNIX, SOCK_SEQPACKET | SOCK_CLOEXEC, 0, sv);
+	g_assert(err == 0);
+
+	cxt->att_io = g_io_channel_unix_new(sv[0]);
+	g_assert(cxt->att_io != NULL);
+
+	g_io_channel_set_close_on_unref(cxt->att_io, TRUE);
+
+	cxt->server_io = g_io_channel_unix_new(sv[1]);
+	g_assert(cxt->server_io != NULL);
+
+	g_io_channel_set_close_on_unref(cxt->server_io, TRUE);
+	g_io_channel_set_encoding(cxt->server_io, NULL, NULL);
+	g_io_channel_set_buffered(cxt->server_io, FALSE);
+
+	cxt->att = g_attrib_new(cxt->att_io, DEFAULT_MTU);
+	g_assert(cxt->att != NULL);
+}
+
+static void teardown_context(struct context *cxt, gconstpointer data)
+{
+	if (cxt->att)
+		g_attrib_unref(cxt->att);
+
+	g_io_channel_unref(cxt->server_io);
+
+	g_io_channel_unref(cxt->att_io);
+
+	g_main_loop_unref(cxt->main_loop);
+}
+
+
+static void test_debug(const char *str, void *user_data)
+{
+	const char *prefix = user_data;
+
+	g_print("%s%s\n", prefix, str);
+}
+
+static void destroy_canary_increment(gpointer data)
+{
+	int *canary = data;
+	(*canary)++;
+}
+
+static void test_refcount(struct context *cxt, gconstpointer unused)
+{
+	GAttrib *extra_ref;
+	int destroy_canary;
+
+	g_attrib_set_destroy_function(cxt->att, destroy_canary_increment,
+							       &destroy_canary);
+
+	extra_ref = g_attrib_ref(cxt->att);
+
+	g_assert(extra_ref == cxt->att);
+
+	g_assert(destroy_canary == 0);
+
+	g_attrib_unref(extra_ref);
+
+	g_assert(destroy_canary == 0);
+
+	g_attrib_unref(cxt->att);
+
+	g_assert(destroy_canary == 1);
+
+	/* Avoid a double-free from the teardown function */
+	cxt->att = NULL;
+}
+
+static void test_get_channel(struct context *cxt, gconstpointer unused)
+{
+	GIOChannel *chan;
+
+	chan = g_attrib_get_channel(cxt->att);
+
+	g_assert(chan == cxt->att_io);
+}
+
+struct expect_response {
+	struct test_pdu expect;
+	struct test_pdu respond;
+	GSourceFunc receive_cb;
+	gpointer user_data;
+};
+
+static gboolean test_client(GIOChannel *channel, GIOCondition cond,
+								  gpointer data)
+{
+	struct expect_response *cr = data;
+	int fd;
+	uint8_t buf[256];
+	ssize_t len;
+	int cmp;
+
+	if (cond & (G_IO_NVAL | G_IO_ERR | G_IO_HUP)) {
+		return FALSE;
+	}
+
+	fd = g_io_channel_unix_get_fd(channel);
+
+	len = read(fd, buf, sizeof(buf));
+
+	g_assert(len > 0);
+	g_assert_cmpint(len, ==, cr->expect.size);
+
+	if (g_test_verbose())
+		util_hexdump('?', cr->expect.data,  cr->expect.size,
+						   test_debug, "test_client: ");
+
+	cmp = memcmp(cr->expect.data, buf, len);
+
+	g_assert(cmp == 0);
+
+	cr->expect.received = true;
+
+	if (cr->receive_cb != NULL)
+		cr->receive_cb(cr->user_data);
+
+	if (cr->respond.valid) {
+		if (g_test_verbose())
+			util_hexdump('<', cr->respond.data, cr->respond.size,
+						   test_debug, "test_client: ");
+		len = write(fd, cr->respond.data, cr->respond.size);
+
+		g_assert_cmpint(len, ==, cr->respond.size);
+
+		cr->respond.sent = true;
+	}
+
+	return TRUE;
+}
+
+struct result_data {
+	guint8 status;
+	guint8 *pdu;
+	guint16 len;
+	GSourceFunc complete_cb;
+	gpointer user_data;
+};
+
+static void result_canary(guint8 status, const guint8 *pdu, guint16 len,
+								gpointer data)
+{
+	struct result_data *result = data;
+	result->status = status;
+	result->pdu = g_malloc0(len);
+	memcpy(result->pdu, pdu, len);
+	result->len = len;
+
+	if (g_test_verbose())
+		util_hexdump('<', pdu, len, test_debug, "result_canary: ");
+
+	if (result->complete_cb != NULL)
+		result->complete_cb(result->user_data);
+}
+
+static gboolean context_stop_main_loop(gpointer user_data) {
+	struct context *cxt = user_data;
+	g_main_loop_quit(cxt->main_loop);
+	return FALSE;
+}
+
+static void test_send(struct context *cxt, gconstpointer unused)
+{
+	int cmp;
+	struct result_data results;
+	struct expect_response data = {
+		.expect = pdu(0x02, 0x00, 0x02),
+		.respond = pdu(0x03, 0x02, 0x03, 0x04),
+		.receive_cb = NULL,
+		.user_data = NULL,
+	};
+
+	g_io_add_watch(cxt->server_io, G_IO_IN | G_IO_HUP | G_IO_ERR | G_IO_NVAL,
+							    test_client, &data);
+
+	results.complete_cb = context_stop_main_loop;
+	results.user_data = cxt;
+
+	g_attrib_send(cxt->att, 0, data.expect.data, data.expect.size,
+				      result_canary, (gpointer) &results, NULL);
+
+	g_main_loop_run(cxt->main_loop);
+
+	g_assert(results.pdu != NULL);
+
+	g_assert_cmpint(results.len, ==, data.respond.size);
+
+	cmp = memcmp(results.pdu, data.respond.data, results.len);
+
+	g_assert(cmp == 0);
+
+	g_free(results.pdu);
+}
+
+struct event_info {
+	struct context *context;
+	int event_id;
+};
+
+static gboolean cancel_existing_attrib_event(gpointer user_data) {
+	struct event_info *info = user_data;
+	gboolean canceled;
+
+	canceled = g_attrib_cancel(info->context->att, info->event_id);
+
+	g_assert(canceled);
+
+	g_idle_add(context_stop_main_loop, info->context);
+
+	return FALSE;
+}
+
+static void test_cancel(struct context *cxt, gconstpointer unused)
+{
+	gboolean canceled;
+	struct result_data results;
+	struct event_info info;
+	struct expect_response data = {
+		.expect = pdu(0x02, 0x00, 0x02),
+		.respond = pdu(0x03, 0x02, 0x03, 0x04),
+	};
+
+	g_io_add_watch(cxt->server_io,
+				      G_IO_IN | G_IO_HUP | G_IO_ERR | G_IO_NVAL,
+							    test_client, &data);
+
+	results.pdu = NULL;
+
+	info.context = cxt;
+	info.event_id = g_attrib_send(cxt->att, 0, data.expect.data,
+						data.expect.size, result_canary,
+								&results, NULL);
+
+	data.receive_cb = cancel_existing_attrib_event;
+	data.user_data = &info;
+
+	g_main_loop_run(cxt->main_loop);
+
+	g_assert(results.pdu == NULL);
+
+	results.pdu = NULL;
+	data.expect.received = false;
+	data.respond.sent = false;
+
+	info.event_id = g_attrib_send(cxt->att, 0, data.expect.data,
+						data.expect.size, result_canary,
+								&results, NULL);
+
+	canceled = g_attrib_cancel(cxt->att, info.event_id);
+	g_assert(canceled);
+
+	g_idle_add(context_stop_main_loop, info.context);
+
+	g_main_loop_run(cxt->main_loop);
+
+	g_assert(!data.expect.received);
+	g_assert(!data.respond.sent);
+	g_assert(results.pdu == NULL);
+
+	/* Invalid ID */
+	canceled = g_attrib_cancel(cxt->att, 42);
+	g_assert(!canceled);
+}
+
+static void test_register(struct context *cxt, gconstpointer user_data)
+{
+	/* TODO */
+}
+
+static void test_buffers(struct context *cxt, gconstpointer unused)
+{
+	size_t buflen;
+	uint8_t *buf;
+	gboolean success;
+
+	buf = g_attrib_get_buffer(cxt->att, &buflen);
+	g_assert(buf != 0);
+	g_assert_cmpint(buflen, ==, DEFAULT_MTU);
+
+	success = g_attrib_set_mtu(cxt->att, 5);
+	g_assert(!success);
+
+	success = g_attrib_set_mtu(cxt->att, 255);
+	g_assert(success);
+
+	buf = g_attrib_get_buffer(cxt->att, &buflen);
+	g_assert(buf != 0);
+	g_assert_cmpint(buflen, ==, 255);
+}
+
+int main(int argc, char *argv[])
+{
+	g_test_init(&argc, &argv, NULL);
+
+	__btd_log_init("*", 0);
+
+	/*
+	 * Test the GAttrib API behavior
+	 */
+	g_test_add("/gattrib/refcount", struct context, NULL, setup_context,
+					      test_refcount, teardown_context);
+	g_test_add("/gattrib/get_channel", struct context, NULL, setup_context,
+					    test_get_channel, teardown_context);
+	g_test_add("/gattrib/send", struct context, NULL, setup_context,
+						   test_send, teardown_context);
+	g_test_add("/gattrib/cancel", struct context, NULL, setup_context,
+						 test_cancel, teardown_context);
+	g_test_add("/gattrib/register", struct context, NULL, setup_context,
+					       test_register, teardown_context);
+	g_test_add("/gattrib/buffers", struct context, NULL, setup_context,
+						test_buffers, teardown_context);
+
+	return g_test_run();
+}
-- 
2.1.0.rc2.206.gedb03e5


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

* [PATCH BlueZ v5 4/8] attrib: Add unit tests for g_attrib_register
  2014-10-31 18:14 [PATCH BlueZ v5 0/8] Unit tests for gattrib API Michael Janssen
                   ` (2 preceding siblings ...)
  2014-10-31 18:14 ` [PATCH BlueZ v5 3/8] Add unit tests for gattrib Michael Janssen
@ 2014-10-31 18:14 ` Michael Janssen
  2014-10-31 18:14 ` [PATCH BlueZ v5 5/8] attrib: fix GATTRIB_ALL_REQS behavior Michael Janssen
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Michael Janssen @ 2014-10-31 18:14 UTC (permalink / raw
  To: linux-bluetooth; +Cc: Michael Janssen

---
 unit/test-gattrib.c | 140 +++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 139 insertions(+), 1 deletion(-)

diff --git a/unit/test-gattrib.c b/unit/test-gattrib.c
index 1f5494f..473112c 100644
--- a/unit/test-gattrib.c
+++ b/unit/test-gattrib.c
@@ -343,9 +343,147 @@ static void test_cancel(struct context *cxt, gconstpointer unused)
 	g_assert(!canceled);
 }
 
+static void send_test_pdus(gpointer context, struct test_pdu *pdus)
+{
+	struct context *cxt = context;
+	size_t len;
+	int fd;
+	struct test_pdu *cur_pdu;
+
+	fd = g_io_channel_unix_get_fd(cxt->server_io);
+
+	for (cur_pdu = pdus; cur_pdu->valid; cur_pdu++)
+		cur_pdu->sent = false;
+
+	for (cur_pdu = pdus; cur_pdu->valid; cur_pdu++) {
+		if (g_test_verbose())
+			util_hexdump('>', cur_pdu->data, cur_pdu->size,
+						 test_debug, "send_test_pdus: ");
+		len = write(fd, cur_pdu->data, cur_pdu->size);
+		g_assert_cmpint(len, ==, cur_pdu->size);
+		cur_pdu->sent = true;
+	}
+
+	g_idle_add(context_stop_main_loop, cxt);
+	g_main_loop_run(cxt->main_loop);
+}
+
+#define PDU_MTU_RESP pdu(ATT_OP_MTU_RESP, 0x17)
+#define PDU_FIND_INFO_REQ pdu(ATT_OP_FIND_INFO_REQ, 0x01, 0x00, 0xFF, 0xFF)
+#define PDU_IND_NODATA pdu(ATT_OP_HANDLE_IND, 0x01, 0x00)
+#define PDU_INVALID_IND pdu(ATT_OP_HANDLE_IND, 0x14)
+#define PDU_IND_DATA pdu(ATT_OP_HANDLE_IND, 0x14, 0x00, 0x01)
+
+static void notify_canary_expect(const guint8 *pdu, guint16 len, gpointer data)
+{
+	struct test_pdu *expected = data;
+	int cmp;
+
+	if (g_test_verbose())
+		util_hexdump('<', pdu, len, test_debug,
+						      "notify_canary_expect: ");
+
+	while (expected->valid && expected->received)
+		expected++;
+
+	g_assert(expected->valid);
+
+	if (g_test_verbose())
+		util_hexdump('?', expected->data, expected->size, test_debug,
+						      "notify_canary_expect: ");
+
+	g_assert_cmpint(expected->size, ==, len);
+
+	cmp = memcmp(pdu, expected->data, expected->size);
+
+	g_assert(cmp == 0);
+
+	expected->received = true;
+}
+
 static void test_register(struct context *cxt, gconstpointer user_data)
 {
-	/* TODO */
+	guint reg_id;
+	gboolean success;
+	struct test_pdu pdus[] = {
+		/* Unmatched by any (GATTRIB_ALL_EVENTS) */
+		PDU_MTU_RESP,
+		/*
+		 * Unmatched PDU opcode
+		 * Unmatched handle (GATTRIB_ALL_REQS) */
+		PDU_FIND_INFO_REQ,
+		/*
+		 * Matched PDU opcode
+		 * Unmatched handle (GATTRIB_ALL_HANDLES) */
+		PDU_IND_NODATA,
+		/*
+		 * Matched PDU opcode
+		 * Invalid length? */
+		PDU_INVALID_IND,
+		/*
+		 * Matched PDU opcode
+		 * Matched handle */
+		PDU_IND_DATA,
+		{ },
+	};
+	struct test_pdu req_pdus[] = { PDU_FIND_INFO_REQ, { } };
+	struct test_pdu all_ind_pdus[] = {
+		PDU_IND_NODATA,
+		PDU_INVALID_IND,
+		PDU_IND_DATA,
+		{ },
+	};
+	struct test_pdu followed_ind_pdus[] = { PDU_IND_DATA, { } };
+
+	/*
+	 * Without registering anything, should be able to ignore everything but
+	 * an unexpected response. */
+	send_test_pdus(cxt, pdus + 1);
+
+	reg_id = g_attrib_register(cxt->att, GATTRIB_ALL_EVENTS,
+				      GATTRIB_ALL_HANDLES, notify_canary_expect,
+								    pdus, NULL);
+
+	send_test_pdus(cxt, pdus);
+
+	g_attrib_unregister(cxt->att, reg_id);
+
+	if (g_test_verbose())
+		g_print("ALL_REQS, ALL_HANDLES\r\n");
+
+	reg_id = g_attrib_register(cxt->att, GATTRIB_ALL_REQS,
+				      GATTRIB_ALL_HANDLES, notify_canary_expect,
+								req_pdus, NULL);
+
+	send_test_pdus(cxt, pdus);
+
+	g_attrib_unregister(cxt->att, reg_id);
+
+	if (g_test_verbose())
+		g_print("IND, ALL_HANDLES\r\n");
+
+	reg_id = g_attrib_register(cxt->att, ATT_OP_HANDLE_IND,
+				      GATTRIB_ALL_HANDLES, notify_canary_expect,
+							    all_ind_pdus, NULL);
+
+	send_test_pdus(cxt, pdus);
+
+	g_attrib_unregister(cxt->att, reg_id);
+
+	if (g_test_verbose())
+		g_print("IND, 0x0014\r\n");
+
+	reg_id = g_attrib_register(cxt->att, ATT_OP_HANDLE_IND, 0x0014,
+					notify_canary_expect, followed_ind_pdus,
+									  NULL);
+
+	send_test_pdus(cxt, pdus);
+
+	g_attrib_unregister(cxt->att, reg_id);
+
+	success = g_attrib_unregister(cxt->att, reg_id);
+
+	g_assert(!success);
 }
 
 static void test_buffers(struct context *cxt, gconstpointer unused)
-- 
2.1.0.rc2.206.gedb03e5


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

* [PATCH BlueZ v5 5/8] attrib: fix GATTRIB_ALL_REQS behavior
  2014-10-31 18:14 [PATCH BlueZ v5 0/8] Unit tests for gattrib API Michael Janssen
                   ` (3 preceding siblings ...)
  2014-10-31 18:14 ` [PATCH BlueZ v5 4/8] attrib: Add unit tests for g_attrib_register Michael Janssen
@ 2014-10-31 18:14 ` Michael Janssen
  2014-10-31 18:14 ` [PATCH BlueZ v5 6/8] unit/gattrib: Check expected PDUs were delivered Michael Janssen
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Michael Janssen @ 2014-10-31 18:14 UTC (permalink / raw
  To: linux-bluetooth; +Cc: Michael Janssen

g_attrib_register(..., GATTRIB_ALL_REQS, ...) behavior would previously
include indications, this fixes it to only include requests and
commands.
---
 attrib/gattrib.c | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/attrib/gattrib.c b/attrib/gattrib.c
index a65d1ca..f678435 100644
--- a/attrib/gattrib.c
+++ b/attrib/gattrib.c
@@ -147,6 +147,27 @@ static bool is_response(guint8 opcode)
 	return false;
 }
 
+static bool is_request(guint8 opcode)
+{
+	switch (opcode) {
+	case ATT_OP_MTU_REQ:
+	case ATT_OP_FIND_INFO_REQ:
+	case ATT_OP_FIND_BY_TYPE_REQ:
+	case ATT_OP_READ_BY_TYPE_REQ:
+	case ATT_OP_READ_REQ:
+	case ATT_OP_READ_BLOB_REQ:
+	case ATT_OP_READ_MULTI_REQ:
+	case ATT_OP_READ_BY_GROUP_REQ:
+	case ATT_OP_WRITE_REQ:
+	case ATT_OP_WRITE_CMD:
+	case ATT_OP_PREP_WRITE_REQ:
+	case ATT_OP_EXEC_WRITE_REQ:
+		return true;
+	}
+
+	return false;
+}
+
 GAttrib *g_attrib_ref(GAttrib *attrib)
 {
 	int refs;
@@ -373,7 +394,7 @@ static bool match_event(struct event *evt, const uint8_t *pdu, gsize len)
 	if (evt->expected == GATTRIB_ALL_EVENTS)
 		return true;
 
-	if (!is_response(pdu[0]) && evt->expected == GATTRIB_ALL_REQS)
+	if (is_request(pdu[0]) && evt->expected == GATTRIB_ALL_REQS)
 		return true;
 
 	if (evt->expected == pdu[0] && evt->handle == GATTRIB_ALL_HANDLES)
-- 
2.1.0.rc2.206.gedb03e5


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

* [PATCH BlueZ v5 6/8] unit/gattrib: Check expected PDUs were delivered
  2014-10-31 18:14 [PATCH BlueZ v5 0/8] Unit tests for gattrib API Michael Janssen
                   ` (4 preceding siblings ...)
  2014-10-31 18:14 ` [PATCH BlueZ v5 5/8] attrib: fix GATTRIB_ALL_REQS behavior Michael Janssen
@ 2014-10-31 18:14 ` Michael Janssen
  2014-10-31 18:14 ` [PATCH BlueZ v5 7/8] unit/gattrib: Respond to the request PDU Michael Janssen
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Michael Janssen @ 2014-10-31 18:14 UTC (permalink / raw
  To: linux-bluetooth; +Cc: Michael Janssen

Check that the PDUs that we expect to receive when
registering for events have been delivered.
---
 unit/test-gattrib.c | 35 ++++++++++++++++++++++++++++-------
 1 file changed, 28 insertions(+), 7 deletions(-)

diff --git a/unit/test-gattrib.c b/unit/test-gattrib.c
index 473112c..326478d 100644
--- a/unit/test-gattrib.c
+++ b/unit/test-gattrib.c
@@ -404,7 +404,7 @@ static void notify_canary_expect(const guint8 *pdu, guint16 len, gpointer data)
 static void test_register(struct context *cxt, gconstpointer user_data)
 {
 	guint reg_id;
-	gboolean success;
+	gboolean canceled;
 	struct test_pdu pdus[] = {
 		/* Unmatched by any (GATTRIB_ALL_EVENTS) */
 		PDU_MTU_RESP,
@@ -434,6 +434,7 @@ static void test_register(struct context *cxt, gconstpointer user_data)
 		{ },
 	};
 	struct test_pdu followed_ind_pdus[] = { PDU_IND_DATA, { } };
+	struct test_pdu *current_pdu;
 
 	/*
 	 * Without registering anything, should be able to ignore everything but
@@ -446,7 +447,12 @@ static void test_register(struct context *cxt, gconstpointer user_data)
 
 	send_test_pdus(cxt, pdus);
 
-	g_attrib_unregister(cxt->att, reg_id);
+	canceled = g_attrib_unregister(cxt->att, reg_id);
+
+	g_assert(canceled);
+
+	for (current_pdu = pdus; current_pdu->valid; current_pdu++)
+		g_assert(current_pdu->received);
 
 	if (g_test_verbose())
 		g_print("ALL_REQS, ALL_HANDLES\r\n");
@@ -457,7 +463,12 @@ static void test_register(struct context *cxt, gconstpointer user_data)
 
 	send_test_pdus(cxt, pdus);
 
-	g_attrib_unregister(cxt->att, reg_id);
+	canceled = g_attrib_unregister(cxt->att, reg_id);
+
+	g_assert(canceled);
+
+	for (current_pdu = req_pdus; current_pdu->valid; current_pdu++)
+		g_assert(current_pdu->received);
 
 	if (g_test_verbose())
 		g_print("IND, ALL_HANDLES\r\n");
@@ -468,7 +479,12 @@ static void test_register(struct context *cxt, gconstpointer user_data)
 
 	send_test_pdus(cxt, pdus);
 
-	g_attrib_unregister(cxt->att, reg_id);
+	canceled = g_attrib_unregister(cxt->att, reg_id);
+
+	g_assert(canceled);
+
+	for (current_pdu = all_ind_pdus; current_pdu->valid; current_pdu++)
+		g_assert(current_pdu->received);
 
 	if (g_test_verbose())
 		g_print("IND, 0x0014\r\n");
@@ -479,11 +495,16 @@ static void test_register(struct context *cxt, gconstpointer user_data)
 
 	send_test_pdus(cxt, pdus);
 
-	g_attrib_unregister(cxt->att, reg_id);
+	canceled = g_attrib_unregister(cxt->att, reg_id);
+
+	g_assert(canceled);
 
-	success = g_attrib_unregister(cxt->att, reg_id);
+	for (current_pdu = followed_ind_pdus; current_pdu->valid; current_pdu++)
+		g_assert(current_pdu->received);
 
-	g_assert(!success);
+	canceled = g_attrib_unregister(cxt->att, reg_id);
+
+	g_assert(!canceled);
 }
 
 static void test_buffers(struct context *cxt, gconstpointer unused)
-- 
2.1.0.rc2.206.gedb03e5


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

* [PATCH BlueZ v5 7/8] unit/gattrib: Respond to the request PDU.
  2014-10-31 18:14 [PATCH BlueZ v5 0/8] Unit tests for gattrib API Michael Janssen
                   ` (5 preceding siblings ...)
  2014-10-31 18:14 ` [PATCH BlueZ v5 6/8] unit/gattrib: Check expected PDUs were delivered Michael Janssen
@ 2014-10-31 18:14 ` Michael Janssen
  2014-10-31 18:14 ` [PATCH BlueZ v5 8/8] unit/gattrib: Only init log on --verbose Michael Janssen
  2014-11-03 15:26 ` [PATCH BlueZ v5 0/8] Unit tests for gattrib API Luiz Augusto von Dentz
  8 siblings, 0 replies; 10+ messages in thread
From: Michael Janssen @ 2014-10-31 18:14 UTC (permalink / raw
  To: linux-bluetooth; +Cc: Michael Janssen

---
 unit/test-gattrib.c | 35 ++++++++++++++++++++++++++++-------
 1 file changed, 28 insertions(+), 7 deletions(-)

diff --git a/unit/test-gattrib.c b/unit/test-gattrib.c
index 326478d..a1ff530 100644
--- a/unit/test-gattrib.c
+++ b/unit/test-gattrib.c
@@ -358,7 +358,7 @@ static void send_test_pdus(gpointer context, struct test_pdu *pdus)
 	for (cur_pdu = pdus; cur_pdu->valid; cur_pdu++) {
 		if (g_test_verbose())
 			util_hexdump('>', cur_pdu->data, cur_pdu->size,
-						 test_debug, "send_test_pdus: ");
+                                                test_debug, "send_test_pdus: ");
 		len = write(fd, cur_pdu->data, cur_pdu->size);
 		g_assert_cmpint(len, ==, cur_pdu->size);
 		cur_pdu->sent = true;
@@ -370,13 +370,20 @@ static void send_test_pdus(gpointer context, struct test_pdu *pdus)
 
 #define PDU_MTU_RESP pdu(ATT_OP_MTU_RESP, 0x17)
 #define PDU_FIND_INFO_REQ pdu(ATT_OP_FIND_INFO_REQ, 0x01, 0x00, 0xFF, 0xFF)
+#define PDU_NO_ATT_ERR pdu(ATT_OP_ERROR, ATT_OP_FIND_INFO_REQ, 0x00, 0x00, 0x0A)
 #define PDU_IND_NODATA pdu(ATT_OP_HANDLE_IND, 0x01, 0x00)
 #define PDU_INVALID_IND pdu(ATT_OP_HANDLE_IND, 0x14)
 #define PDU_IND_DATA pdu(ATT_OP_HANDLE_IND, 0x14, 0x00, 0x01)
 
+struct expect_test_data {
+  struct test_pdu *expected;
+  GAttrib *att;
+};
+
 static void notify_canary_expect(const guint8 *pdu, guint16 len, gpointer data)
 {
-	struct test_pdu *expected = data;
+	struct expect_test_data *expect = data;
+        struct test_pdu *expected = expect->expected;
 	int cmp;
 
 	if (g_test_verbose())
@@ -399,6 +406,14 @@ static void notify_canary_expect(const guint8 *pdu, guint16 len, gpointer data)
 	g_assert(cmp == 0);
 
 	expected->received = true;
+
+	if (pdu[0] == ATT_OP_FIND_INFO_REQ) {
+		struct test_pdu no_attributes = PDU_NO_ATT_ERR;
+		int reqid;
+		reqid = g_attrib_send(expect->att, 0, no_attributes.data,
+					  no_attributes.size, NULL, NULL, NULL);
+		g_assert(reqid != 0);
+	}
 }
 
 static void test_register(struct context *cxt, gconstpointer user_data)
@@ -435,15 +450,19 @@ static void test_register(struct context *cxt, gconstpointer user_data)
 	};
 	struct test_pdu followed_ind_pdus[] = { PDU_IND_DATA, { } };
 	struct test_pdu *current_pdu;
+	struct expect_test_data expect;
+
+	expect.att = cxt->att;
 
 	/*
 	 * Without registering anything, should be able to ignore everything but
 	 * an unexpected response. */
 	send_test_pdus(cxt, pdus + 1);
 
+	expect.expected = pdus;
 	reg_id = g_attrib_register(cxt->att, GATTRIB_ALL_EVENTS,
 				      GATTRIB_ALL_HANDLES, notify_canary_expect,
-								    pdus, NULL);
+								 &expect, NULL);
 
 	send_test_pdus(cxt, pdus);
 
@@ -457,9 +476,10 @@ static void test_register(struct context *cxt, gconstpointer user_data)
 	if (g_test_verbose())
 		g_print("ALL_REQS, ALL_HANDLES\r\n");
 
+	expect.expected = req_pdus;
 	reg_id = g_attrib_register(cxt->att, GATTRIB_ALL_REQS,
 				      GATTRIB_ALL_HANDLES, notify_canary_expect,
-								req_pdus, NULL);
+								 &expect, NULL);
 
 	send_test_pdus(cxt, pdus);
 
@@ -473,9 +493,10 @@ static void test_register(struct context *cxt, gconstpointer user_data)
 	if (g_test_verbose())
 		g_print("IND, ALL_HANDLES\r\n");
 
+	expect.expected = all_ind_pdus;
 	reg_id = g_attrib_register(cxt->att, ATT_OP_HANDLE_IND,
 				      GATTRIB_ALL_HANDLES, notify_canary_expect,
-							    all_ind_pdus, NULL);
+								 &expect, NULL);
 
 	send_test_pdus(cxt, pdus);
 
@@ -489,9 +510,9 @@ static void test_register(struct context *cxt, gconstpointer user_data)
 	if (g_test_verbose())
 		g_print("IND, 0x0014\r\n");
 
+	expect.expected = followed_ind_pdus;
 	reg_id = g_attrib_register(cxt->att, ATT_OP_HANDLE_IND, 0x0014,
-					notify_canary_expect, followed_ind_pdus,
-									  NULL);
+					notify_canary_expect, &expect, NULL);
 
 	send_test_pdus(cxt, pdus);
 
-- 
2.1.0.rc2.206.gedb03e5


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

* [PATCH BlueZ v5 8/8] unit/gattrib: Only init log on --verbose
  2014-10-31 18:14 [PATCH BlueZ v5 0/8] Unit tests for gattrib API Michael Janssen
                   ` (6 preceding siblings ...)
  2014-10-31 18:14 ` [PATCH BlueZ v5 7/8] unit/gattrib: Respond to the request PDU Michael Janssen
@ 2014-10-31 18:14 ` Michael Janssen
  2014-11-03 15:26 ` [PATCH BlueZ v5 0/8] Unit tests for gattrib API Luiz Augusto von Dentz
  8 siblings, 0 replies; 10+ messages in thread
From: Michael Janssen @ 2014-10-31 18:14 UTC (permalink / raw
  To: linux-bluetooth; +Cc: Michael Janssen

---
 unit/test-gattrib.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/unit/test-gattrib.c b/unit/test-gattrib.c
index a1ff530..cd43c3a 100644
--- a/unit/test-gattrib.c
+++ b/unit/test-gattrib.c
@@ -553,7 +553,8 @@ int main(int argc, char *argv[])
 {
 	g_test_init(&argc, &argv, NULL);
 
-	__btd_log_init("*", 0);
+	if (g_test_verbose())
+		__btd_log_init("*", 0);
 
 	/*
 	 * Test the GAttrib API behavior
-- 
2.1.0.rc2.206.gedb03e5


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

* Re: [PATCH BlueZ v5 0/8] Unit tests for gattrib API
  2014-10-31 18:14 [PATCH BlueZ v5 0/8] Unit tests for gattrib API Michael Janssen
                   ` (7 preceding siblings ...)
  2014-10-31 18:14 ` [PATCH BlueZ v5 8/8] unit/gattrib: Only init log on --verbose Michael Janssen
@ 2014-11-03 15:26 ` Luiz Augusto von Dentz
  8 siblings, 0 replies; 10+ messages in thread
From: Luiz Augusto von Dentz @ 2014-11-03 15:26 UTC (permalink / raw
  To: Michael Janssen; +Cc: linux-bluetooth@vger.kernel.org

Hi Michael,

On Fri, Oct 31, 2014 at 8:14 PM, Michael Janssen <jamuraa@chromium.org> wrote:
> Since we want to replace GAttrib with a bt_att shim for a smooth transition
> to bt_att profiles, this patch cleans up the API and adds unit tests for
> GAttrib functions.
>
> This adds tests for all but the register / unregister functions, which
> I will send in a separate patch since they are slightly more complicated due
> to catchalls.
>
> v1 -> v2:
>   * Add g_attrib_register tests
>   * Cleanup of unit/test-gattrib.c
>   * Bugfix for GATTRIB_ALL_REQS
>
> v2 -> v3:
>   * Just move g_attrib_is_encrypted to static (Luiz)
>   * Check that registered-for PDUs were delivered
>   * Respond to Request PDU when registered for it.
>   * Copy result PDUs since they aren't guaranteed to live beyond callback.
>
> v3 -> v4:
>   * Rework to use g_main_loop_run() for running
>   * Reuse pdu_data struct in expect_response
>
> v4 -> v5:
>   * Rebase, fix compilation problems
>   * Only init log on --verbose
>
> Michael Janssen (8):
>   attrib: Add mtu argument to g_attrib_new
>   attrib: remove g_attrib_is_encrypted
>   Add unit tests for gattrib
>   attrib: Add unit tests for g_attrib_register
>   attrib: fix GATTRIB_ALL_REQS behavior
>   unit/gattrib: Check expected PDUs were delivered
>   unit/gattrib: Respond to the request PDU.
>   unit/gattrib: Only init log on --verbose
>
>  .gitignore           |   1 +
>  Makefile.am          |   7 +
>  attrib/gattrib.c     |  44 ++--
>  attrib/gattrib.h     |   4 +-
>  attrib/gatttool.c    |  17 +-
>  attrib/interactive.c |  17 +-
>  src/attrib-server.c  |  12 ++
>  src/device.c         |  11 +-
>  unit/test-gattrib.c  | 576 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  9 files changed, 664 insertions(+), 25 deletions(-)
>  create mode 100644 unit/test-gattrib.c
>
> --
> 2.1.0.rc2.206.gedb03e5

Ive applied this set, but notice that I have to change quite many
things that check-patch was complaining and there was still a build
problem in android/gatt.c:1442:g_attrib_new which you did not update
so I had to fix it myself.

Btw, after applying I realize the very first test had some problems:

bluetoothd[14700]: attrib/gattrib.c:g_attrib_ref() 0x5da8670: ref=2
==14700== Conditional jump or move depends on uninitialised value(s)
==14700==    at 0x4032A6: test_refcount (test-gattrib.c:136)
==14700==    by 0x4E9E5E0: ??? (in /usr/lib64/libglib-2.0.so.0.3800.2)
==14700==    by 0x4E9E7A5: ??? (in /usr/lib64/libglib-2.0.so.0.3800.2)
==14700==    by 0x4E9EB1A: g_test_run_suite (in
/usr/lib64/libglib-2.0.so.0.3800.2)
==14700==    by 0x402197: main (test-gattrib.c:579)
==14700==  Uninitialised value was created by a stack allocation
==14700==    at 0x403250: test_refcount (test-gattrib.c:125)
==14700==
bluetoothd[14700]: attrib/gattrib.c:g_attrib_unref() 0x5da8670: ref=1
==14700== Conditional jump or move depends on uninitialised value(s)
==14700==    at 0x4032D3: test_refcount (test-gattrib.c:140)
==14700==    by 0x4E9E5E0: ??? (in /usr/lib64/libglib-2.0.so.0.3800.2)
==14700==    by 0x4E9E7A5: ??? (in /usr/lib64/libglib-2.0.so.0.3800.2)
==14700==    by 0x4E9EB1A: g_test_run_suite (in
/usr/lib64/libglib-2.0.so.0.3800.2)
==14700==    by 0x402197: main (test-gattrib.c:579)
==14700==  Uninitialised value was created by a stack allocation
==14700==    at 0x403250: test_refcount (test-gattrib.c:125)
==14700==
bluetoothd[14700]: attrib/gattrib.c:g_attrib_unref() 0x5da8670: ref=0
==14700== Conditional jump or move depends on uninitialised value(s)
==14700==    at 0x4032E3: test_refcount (test-gattrib.c:144)
==14700==    by 0x4E9E5E0: ??? (in /usr/lib64/libglib-2.0.so.0.3800.2)
==14700==    by 0x4E9E7A5: ??? (in /usr/lib64/libglib-2.0.so.0.3800.2)
==14700==    by 0x4E9EB1A: g_test_run_suite (in
/usr/lib64/libglib-2.0.so.0.3800.2)
==14700==    by 0x402197: main (test-gattrib.c:579)
==14700==  Uninitialised value was created by a stack allocation
==14700==    at 0x403250: test_refcount (test-gattrib.c:125)
==14700==



-- 
Luiz Augusto von Dentz

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

end of thread, other threads:[~2014-11-03 15:26 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-10-31 18:14 [PATCH BlueZ v5 0/8] Unit tests for gattrib API Michael Janssen
2014-10-31 18:14 ` [PATCH BlueZ v5 1/8] attrib: Add mtu argument to g_attrib_new Michael Janssen
2014-10-31 18:14 ` [PATCH BlueZ v5 2/8] attrib: remove g_attrib_is_encrypted Michael Janssen
2014-10-31 18:14 ` [PATCH BlueZ v5 3/8] Add unit tests for gattrib Michael Janssen
2014-10-31 18:14 ` [PATCH BlueZ v5 4/8] attrib: Add unit tests for g_attrib_register Michael Janssen
2014-10-31 18:14 ` [PATCH BlueZ v5 5/8] attrib: fix GATTRIB_ALL_REQS behavior Michael Janssen
2014-10-31 18:14 ` [PATCH BlueZ v5 6/8] unit/gattrib: Check expected PDUs were delivered Michael Janssen
2014-10-31 18:14 ` [PATCH BlueZ v5 7/8] unit/gattrib: Respond to the request PDU Michael Janssen
2014-10-31 18:14 ` [PATCH BlueZ v5 8/8] unit/gattrib: Only init log on --verbose Michael Janssen
2014-11-03 15:26 ` [PATCH BlueZ v5 0/8] Unit tests for gattrib API Luiz Augusto von Dentz

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.