There is no need to destroy these queues here, there are destroyed in
hog_free. Prevents from memory violation.
---
android/hog.c | 16 ++--------------
1 file changed, 2 insertions(+), 14 deletions(-)
diff --git a/android/hog.c b/android/hog.c
index 88a5460..9f1fbca 100644
--- a/android/hog.c
+++ b/android/hog.c
@@ -1180,23 +1180,11 @@ struct bt_hog *bt_hog_new(const char *name, uint16_t vendor, uint16_t product,
return NULL;
hog->gatt_op = queue_new();
- if (!hog->gatt_op) {
- hog_free(hog);
- return NULL;
- }
-
hog->bas = queue_new();
- if (!hog->bas) {
- queue_destroy(hog->gatt_op, NULL);
- hog_free(hog);
- return NULL;
- }
-
hog->uhid = bt_uhid_new_default();
- if (!hog->uhid) {
+
+ if (!(hog->gatt_op && hog->bas && hog->uhid)) {
hog_free(hog);
- queue_destroy(hog->gatt_op, NULL);
- queue_destroy(hog->bas, NULL);
return NULL;
}
--
1.9.1
Hi Mariusz,
On Thursday 12 of March 2015 15:48:11 Mariusz Skamra wrote:
> There is no need to destroy these queues here, there are destroyed in
> hog_free. Prevents from memory violation.
> ---
> android/hog.c | 16 ++--------------
> 1 file changed, 2 insertions(+), 14 deletions(-)
>
> diff --git a/android/hog.c b/android/hog.c
> index 88a5460..8133303 100644
> --- a/android/hog.c
> +++ b/android/hog.c
> @@ -1180,23 +1180,11 @@ struct bt_hog *bt_hog_new(const char *name, uint16_t
> vendor, uint16_t product, return NULL;
>
> hog->gatt_op = queue_new();
> - if (!hog->gatt_op) {
> - hog_free(hog);
> - return NULL;
> - }
> -
> hog->bas = queue_new();
> - if (!hog->bas) {
> - queue_destroy(hog->gatt_op, NULL);
> - hog_free(hog);
> - return NULL;
> - }
> -
> hog->uhid = bt_uhid_new_default();
> - if (!hog->uhid) {
> +
> + if (!hog->gatt_op || !hog->bas || !hog->uhid) {
> hog_free(hog);
> - queue_destroy(hog->gatt_op, NULL);
> - queue_destroy(hog->bas, NULL);
> return NULL;
> }
This patch is now applied, thanks.
--
BR
Szymon Janc
There is no need to destroy these queues here, there are destroyed in
hog_free. Prevents from memory violation.
---
android/hog.c | 16 ++--------------
1 file changed, 2 insertions(+), 14 deletions(-)
diff --git a/android/hog.c b/android/hog.c
index 88a5460..8133303 100644
--- a/android/hog.c
+++ b/android/hog.c
@@ -1180,23 +1180,11 @@ struct bt_hog *bt_hog_new(const char *name, uint16_t vendor, uint16_t product,
return NULL;
hog->gatt_op = queue_new();
- if (!hog->gatt_op) {
- hog_free(hog);
- return NULL;
- }
-
hog->bas = queue_new();
- if (!hog->bas) {
- queue_destroy(hog->gatt_op, NULL);
- hog_free(hog);
- return NULL;
- }
-
hog->uhid = bt_uhid_new_default();
- if (!hog->uhid) {
+
+ if (!hog->gatt_op || !hog->bas || !hog->uhid) {
hog_free(hog);
- queue_destroy(hog->gatt_op, NULL);
- queue_destroy(hog->bas, NULL);
return NULL;
}
--
1.9.1
Hi Mariusz,
On Thursday 12 of March 2015 14:37:34 Mariusz Skamra wrote:
> There is no need to destroy these queues here, there are destroyed in
> hog_free. Prevents from memory violation.
> ---
> android/hog.c | 16 ++--------------
> 1 file changed, 2 insertions(+), 14 deletions(-)
>
> diff --git a/android/hog.c b/android/hog.c
> index 88a5460..9f1fbca 100644
> --- a/android/hog.c
> +++ b/android/hog.c
> @@ -1180,23 +1180,11 @@ struct bt_hog *bt_hog_new(const char *name, uint16_t
> vendor, uint16_t product, return NULL;
>
> hog->gatt_op = queue_new();
> - if (!hog->gatt_op) {
> - hog_free(hog);
> - return NULL;
> - }
> -
> hog->bas = queue_new();
> - if (!hog->bas) {
> - queue_destroy(hog->gatt_op, NULL);
> - hog_free(hog);
> - return NULL;
> - }
> -
> hog->uhid = bt_uhid_new_default();
> - if (!hog->uhid) {
> +
> + if (!(hog->gatt_op && hog->bas && hog->uhid)) {
Please make this if (!hog->gatt_op || !hog->bas || !hog->uhid)
> hog_free(hog);
> - queue_destroy(hog->gatt_op, NULL);
> - queue_destroy(hog->bas, NULL);
> return NULL;
> }
--
BR
Szymon Janc
Verify that all Report Map characteristics can be read by
Report Host IUT
---
Makefile.am | 15 ++++
unit/test-hog.c | 274 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 289 insertions(+)
create mode 100644 unit/test-hog.c
diff --git a/Makefile.am b/Makefile.am
index ba32a36..885a829 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -382,6 +382,21 @@ unit_tests += unit/test-gatt
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-hog
+
+unit_test_hog_SOURCES = unit/test-hog.c \
+ $(btio_sources) \
+ android/hog.h android/hog.c \
+ android/scpp.h android/scpp.c \
+ android/bas.h android/bas.c \
+ android/dis.h android/dis.c \
+ src/log.h src/log.c \
+ attrib/att.h attrib/att.c \
+ attrib/gatt.h attrib/gatt.c \
+ attrib/gattrib.h attrib/gattrib.c
+unit_test_hog_LDADD = src/libshared-glib.la \
+ lib/libbluetooth-internal.la @GLIB_LIBS@
unit_tests += unit/test-gattrib
diff --git a/unit/test-hog.c b/unit/test-hog.c
new file mode 100644
index 0000000..941e860
--- /dev/null
+++ b/unit/test-hog.c
@@ -0,0 +1,274 @@
+/*
+ *
+ * BlueZ - Bluetooth protocol stack for Linux
+ *
+ * Copyright (C) 2015 Intel Corporation. All rights reserved.
+ *
+ *
+ * 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 <string.h>
+#include <sys/socket.h>
+
+#include <glib.h>
+
+#include "src/shared/util.h"
+#include "src/shared/tester.h"
+
+#include "attrib/gattrib.h"
+
+#include "android/hog.h"
+
+struct test_pdu {
+ bool valid;
+ const uint8_t *data;
+ size_t size;
+};
+
+struct test_data {
+ char *test_name;
+ struct test_pdu *pdu_list;
+};
+
+struct context {
+ GAttrib *attrib;
+ struct bt_hog *hog;
+ guint source;
+ guint process;
+ int fd;
+ unsigned int pdu_offset;
+ const struct test_data *data;
+};
+
+#define data(args...) ((const unsigned char[]) { args })
+
+#define raw_pdu(args...) \
+{ \
+ .valid = true, \
+ .data = data(args), \
+ .size = sizeof(data(args)),\
+}
+
+#define false_pdu() \
+{ \
+ .valid = false, \
+}
+
+#define define_test(name, function, args...) \
+ do { \
+ const struct test_pdu pdus[] = { \
+ args, { } \
+ }; \
+ static struct test_data data; \
+ data.test_name = g_strdup(name); \
+ data.pdu_list = g_malloc(sizeof(pdus)); \
+ memcpy(data.pdu_list, pdus, sizeof(pdus)); \
+ tester_add(name, &data, NULL, function, NULL); \
+ } while (0)
+
+static void test_debug(const char *str, void *user_data)
+{
+ const char *prefix = user_data;
+
+ tester_debug("%s%s", prefix, str);
+}
+
+static gboolean context_quit(gpointer user_data)
+{
+ struct context *context = user_data;
+
+ if (context->process > 0)
+ g_source_remove(context->process);
+
+ if (context->source > 0)
+ g_source_remove(context->source);
+
+ bt_hog_unref(context->hog);
+
+ g_attrib_unref(context->attrib);
+
+ g_free(context);
+
+ tester_test_passed();
+
+ return FALSE;
+}
+
+static gboolean send_pdu(gpointer user_data)
+{
+ struct context *context = user_data;
+ const struct test_pdu *pdu;
+ ssize_t len;
+
+ pdu = &context->data->pdu_list[context->pdu_offset++];
+
+ len = write(context->fd, pdu->data, pdu->size);
+
+ util_hexdump('<', pdu->data, len, test_debug, "hog: ");
+
+ g_assert_cmpint(len, ==, pdu->size);
+
+ context->process = 0;
+
+ if (!context->data->pdu_list[context->pdu_offset].valid)
+ context_quit(context);
+
+ return FALSE;
+}
+
+static gboolean test_handler(GIOChannel *channel, GIOCondition cond,
+ gpointer user_data)
+{
+ struct context *context = user_data;
+ unsigned char buf[512];
+ const struct test_pdu *pdu;
+ ssize_t len;
+ int fd;
+
+ pdu = &context->data->pdu_list[context->pdu_offset++];
+
+ if (cond & (G_IO_NVAL | G_IO_ERR | G_IO_HUP)) {
+ context->source = 0;
+ g_print("%s: cond %x\n", __func__, cond);
+ return FALSE;
+ }
+
+ fd = g_io_channel_unix_get_fd(channel);
+
+ len = read(fd, buf, sizeof(buf));
+
+ g_assert(len > 0);
+
+ util_hexdump('>', buf, len, test_debug, "hog: ");
+
+ g_assert_cmpint(len, ==, pdu->size);
+
+ g_assert(memcmp(buf, pdu->data, pdu->size) == 0);
+
+ context->process = g_idle_add(send_pdu, context);
+
+ return TRUE;
+}
+
+static struct context *create_context(gconstpointer data)
+{
+ struct context *context;
+ GIOChannel *channel, *att_io;
+ int err, sv[2];
+ char name[] = "bluez-hog";
+ uint16_t vendor = 0x0002;
+ uint16_t product = 0x0001;
+ uint16_t version = 0x0001;
+
+ context = g_new0(struct context, 1);
+ err = socketpair(AF_UNIX, SOCK_SEQPACKET | SOCK_CLOEXEC, 0, sv);
+ g_assert(err == 0);
+
+ att_io = g_io_channel_unix_new(sv[0]);
+
+ g_io_channel_set_close_on_unref(att_io, TRUE);
+
+ context->attrib = g_attrib_new(att_io, 23);
+ g_assert(context->attrib);
+
+ g_io_channel_unref(att_io);
+
+ context->hog = bt_hog_new(name, vendor, product, version, NULL);
+ g_assert(context->hog);
+
+ channel = g_io_channel_unix_new(sv[1]);
+
+ g_io_channel_set_close_on_unref(channel, TRUE);
+ g_io_channel_set_encoding(channel, NULL, NULL);
+ g_io_channel_set_buffered(channel, FALSE);
+
+ context->source = g_io_add_watch(channel,
+ G_IO_IN | G_IO_HUP | G_IO_ERR | G_IO_NVAL,
+ test_handler, context);
+ g_assert(context->source > 0);
+
+ g_io_channel_unref(channel);
+
+ context->fd = sv[1];
+ context->data = data;
+
+ return context;
+}
+
+static void test_hog(gconstpointer data)
+{
+ struct context *context = create_context(data);
+
+ g_assert(bt_hog_attach(context->hog, context->attrib));
+}
+
+int main(int argc, char *argv[])
+{
+ tester_init(&argc, &argv);
+
+ define_test("/TP/HGRF/RH/BV-01-I", test_hog,
+ raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28),
+ raw_pdu(0x11, 0x06, 0x01, 0x00, 0x04, 0x00, 0x12,
+ 0x18, 0x05, 0x00, 0x08, 0x00, 0x12, 0x18),
+ raw_pdu(0x10, 0x09, 0x00, 0xff, 0xff, 0x00, 0x28),
+ raw_pdu(0x01, 0x10, 0x09, 0x00, 0x0a),
+ raw_pdu(0x08, 0x01, 0x00, 0x04, 0x00, 0x03, 0x28),
+ raw_pdu(0x09, 0x07, 0x03, 0x00, 0x02, 0x04, 0x00,
+ 0x4b, 0x2a),
+ raw_pdu(0x08, 0x01, 0x00, 0x04, 0x00, 0x02, 0x28),
+ raw_pdu(0x01, 0x08, 0x01, 0x00, 0x0a),
+ raw_pdu(0x08, 0x05, 0x00, 0x08, 0x00, 0x02, 0x28),
+ raw_pdu(0x01, 0x08, 0x05, 0x00, 0x0a),
+ raw_pdu(0x08, 0x05, 0x00, 0x08, 0x00, 0x03, 0x28),
+ raw_pdu(0x09, 0x07, 0x07, 0x00, 0x02, 0x08, 0x00,
+ 0x4b, 0x2a),
+ raw_pdu(0x0a, 0x04, 0x00),
+ raw_pdu(0x0b, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06,
+ 0x07, 0x08, 0x09, 0x0a, 0x0b, 0x0c, 0x0d,
+ 0x0e, 0x0f, 0x10, 0x11, 0x12, 0x13, 0x14,
+ 0x15, 0x16),
+ raw_pdu(0x0a, 0x08, 0x00),
+ raw_pdu(0x0b, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06,
+ 0x07, 0x08, 0x09, 0x0a, 0x0b, 0x0c, 0x0d,
+ 0x0e, 0x0f, 0x10, 0x11, 0x12, 0x13, 0x14,
+ 0x15, 0x16),
+ raw_pdu(0x0c, 0x04, 0x00, 0x16, 0x00),
+ raw_pdu(0x0d, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06,
+ 0x07, 0x08, 0x09, 0x0a, 0x0b, 0x0c, 0x0d,
+ 0x0e, 0x0f, 0x10, 0x11, 0x12, 0x13, 0x14,
+ 0x15, 0x16),
+ raw_pdu(0x0c, 0x08, 0x00, 0x16, 0x00),
+ raw_pdu(0x0d, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06,
+ 0x07, 0x08, 0x09, 0x0a, 0x0b, 0x0c, 0x0d,
+ 0x0e, 0x0f, 0x10, 0x11, 0x12, 0x13, 0x14,
+ 0x15, 0x16),
+ raw_pdu(0x0c, 0x04, 0x00, 0x2c, 0x00),
+ raw_pdu(0x0d, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06,
+ 0x07, 0x08, 0x09, 0x0a, 0x0b, 0x0c, 0x0d,
+ 0x0e, 0x0f, 0x10, 0x11, 0x12, 0x13),
+ raw_pdu(0x0c, 0x08, 0x00, 0x2c, 0x00),
+ raw_pdu(0x0d, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06,
+ 0x07, 0x08, 0x09, 0x0a, 0x0b, 0x0c, 0x0d,
+ 0x0e, 0x0f, 0x10, 0x11, 0x12, 0x13));
+
+ return tester_run();
+}
--
1.9.1