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.
Michael Janssen (10):
Remove unused g_attrib_set_debug function
attrib: Remove MTU-probing code
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: copy result PDU
.gitignore | 1 +
Makefile.am | 7 +
attrib/gattrib.c | 61 +++---
attrib/gattrib.h | 7 +-
attrib/gatttool.c | 17 +-
attrib/interactive.c | 17 +-
src/attrib-server.c | 12 ++
src/device.c | 11 +-
unit/test-gattrib.c | 552 +++++++++++++++++++++++++++++++++++++++++++++++++++
9 files changed, 640 insertions(+), 45 deletions(-)
create mode 100644 unit/test-gattrib.c
--
2.1.0.rc2.206.gedb03e5
Hi Luiz,
On Tue, Oct 28, 2014 at 1:47 AM, Luiz Augusto von Dentz
<[email protected]> wrote:
> Hi Michael,
>
> On Tue, Oct 28, 2014 at 12:13 AM, Michael Janssen <[email protected]> wrote:
>> This will ensure that the API behavior of gattrib is preserved.
>> ---
>> .gitignore | 1 +
>> Makefile.am | 7 ++
>> unit/test-gattrib.c | 350 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 358 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..23a5485
>> --- /dev/null
>> +++ b/unit/test-gattrib.c
>> @@ -0,0 +1,350 @@
>> +/*
>> + *
>> + * 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 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 challenge_response {
>> + const uint8_t *expect;
>> + const size_t expect_len;
>> + const uint8_t *respond;
>> + const size_t respond_len;
>> + bool received;
>> +};
>> +
>> +static gboolean test_client(GIOChannel *channel, GIOCondition cond,
>> + gpointer data)
>> +{
>> + struct challenge_response *cr = data;
>> + int fd;
>> + uint8_t buf[256];
>> + ssize_t len;
>> +
>> + 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_len);
>> +
>> + if (g_test_verbose())
>> + util_hexdump('>', buf, len, test_debug, "test_client: ");
>> +
>> + cr->received = true;
>> +
>> + if (cr->respond != NULL) {
>> + if (g_test_verbose())
>> + util_hexdump('<', cr->respond, cr->respond_len,
>> + test_debug, "test_client: ");
>> + len = write(fd, cr->respond, cr->respond_len);
>> +
>> + g_assert_cmpint(len, ==, cr->respond_len);
>> + }
>> +
>> + return TRUE;
>> +}
>> +
>> +struct result_data {
>> + guint8 status;
>> + const guint8 *pdu;
>> + guint16 len;
>> + bool done;
>> +};
>> +
>> +static void result_canary(guint8 status, const guint8 *pdu, guint16 len,
>> + gpointer data)
>> +{
>> + struct result_data *result = data;
>> + result->status = status;
>> + result->pdu = pdu;
>> + result->len = len;
>> +
>> + if (g_test_verbose())
>> + util_hexdump('<', pdu, len, test_debug, "result_canary: ");
>> +
>> + result->done = true;
>> +}
>> +
>> +static void test_send(struct context *cxt, gconstpointer unused)
>> +{
>> + int cmp;
>> + struct result_data results;
>> + struct challenge_response data = {
>> + .expect = data(0x02, 0x00, 0x02),
>> + .expect_len = 3,
>> + .respond = data(0x01, 0x02, 0x03, 0x04),
>> + .respond_len = 4,
>> + .received = false,
>> + };
>> +
>> + g_io_add_watch(cxt->server_io, G_IO_IN | G_IO_HUP | G_IO_ERR | G_IO_NVAL,
>> + test_client, &data);
>> +
>> + results.done = false;
>> +
>> + g_attrib_send(cxt->att, 0, data.expect, data.expect_len, result_canary,
>> + (gpointer) &results, NULL);
>> +
>> + /* Spin the main loop until we are ready. */
>> + do {
>> + g_main_context_iteration(NULL, TRUE);
>> + } while (!results.done);
>
> I do not understand why you have to manually iterate here? Im fact
> none of our unit tests uses g_main_context_iteration.
>
>> + g_assert_cmpint(results.len, ==, data.respond_len);
>> +
>> + cmp = memcmp(results.pdu, data.respond, results.len);
>> +
>> + g_assert(cmp == 0);
>> +}
>> +
>> +static void test_cancel(struct context *cxt, gconstpointer unused)
>> +{
>> + guint event_id;
>> + gboolean canceled;
>> + struct result_data results;
>> + struct challenge_response data = {
>> + .expect = data(0x02, 0x00, 0x02),
>> + .expect_len = 3,
>> + .respond = data(0x01, 0x02, 0x03, 0x04),
>> + .respond_len = 4,
>> + .received = false,
>> + };
>> +
>> + g_io_add_watch(cxt->server_io,
>> + G_IO_IN | G_IO_HUP | G_IO_ERR | G_IO_NVAL,
>> + test_client, &data);
>> +
>> + results.done = false;
>> + event_id = g_attrib_send(cxt->att, 0, data.expect, data.expect_len,
>> + result_canary, &results, NULL);
>> +
>> + /* Spin the main loop until we receive the first message */
>> + do {
>> + g_main_context_iteration(NULL, FALSE);
>> + } while (!data.received);
>> +
>> + canceled = g_attrib_cancel(cxt->att, event_id);
>> + g_assert(canceled);
>> +
>> + /*
>> + * Spin the main loop until all events are processed,
>> + * no result should be called because it was cancelled */
>> + do {
>> + g_main_context_iteration(NULL, TRUE);
>> + } while (g_main_context_pending(NULL));
>> +
>> + g_assert(!results.done);
>> +
>> + results.done = false;
>> + data.received = false;
>> + event_id = g_attrib_send(cxt->att, 0, data.expect, data.expect_len,
>> + result_canary, &results, NULL);
>> +
>> + canceled = g_attrib_cancel(cxt->att, event_id);
>> + g_assert(canceled);
>> +
>> + /*
>> + * Spin the main loop until all events are processed,
>> + * the message should never be delivered.
>> + */
>> + do {
>> + g_main_context_iteration(NULL, TRUE);
>> + } while (g_main_context_pending(NULL));
>> +
>> + g_assert(!data.received);
>> + g_assert(!results.done);
>> +
>> + /* 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);
>
> Init the log only in case of g_test_verbose is true.
>
>> + /*
>> + * 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);
>
> Overall I think using define_test such as in unit/test-gatt produces
> less code and is a little bit simpler to maintain, perhaps we should
> make the context a bit more generic so new unit test can just provide
> their own data but all the pdu handling is done in common place e.g.
> unit/test{.c,.h} which later make easier to port to something else
> other than glib.
>
The main purpose of this test is so that we can be confident when we
rewrite gattrib.c to route everything to an internal bt_att structure
(this is the first step in transitioning to the shared stack), so I
don't expect many more additions to this in the future. I don't have a
strong opinion on this though we will eventually remove this test once
everything has transitioned to the new stack anyway, so we don't want
to over-engineer this.
Cheers,
Arman
Hi Michael,
On Tue, Oct 28, 2014 at 12:13 AM, Michael Janssen <[email protected]> wrote:
> This will ensure that the API behavior of gattrib is preserved.
> ---
> .gitignore | 1 +
> Makefile.am | 7 ++
> unit/test-gattrib.c | 350 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 358 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..23a5485
> --- /dev/null
> +++ b/unit/test-gattrib.c
> @@ -0,0 +1,350 @@
> +/*
> + *
> + * 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 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 challenge_response {
> + const uint8_t *expect;
> + const size_t expect_len;
> + const uint8_t *respond;
> + const size_t respond_len;
> + bool received;
> +};
> +
> +static gboolean test_client(GIOChannel *channel, GIOCondition cond,
> + gpointer data)
> +{
> + struct challenge_response *cr = data;
> + int fd;
> + uint8_t buf[256];
> + ssize_t len;
> +
> + 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_len);
> +
> + if (g_test_verbose())
> + util_hexdump('>', buf, len, test_debug, "test_client: ");
> +
> + cr->received = true;
> +
> + if (cr->respond != NULL) {
> + if (g_test_verbose())
> + util_hexdump('<', cr->respond, cr->respond_len,
> + test_debug, "test_client: ");
> + len = write(fd, cr->respond, cr->respond_len);
> +
> + g_assert_cmpint(len, ==, cr->respond_len);
> + }
> +
> + return TRUE;
> +}
> +
> +struct result_data {
> + guint8 status;
> + const guint8 *pdu;
> + guint16 len;
> + bool done;
> +};
> +
> +static void result_canary(guint8 status, const guint8 *pdu, guint16 len,
> + gpointer data)
> +{
> + struct result_data *result = data;
> + result->status = status;
> + result->pdu = pdu;
> + result->len = len;
> +
> + if (g_test_verbose())
> + util_hexdump('<', pdu, len, test_debug, "result_canary: ");
> +
> + result->done = true;
> +}
> +
> +static void test_send(struct context *cxt, gconstpointer unused)
> +{
> + int cmp;
> + struct result_data results;
> + struct challenge_response data = {
> + .expect = data(0x02, 0x00, 0x02),
> + .expect_len = 3,
> + .respond = data(0x01, 0x02, 0x03, 0x04),
> + .respond_len = 4,
> + .received = false,
> + };
> +
> + g_io_add_watch(cxt->server_io, G_IO_IN | G_IO_HUP | G_IO_ERR | G_IO_NVAL,
> + test_client, &data);
> +
> + results.done = false;
> +
> + g_attrib_send(cxt->att, 0, data.expect, data.expect_len, result_canary,
> + (gpointer) &results, NULL);
> +
> + /* Spin the main loop until we are ready. */
> + do {
> + g_main_context_iteration(NULL, TRUE);
> + } while (!results.done);
I do not understand why you have to manually iterate here? Im fact
none of our unit tests uses g_main_context_iteration.
> + g_assert_cmpint(results.len, ==, data.respond_len);
> +
> + cmp = memcmp(results.pdu, data.respond, results.len);
> +
> + g_assert(cmp == 0);
> +}
> +
> +static void test_cancel(struct context *cxt, gconstpointer unused)
> +{
> + guint event_id;
> + gboolean canceled;
> + struct result_data results;
> + struct challenge_response data = {
> + .expect = data(0x02, 0x00, 0x02),
> + .expect_len = 3,
> + .respond = data(0x01, 0x02, 0x03, 0x04),
> + .respond_len = 4,
> + .received = false,
> + };
> +
> + g_io_add_watch(cxt->server_io,
> + G_IO_IN | G_IO_HUP | G_IO_ERR | G_IO_NVAL,
> + test_client, &data);
> +
> + results.done = false;
> + event_id = g_attrib_send(cxt->att, 0, data.expect, data.expect_len,
> + result_canary, &results, NULL);
> +
> + /* Spin the main loop until we receive the first message */
> + do {
> + g_main_context_iteration(NULL, FALSE);
> + } while (!data.received);
> +
> + canceled = g_attrib_cancel(cxt->att, event_id);
> + g_assert(canceled);
> +
> + /*
> + * Spin the main loop until all events are processed,
> + * no result should be called because it was cancelled */
> + do {
> + g_main_context_iteration(NULL, TRUE);
> + } while (g_main_context_pending(NULL));
> +
> + g_assert(!results.done);
> +
> + results.done = false;
> + data.received = false;
> + event_id = g_attrib_send(cxt->att, 0, data.expect, data.expect_len,
> + result_canary, &results, NULL);
> +
> + canceled = g_attrib_cancel(cxt->att, event_id);
> + g_assert(canceled);
> +
> + /*
> + * Spin the main loop until all events are processed,
> + * the message should never be delivered.
> + */
> + do {
> + g_main_context_iteration(NULL, TRUE);
> + } while (g_main_context_pending(NULL));
> +
> + g_assert(!data.received);
> + g_assert(!results.done);
> +
> + /* 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);
Init the log only in case of g_test_verbose is true.
> + /*
> + * 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);
Overall I think using define_test such as in unit/test-gatt produces
less code and is a little bit simpler to maintain, perhaps we should
make the context a bit more generic so new unit test can just provide
their own data but all the pdu handling is done in common place e.g.
unit/test{.c,.h} which later make easier to port to something else
other than glib.
--
Luiz Augusto von Dentz
Memory is not guaranteed to stay around after the result
function is called, so we must copy the PDU.
---
unit/test-gattrib.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/unit/test-gattrib.c b/unit/test-gattrib.c
index 0ccf09a..e8d7c84 100644
--- a/unit/test-gattrib.c
+++ b/unit/test-gattrib.c
@@ -203,7 +203,7 @@ static gboolean test_client(GIOChannel *channel, GIOCondition cond,
struct result_data {
guint8 status;
- const guint8 *pdu;
+ guint8 *pdu;
guint16 len;
bool done;
};
@@ -212,8 +212,10 @@ static void result_canary(guint8 status, const guint8 *pdu, guint16 len,
gpointer data)
{
struct result_data *result = data;
+
result->status = status;
- result->pdu = pdu;
+ result->pdu = g_malloc0(len);
+ memcpy(result->pdu, pdu, len);
result->len = len;
if (g_test_verbose())
@@ -229,7 +231,7 @@ static void test_send(struct context *cxt, gconstpointer unused)
struct challenge_response data = {
.expect = data(0x02, 0x00, 0x02),
.expect_len = 3,
- .respond = data(0x01, 0x02, 0x03, 0x04),
+ .respond = data(0x03, 0x02, 0x03, 0x04),
.respond_len = 4,
.received = false,
};
@@ -251,6 +253,8 @@ static void test_send(struct context *cxt, gconstpointer unused)
cmp = memcmp(results.pdu, data.respond, results.len);
+ g_free(results.pdu);
+
g_assert(cmp == 0);
}
@@ -262,7 +266,7 @@ static void test_cancel(struct context *cxt, gconstpointer unused)
struct challenge_response data = {
.expect = data(0x02, 0x00, 0x02),
.expect_len = 3,
- .respond = data(0x01, 0x02, 0x03, 0x04),
+ .respond = data(0x03, 0x02, 0x03, 0x04),
.respond_len = 4,
.received = false,
};
--
2.1.0.rc2.206.gedb03e5
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 61db4a1..f93ed35 100644
--- a/unit/test-gattrib.c
+++ b/unit/test-gattrib.c
@@ -376,7 +376,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,
@@ -406,6 +406,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
@@ -418,7 +419,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");
@@ -429,7 +435,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");
@@ -440,7 +451,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");
@@ -451,11 +467,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
Probing for the MTU using bt_io is problematic for
testing because you cannot impersonate AF_BLUETOOTH sockets
with a socketpair.
---
attrib/gattrib.c | 13 +------------
1 file changed, 1 insertion(+), 12 deletions(-)
diff --git a/attrib/gattrib.c b/attrib/gattrib.c
index 71d1cef..fa41ade 100644
--- a/attrib/gattrib.c
+++ b/attrib/gattrib.c
@@ -476,27 +476,17 @@ done:
GAttrib *g_attrib_new(GIOChannel *io)
{
struct _GAttrib *attrib;
- uint16_t imtu;
uint16_t att_mtu;
uint16_t cid;
- GError *gerr = NULL;
g_io_channel_set_encoding(io, NULL, NULL);
g_io_channel_set_buffered(io, FALSE);
- bt_io_get(io, &gerr, BT_IO_OPT_IMTU, &imtu,
- BT_IO_OPT_CID, &cid, BT_IO_OPT_INVALID);
- if (gerr) {
- error("%s", gerr->message);
- g_error_free(gerr);
- return NULL;
- }
-
attrib = g_try_new0(struct _GAttrib, 1);
if (attrib == NULL)
return NULL;
- att_mtu = (cid == ATT_CID) ? ATT_DEFAULT_LE_MTU : imtu;
+ att_mtu = ATT_DEFAULT_LE_MTU;
attrib->buf = g_malloc0(att_mtu);
attrib->buflen = att_mtu;
@@ -651,7 +641,6 @@ gboolean g_attrib_cancel_all(GAttrib *attrib)
return ret;
}
-
uint8_t *g_attrib_get_buffer(GAttrib *attrib, size_t *len)
{
if (len == NULL)
--
2.1.0.rc2.206.gedb03e5
This will ensure that the API behavior of gattrib is preserved.
---
.gitignore | 1 +
Makefile.am | 7 ++
unit/test-gattrib.c | 350 ++++++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 358 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..23a5485
--- /dev/null
+++ b/unit/test-gattrib.c
@@ -0,0 +1,350 @@
+/*
+ *
+ * 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 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 challenge_response {
+ const uint8_t *expect;
+ const size_t expect_len;
+ const uint8_t *respond;
+ const size_t respond_len;
+ bool received;
+};
+
+static gboolean test_client(GIOChannel *channel, GIOCondition cond,
+ gpointer data)
+{
+ struct challenge_response *cr = data;
+ int fd;
+ uint8_t buf[256];
+ ssize_t len;
+
+ 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_len);
+
+ if (g_test_verbose())
+ util_hexdump('>', buf, len, test_debug, "test_client: ");
+
+ cr->received = true;
+
+ if (cr->respond != NULL) {
+ if (g_test_verbose())
+ util_hexdump('<', cr->respond, cr->respond_len,
+ test_debug, "test_client: ");
+ len = write(fd, cr->respond, cr->respond_len);
+
+ g_assert_cmpint(len, ==, cr->respond_len);
+ }
+
+ return TRUE;
+}
+
+struct result_data {
+ guint8 status;
+ const guint8 *pdu;
+ guint16 len;
+ bool done;
+};
+
+static void result_canary(guint8 status, const guint8 *pdu, guint16 len,
+ gpointer data)
+{
+ struct result_data *result = data;
+ result->status = status;
+ result->pdu = pdu;
+ result->len = len;
+
+ if (g_test_verbose())
+ util_hexdump('<', pdu, len, test_debug, "result_canary: ");
+
+ result->done = true;
+}
+
+static void test_send(struct context *cxt, gconstpointer unused)
+{
+ int cmp;
+ struct result_data results;
+ struct challenge_response data = {
+ .expect = data(0x02, 0x00, 0x02),
+ .expect_len = 3,
+ .respond = data(0x01, 0x02, 0x03, 0x04),
+ .respond_len = 4,
+ .received = false,
+ };
+
+ g_io_add_watch(cxt->server_io, G_IO_IN | G_IO_HUP | G_IO_ERR | G_IO_NVAL,
+ test_client, &data);
+
+ results.done = false;
+
+ g_attrib_send(cxt->att, 0, data.expect, data.expect_len, result_canary,
+ (gpointer) &results, NULL);
+
+ /* Spin the main loop until we are ready. */
+ do {
+ g_main_context_iteration(NULL, TRUE);
+ } while (!results.done);
+
+ g_assert_cmpint(results.len, ==, data.respond_len);
+
+ cmp = memcmp(results.pdu, data.respond, results.len);
+
+ g_assert(cmp == 0);
+}
+
+static void test_cancel(struct context *cxt, gconstpointer unused)
+{
+ guint event_id;
+ gboolean canceled;
+ struct result_data results;
+ struct challenge_response data = {
+ .expect = data(0x02, 0x00, 0x02),
+ .expect_len = 3,
+ .respond = data(0x01, 0x02, 0x03, 0x04),
+ .respond_len = 4,
+ .received = false,
+ };
+
+ g_io_add_watch(cxt->server_io,
+ G_IO_IN | G_IO_HUP | G_IO_ERR | G_IO_NVAL,
+ test_client, &data);
+
+ results.done = false;
+ event_id = g_attrib_send(cxt->att, 0, data.expect, data.expect_len,
+ result_canary, &results, NULL);
+
+ /* Spin the main loop until we receive the first message */
+ do {
+ g_main_context_iteration(NULL, FALSE);
+ } while (!data.received);
+
+ canceled = g_attrib_cancel(cxt->att, event_id);
+ g_assert(canceled);
+
+ /*
+ * Spin the main loop until all events are processed,
+ * no result should be called because it was cancelled */
+ do {
+ g_main_context_iteration(NULL, TRUE);
+ } while (g_main_context_pending(NULL));
+
+ g_assert(!results.done);
+
+ results.done = false;
+ data.received = false;
+ event_id = g_attrib_send(cxt->att, 0, data.expect, data.expect_len,
+ result_canary, &results, NULL);
+
+ canceled = g_attrib_cancel(cxt->att, event_id);
+ g_assert(canceled);
+
+ /*
+ * Spin the main loop until all events are processed,
+ * the message should never be delivered.
+ */
+ do {
+ g_main_context_iteration(NULL, TRUE);
+ } while (g_main_context_pending(NULL));
+
+ g_assert(!data.received);
+ g_assert(!results.done);
+
+ /* 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
---
unit/test-gattrib.c | 157 +++++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 156 insertions(+), 1 deletion(-)
diff --git a/unit/test-gattrib.c b/unit/test-gattrib.c
index 23a5485..61db4a1 100644
--- a/unit/test-gattrib.c
+++ b/unit/test-gattrib.c
@@ -45,6 +45,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;
@@ -298,9 +315,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);
+ do {
+ g_main_context_iteration(NULL, TRUE);
+ } while (g_main_context_pending(NULL));
+ cur_pdu->sent = true;
+ }
+}
+
+#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
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 | 10 +++-------
attrib/gattrib.h | 2 +-
attrib/gatttool.c | 17 ++++++++++++++++-
attrib/interactive.c | 17 ++++++++++++++++-
src/device.c | 11 ++++++++++-
5 files changed, 46 insertions(+), 11 deletions(-)
diff --git a/attrib/gattrib.c b/attrib/gattrib.c
index fa41ade..a6511a2 100644
--- a/attrib/gattrib.c
+++ b/attrib/gattrib.c
@@ -473,11 +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;
- uint16_t cid;
g_io_channel_set_encoding(io, NULL, NULL);
g_io_channel_set_buffered(io, FALSE);
@@ -486,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 875a5c5..0925951 100644
--- a/src/device.c
+++ b/src/device.c
@@ -3627,11 +3627,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;
}
@@ -3649,7 +3655,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
---
unit/test-gattrib.c | 38 ++++++++++++++++++++++++++++++--------
1 file changed, 30 insertions(+), 8 deletions(-)
diff --git a/unit/test-gattrib.c b/unit/test-gattrib.c
index f93ed35..0ccf09a 100644
--- a/unit/test-gattrib.c
+++ b/unit/test-gattrib.c
@@ -184,7 +184,8 @@ static gboolean test_client(GIOChannel *channel, GIOCondition cond,
g_assert_cmpint(len, ==, cr->expect_len);
if (g_test_verbose())
- util_hexdump('>', buf, len, test_debug, "test_client: ");
+ util_hexdump('?', cr->expect, cr->expect_len, test_debug,
+ "test_client: ");
cr->received = true;
@@ -330,7 +331,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);
do {
@@ -342,13 +343,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())
@@ -371,6 +379,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)
@@ -407,15 +423,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);
@@ -429,9 +449,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);
@@ -445,9 +466,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);
@@ -461,9 +483,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
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
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
This function is not used, and also not implemented.
---
attrib/gattrib.c | 5 -----
attrib/gattrib.h | 3 ---
2 files changed, 8 deletions(-)
diff --git a/attrib/gattrib.c b/attrib/gattrib.c
index 912dffb..71d1cef 100644
--- a/attrib/gattrib.c
+++ b/attrib/gattrib.c
@@ -651,11 +651,6 @@ gboolean g_attrib_cancel_all(GAttrib *attrib)
return ret;
}
-gboolean g_attrib_set_debug(GAttrib *attrib,
- GAttribDebugFunc func, gpointer user_data)
-{
- return TRUE;
-}
uint8_t *g_attrib_get_buffer(GAttrib *attrib, size_t *len)
{
diff --git a/attrib/gattrib.h b/attrib/gattrib.h
index 3fe92c7..18df2ad 100644
--- a/attrib/gattrib.h
+++ b/attrib/gattrib.h
@@ -58,9 +58,6 @@ guint g_attrib_send(GAttrib *attrib, guint id, const guint8 *pdu, guint16 len,
gboolean g_attrib_cancel(GAttrib *attrib, guint id);
gboolean g_attrib_cancel_all(GAttrib *attrib);
-gboolean g_attrib_set_debug(GAttrib *attrib,
- GAttribDebugFunc func, gpointer user_data);
-
guint g_attrib_register(GAttrib *attrib, guint8 opcode, guint16 handle,
GAttribNotifyFunc func, gpointer user_data,
GDestroyNotify notify);
--
2.1.0.rc2.206.gedb03e5