[PATCH v6 0/7] Enhanced support for extended inquiry response

[PATCH v6 0/7] Enhanced support for extended inquiry response

[Inga Stotland]

EIR needs to be updated whenever local SDP record database changes.
Previously, when an SDP record was added/removed/updated, EIR would 
be written in callback fired off HCI "Command Complete" event set off by
"Write Class Of Device" command. However, if the class of device did not
need to be updated, the EIR write was not happening either. This
implementation has been augmented to write new EIR when modification
in SDP database does not lead to updating of class of device.

Added support for UUID128 service descriptors in local EIR. 

Service UUIDs are written to "device properties" dictionary  when emitting
"Device Found" signal. This allows peek at available services offered by 
a remote device without establishing SDP connection.


-- 
Inga Stotland
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

[PATCH 1/7] Spec constants for Extended Inquiry Response field types

[Inga Stotland]

---
 src/sdpd.h |   13 +++++++++++++
 1 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/src/sdpd.h b/src/sdpd.h
index e93b0b6..5bab869 100644
--- a/src/sdpd.h
+++ b/src/sdpd.h
@@ -34,6 +34,19 @@
 #define SDPDBG(fmt...)
 #endif
 
+#define EIR_DATA_LENGTH  240
+
+#define EIR_UUID16_SOME             0x02  /* 16-bit UUID, more available */
+#define EIR_UUID16_ALL              0x03  /* 16-bit UUID, all listed */
+#define EIR_UUID32_SOME             0x04  /* 32-bit UUID, more available */
+#define EIR_UUID32_ALL              0x05  /* 32-bit UUID, all listed */
+#define EIR_UUID128_SOME            0x06  /* 128-bit UUID, more available */
+#define EIR_UUID128_ALL             0x07  /* 128-bit UUID, all listed */
+#define EIR_NAME_SHORT              0x08  /* shortened local name */
+#define EIR_NAME_COMPLETE           0x09  /* complete local name */
+#define EIR_TX_POWER                0x0A  /* transmit power level */
+#define EIR_DEVICE_ID               0x10  /* device ID */
+
 typedef struct request {
 	bdaddr_t device;
 	bdaddr_t bdaddr;
-- 
1.7.2

-- 
Inga Stotland
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

[PATCH 2/7] Minor fix when skipping duplicate UUID16 from EIR.

[Inga Stotland]

---
 src/sdpd-service.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/src/sdpd-service.c b/src/sdpd-service.c
index cdbb4f4..35e333d 100644
--- a/src/sdpd-service.c
+++ b/src/sdpd-service.c
@@ -242,7 +242,7 @@ void create_ext_inquiry_response(const char *name,
 			if (uuid[i] == rec->svclass.value.uuid16)
 				break;
 
-		if (i == index - 1)
+		if (i < index)
 			continue;
 
 		uuid[index++] = rec->svclass.value.uuid16;
-- 
1.7.2

-- 
Inga Stotland
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

[PATCH 3/7] Clean up code that generates extended inquiry response.

[Inga Stotland]

---
 src/sdpd-service.c |   44 +++++++++++++++++++++++++++++---------------
 1 files changed, 29 insertions(+), 15 deletions(-)

diff --git a/src/sdpd-service.c b/src/sdpd-service.c
index 35e333d..50c0a97 100644
--- a/src/sdpd-service.c
+++ b/src/sdpd-service.c
@@ -180,35 +180,41 @@ void create_ext_inquiry_response(const char *name,
 {
 	sdp_list_t *list = services;
 	uint8_t *ptr = data;
-	uint16_t uuid[24];
+	uint16_t eir_len = 0;
+	uint16_t uuid16[EIR_DATA_LENGTH / 2];
 	int i, index = 0;
+	gboolean truncated = FALSE;
 
 	if (name) {
 		int len = strlen(name);
 
+		/* EIR Data type */
 		if (len > 48) {
 			len = 48;
-			ptr[1] = 0x08;
+			ptr[1] = EIR_NAME_SHORT;
 		} else
-			ptr[1] = 0x09;
+			ptr[1] = EIR_NAME_COMPLETE;
 
+		/* EIR Data length */
 		ptr[0] = len + 1;
 
 		memcpy(ptr + 2, name, len);
 
-		ptr += len + 2;
+		eir_len += (len + 2);
+		ptr += (len + 2);
 	}
 
 	if (tx_power != 0) {
 		*ptr++ = 2;
-		*ptr++ = 0x0a;
+		*ptr++ = EIR_TX_POWER;
 		*ptr++ = (uint8_t) tx_power;
+		eir_len += 3;
 	}
 
 	if (did_vendor != 0x0000) {
 		uint16_t source = 0x0002;
 		*ptr++ = 9;
-		*ptr++ = 0x10;
+		*ptr++ = EIR_DEVICE_ID;
 		*ptr++ = (source & 0x00ff);
 		*ptr++ = (source & 0xff00) >> 8;
 		*ptr++ = (did_vendor & 0x00ff);
@@ -217,10 +223,10 @@ void create_ext_inquiry_response(const char *name,
 		*ptr++ = (did_product & 0xff00) >> 8;
 		*ptr++ = (did_version & 0x00ff);
 		*ptr++ = (did_version & 0xff00) >> 8;
+		eir_len += 10;
 	}
 
-	ptr[1] = 0x03;
-
+	/* Group all UUID16 types */
 	for (; list; list = list->next) {
 		sdp_record_t *rec = (sdp_record_t *) list->data;
 
@@ -233,28 +239,36 @@ void create_ext_inquiry_response(const char *name,
 		if (rec->svclass.value.uuid16 == PNP_INFO_SVCLASS_ID)
 			continue;
 
-		if (index > 23) {
-			ptr[1] = 0x02;
+		/* Stop if not enough space to put next UUID16 */
+		if ((eir_len + 2 + sizeof(uint16_t)) > EIR_DATA_LENGTH) {
+			truncated = TRUE;
 			break;
 		}
 
+		/* Check for duplicates */
 		for (i = 0; i < index; i++)
-			if (uuid[i] == rec->svclass.value.uuid16)
+			if (uuid16[i] == rec->svclass.value.uuid16)
 				break;
 
 		if (i < index)
 			continue;
 
-		uuid[index++] = rec->svclass.value.uuid16;
+		uuid16[index++] = rec->svclass.value.uuid16;
+		eir_len += sizeof(uint16_t);
 	}
 
 	if (index > 0) {
-		ptr[0] = (index * 2) + 1;
+		/* EIR Data length */
+		ptr[0] = (index * sizeof(uint16_t)) + 1;
+		/* EIR Data type */
+		ptr[1] = (truncated) ? EIR_UUID16_SOME : EIR_UUID16_ALL;
+
 		ptr += 2;
+		eir_len += 2;
 
 		for (i = 0; i < index; i++) {
-			*ptr++ = (uuid[i] & 0x00ff);
-			*ptr++ = (uuid[i] & 0xff00) >> 8;
+			*ptr++ = (uuid16[i] & 0x00ff);
+			*ptr++ = (uuid16[i] & 0xff00) >> 8;
 		}
 	}
 }
-- 
1.7.2

-- 
Inga Stotland
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

Re: [PATCH 3/7] Clean up code that generates extended inquiry response.

[Johan Hedberg]

Hi Inga,

The patches are starting to look pretty good. A couple more coding style
issues though (in this and a few consequtive emails):

On Tue, Aug 03, 2010, Inga Stotland wrote:
> +		ptr[1] = (truncated) ? EIR_UUID16_SOME : EIR_UUID16_ALL;

Another patch in your patch set "fixes" this and removes the parenthesis
around truncated. Could you just fix it in this patch itself. In general
patches with fixes to other patches that aren't yet upstream should be
avoided by redoing the the first patch that caused the need for a fix to
begin with.

Johan

[PATCH 4/7] Support for adding UUID128 to extended inquiry response

[Inga Stotland]

---
 src/sdpd-service.c |   65 +++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 64 insertions(+), 1 deletions(-)

diff --git a/src/sdpd-service.c b/src/sdpd-service.c
index 50c0a97..651e24c 100644
--- a/src/sdpd-service.c
+++ b/src/sdpd-service.c
@@ -49,6 +49,8 @@
 #include "manager.h"
 #include "adapter.h"
 
+#define SIZEOF_UUID128 16
+
 static sdp_record_t *server = NULL;
 
 static uint16_t did_vendor = 0x0000;
@@ -174,6 +176,63 @@ static void update_svclass_list(const bdaddr_t *src)
 
 }
 
+static void eir_generate_uuid128(sdp_list_t *list,
+					uint8_t *ptr, uint16_t *eir_len)
+{
+	int i, k, index = 0;
+	uint16_t len = *eir_len;
+	uint8_t *uuid128;
+	gboolean truncated = FALSE;
+
+	/* Store UUID128 in place, skip 2 bytes to insert type and length later */
+	uuid128 = ptr + 2;
+
+	for (; list; list = list->next) {
+		sdp_record_t *rec = (sdp_record_t *) list->data;
+		uint8_t *uuid128_data = rec->svclass.value.uuid128.data;
+
+		if (rec->svclass.type != SDP_UUID128)
+			continue;
+
+		/* Stop if not enough space to put next UUID128 */
+		if ((len + 2 + SIZEOF_UUID128) > EIR_DATA_LENGTH) {
+			truncated = TRUE;
+			break;
+		}
+
+		/* Check for duplicates, EIR data is Little Endian */
+		for (i = 0; i < index; i++) {
+			for (k = 0; k < SIZEOF_UUID128; k++) {
+				if (uuid128[i * SIZEOF_UUID128 + k] !=
+					uuid128_data[SIZEOF_UUID128 - k])
+					break;
+			}
+			if (k == SIZEOF_UUID128)
+				break;
+		}
+
+		if (i < index)
+			continue;
+
+		/* EIR data is Little Endian */
+		for (k = 0; k < SIZEOF_UUID128; k++)
+			uuid128[index * SIZEOF_UUID128 + k] =
+				uuid128_data[SIZEOF_UUID128 - 1 - k];
+
+		len += SIZEOF_UUID128;
+		index++;
+	}
+
+	if (index > 0 || truncated) {
+		/* EIR Data length */
+		ptr[0] = (index * SIZEOF_UUID128) + 1;
+		/* EIR Data type */
+		ptr[1] = truncated ? EIR_UUID128_SOME : EIR_UUID128_ALL;
+		len += 2;
+		*eir_len = len;
+	}
+}
+
 void create_ext_inquiry_response(const char *name,
 					int8_t tx_power, sdp_list_t *services,
 					uint8_t *data)
@@ -261,7 +320,7 @@ void create_ext_inquiry_response(const char *name,
 		/* EIR Data length */
 		ptr[0] = (index * sizeof(uint16_t)) + 1;
 		/* EIR Data type */
-		ptr[1] = (truncated) ? EIR_UUID16_SOME : EIR_UUID16_ALL;
+		ptr[1] = truncated ? EIR_UUID16_SOME : EIR_UUID16_ALL;
 
 		ptr += 2;
 		eir_len += 2;
@@ -271,6 +330,10 @@ void create_ext_inquiry_response(const char *name,
 			*ptr++ = (uuid16[i] & 0xff00) >> 8;
 		}
 	}
+
+	/* Group all UUID128 types */
+	if (eir_len <= EIR_DATA_LENGTH - 2)
+		eir_generate_uuid128(services, ptr, &eir_len);
 }
 
 void register_public_browse_group(void)
-- 
1.7.2

-- 
Inga Stotland
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

Re: [PATCH 4/7] Support for adding UUID128 to extended inquiry response

[Johan Hedberg]

Hi Inga,

On Tue, Aug 03, 2010, Inga Stotland wrote:
> -		ptr[1] = (truncated) ? EIR_UUID16_SOME : EIR_UUID16_ALL;
> +		ptr[1] = truncated ? EIR_UUID16_SOME : EIR_UUID16_ALL;

And here's the fix I mentioned in the previous email that shouldn't be
needed to begin with.

Johan

[PATCH 5/7] Fix in logic to write EIR when SDP records are changed.

[Inga Stotland]

Whenever SDP service record is added/deleted/modified check for whether
class of device needs to be updated as well. If the update is
needed, proceed as before: new EIR will be written subsequently.
If the class of device is already present, just update EIR and return.
---
 src/adapter.c |   15 +++++++++++----
 1 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/src/adapter.c b/src/adapter.c
index 4615326..e1f3051 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -137,6 +137,8 @@ struct btd_adapter {
 	gint ref;
 };
 
+static void update_ext_inquiry_response(struct btd_adapter *adapter);
+
 static void adapter_set_pairable_timeout(struct btd_adapter *adapter,
 					guint interval);
 
@@ -216,12 +218,17 @@ static int adapter_set_service_classes(struct btd_adapter *adapter,
 	adapter->wanted_cod &= 0x00ffff;
 	adapter->wanted_cod |= (value << 16);
 
-	/* If we already have the CoD we want or the cache is enabled or an
-	 * existing CoD write is in progress just bail out */
-	if (adapter->current_cod == adapter->wanted_cod ||
-			adapter->cache_enable || adapter->pending_cod)
+	/* If the cache is enabled or an existing CoD write is in progress
+	 * just bail out */
+	if (adapter->cache_enable || adapter->pending_cod)
 		return 0;
 
+	/* If we already have the CoD we want, update EIR and return */
+	if (adapter->current_cod == adapter->wanted_cod) {
+		update_ext_inquiry_response(adapter);
+		return 0;
+	}
+
 	DBG("Changing service classes to 0x%06x", adapter->wanted_cod);
 
 	err = adapter_ops->set_class(adapter->dev_id, adapter->wanted_cod);
-- 
1.7.2

-- 
Inga Stotland
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

Re: [PATCH 5/7] Fix in logic to write EIR when SDP records are changed.

[Johan Hedberg]

Hi Inga,

On Tue, Aug 03, 2010, Inga Stotland wrote:
> +static void update_ext_inquiry_response(struct btd_adapter *adapter);

Is this forward declaration really necessary? Would it be possible to
get rid of it by rearranging the static functions in this file (i.e.
move update_ext_inquiry_response higher up in the file)?

Johan

[PATCH 6/7] Handle arrays in device properties dictionary.

[Inga Stotland]

---
 src/adapter.c |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/src/adapter.c b/src/adapter.c
index e1f3051..a2ef9aa 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -2702,6 +2702,7 @@ static void append_dict_valist(DBusMessageIter *iter,
 	DBusMessageIter dict;
 	const char *key;
 	int type;
+	int n_elements;
 	void *val;
 
 	dbus_message_iter_open_container(iter, DBUS_TYPE_ARRAY,
@@ -2713,7 +2714,12 @@ static void append_dict_valist(DBusMessageIter *iter,
 	while (key) {
 		type = va_arg(var_args, int);
 		val = va_arg(var_args, void *);
-		dict_append_entry(&dict, key, type, val);
+		if (type == DBUS_TYPE_ARRAY) {
+			n_elements = va_arg(var_args, int);
+			if (n_elements > 0)
+				dict_append_array(&dict, key, DBUS_TYPE_STRING, val, n_elements);
+		} else
+			dict_append_entry(&dict, key, type, val);
 		key = va_arg(var_args, char *);
 	}
 
-- 
1.7.2

-- 
Inga Stotland
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

[PATCH 7/7] Add service UUIDs from EIR to device properties in "Device Found" signal.

[Inga Stotland]

---
 src/adapter.c |  106 +++++++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 files changed, 103 insertions(+), 3 deletions(-)

diff --git a/src/adapter.c b/src/adapter.c
index a2ef9aa..9603ba7 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -2750,8 +2750,100 @@ static void emit_device_found(const char *path, const char *address,
 	g_dbus_send_message(connection, signal);
 }
 
+static char **get_eir_uuids(uint8_t *eir_data, size_t *uuid_count)
+{
+	uint8_t len = 0;
+	char **uuid_buf = NULL;
+	char **uuids;
+	size_t total = 0;
+	size_t uuid16_count = 0;
+	size_t uuid32_count = 0;
+	size_t uuid128_count = 0;
+	uint8_t *uuid16;
+	uint8_t *uuid32;
+	uint8_t *uuid128;
+	uuid_t service;
+	int i;
+
+	while (len < EIR_DATA_LENGTH) {
+		uint8_t type = eir_data[1];
+		uint8_t field_len = eir_data[0];
+
+		switch(type) {
+		case EIR_UUID16_SOME:
+		case EIR_UUID16_ALL:
+			uuid16_count = field_len / 2;
+			uuid16 = &eir_data[2];
+			break;
+		case EIR_UUID32_SOME:
+		case EIR_UUID32_ALL:
+			uuid32_count = field_len / 4;
+			uuid32 = &eir_data[2];
+			break;
+		case EIR_UUID128_SOME:
+		case EIR_UUID128_ALL:
+			uuid128_count = field_len / 16;
+			uuid128 = &eir_data[2];
+			break;
+		}
+
+		len += field_len + 1;
+		eir_data += field_len + 1;
+	}
+
+	total = uuid16_count + uuid32_count + uuid128_count;
+	*uuid_count = total;
+
+	if (!total)
+		return NULL;
+
+	uuid_buf = g_new0(char *, total + 1);
+
+	if (!uuid_buf)
+		return NULL;
+
+	uuids = uuid_buf;
+
+	/* Generate uuids in SDP format (EIR data is Little Endian) */
+	service.type = SDP_UUID16;
+	for (i = 0; i < uuid16_count; i++) {
+		uint16_t val16 = uuid16[1];
+
+		val16 = (val16<<8) + uuid16[0];
+		service.value.uuid16 = val16;
+		*uuids++ = bt_uuid2string(&service);
+		uuid16 += 2;
+	}
+
+	service.type = SDP_UUID32;
+	for (i = 0; i < uuid32_count; i++) {
+		uint32_t val32 = uuid32[3];
+		int k;
+
+		for (k = 2; k >= 0; k--)
+			val32 = (val32 << 8) + uuid32[k];
+
+		service.value.uuid32 = val32;
+		*uuids++ = bt_uuid2string(&service);
+		uuid32 += 4;
+	}
+
+	service.type = SDP_UUID128;
+	for (i = 0; i < uuid128_count; i++) {
+		int k;
+
+		for (k = 0; k < 16; k++)
+			service.value.uuid128.data[k] = uuid128[16 - k - 1];
+
+		*uuids++ = bt_uuid2string(&service);
+		uuid128 += 16;
+	}
+
+	return uuid_buf;
+}
+
 void adapter_emit_device_found(struct btd_adapter *adapter,
-				struct remote_dev_info *dev)
+				struct remote_dev_info *dev, uint8_t *eir_data)
 {
 	struct btd_device *device;
 	char peer_addr[18], local_addr[18];
@@ -2759,6 +2851,8 @@ void adapter_emit_device_found(struct btd_adapter *adapter,
 	dbus_bool_t paired = FALSE;
 	dbus_int16_t rssi = dev->rssi;
 	char *alias;
+	char **uuids = NULL;
+	size_t uuid_count = 0;
 
 	ba2str(&dev->bdaddr, peer_addr);
 	ba2str(&adapter->bdaddr, local_addr);
@@ -2778,6 +2872,10 @@ void adapter_emit_device_found(struct btd_adapter *adapter,
 	} else
 		alias = g_strdup(dev->alias);
 
+	/* Extract UUIDs from extended inquiry response if any*/
+	if (eir_data != NULL)
+		uuids = get_eir_uuids(eir_data, &uuid_count);
+
 	emit_device_found(adapter->path, paddr,
 			"Address", DBUS_TYPE_STRING, &paddr,
 			"Class", DBUS_TYPE_UINT32, &dev->class,
@@ -2787,15 +2885,17 @@ void adapter_emit_device_found(struct btd_adapter *adapter,
 			"Alias", DBUS_TYPE_STRING, &alias,
 			"LegacyPairing", DBUS_TYPE_BOOLEAN, &dev->legacy,
 			"Paired", DBUS_TYPE_BOOLEAN, &paired,
+			"UUIDs", DBUS_TYPE_ARRAY, &uuids, uuid_count,
 			NULL);
 
 	g_free(alias);
+	g_strfreev(uuids);
 }
 
 void adapter_update_found_devices(struct btd_adapter *adapter, bdaddr_t *bdaddr,
 				int8_t rssi, uint32_t class, const char *name,
 				const char *alias, gboolean legacy,
-				name_status_t name_status)
+				name_status_t name_status, uint8_t *eir_data)
 {
 	struct remote_dev_info *dev, match;
 
@@ -2834,7 +2934,7 @@ done:
 	adapter->found_devices = g_slist_sort(adapter->found_devices,
 						(GCompareFunc) dev_rssi_cmp);
 
-	adapter_emit_device_found(adapter, dev);
+	adapter_emit_device_found(adapter, dev, eir_data);
 }
 
 int adapter_remove_found_device(struct btd_adapter *adapter, bdaddr_t *bdaddr)
-- 
1.7.2

-- 
Inga Stotland
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

[PATCH v7 0/7] Enhanced support for extended inquiry response

[Inga Stotland]

EIR needs to be updated whenever local SDP record database changes.
Previously, when an SDP record was added/removed/updated, EIR would 
be written in callback fired off HCI "Command Complete" event set off by
"Write Class Of Device" command. However, if the class of device did not
need to be updated, the EIR write was not happening either. This
implementation has been augmented to write new EIR when modification
in SDP database does not lead to updating of class of device.

Added support for UUID128 service descriptors in local EIR. 

Service UUIDs are written to "device properties" dictionary  when emitting
"Device Found" signal. This allows peek at available services offered by 
a remote device without establishing SDP connection.


-- 
Inga Stotland
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

[PATCH 7/7] Add service UUIDs from EIR to device properties in "Device Found" signal.

[Inga Stotland]

---
 src/adapter.c  |  106 ++++++++++++++++++++++++++++++++++++++++++++++++++++++--
 src/adapter.h  |    4 +-
 src/dbus-hci.c |    6 ++--
 3 files changed, 108 insertions(+), 8 deletions(-)

diff --git a/src/adapter.c b/src/adapter.c
index c142a4a..9310896 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -2749,8 +2749,100 @@ static void emit_device_found(const char *path, const char *address,
 	g_dbus_send_message(connection, signal);
 }
 
+static char **get_eir_uuids(uint8_t *eir_data, size_t *uuid_count)
+{
+	uint8_t len = 0;
+	char **uuid_buf = NULL;
+	char **uuids;
+	size_t total = 0;
+	size_t uuid16_count = 0;
+	size_t uuid32_count = 0;
+	size_t uuid128_count = 0;
+	uint8_t *uuid16;
+	uint8_t *uuid32;
+	uint8_t *uuid128;
+	uuid_t service;
+	int i;
+
+	while (len < EIR_DATA_LENGTH) {
+		uint8_t type = eir_data[1];
+		uint8_t field_len = eir_data[0];
+
+		switch (type) {
+		case EIR_UUID16_SOME:
+		case EIR_UUID16_ALL:
+			uuid16_count = field_len / 2;
+			uuid16 = &eir_data[2];
+			break;
+		case EIR_UUID32_SOME:
+		case EIR_UUID32_ALL:
+			uuid32_count = field_len / 4;
+			uuid32 = &eir_data[2];
+			break;
+		case EIR_UUID128_SOME:
+		case EIR_UUID128_ALL:
+			uuid128_count = field_len / 16;
+			uuid128 = &eir_data[2];
+			break;
+		}
+
+		len += field_len + 1;
+		eir_data += field_len + 1;
+	}
+
+	total = uuid16_count + uuid32_count + uuid128_count;
+	*uuid_count = total;
+
+	if (!total)
+		return NULL;
+
+	uuid_buf = g_new0(char *, total + 1);
+
+	if (!uuid_buf)
+		return NULL;
+
+	uuids = uuid_buf;
+
+	/* Generate uuids in SDP format (EIR data is Little Endian) */
+	service.type = SDP_UUID16;
+	for (i = 0; i < uuid16_count; i++) {
+		uint16_t val16 = uuid16[1];
+
+		val16 = (val16<<8) + uuid16[0];
+		service.value.uuid16 = val16;
+		*uuids++ = bt_uuid2string(&service);
+		uuid16 += 2;
+	}
+
+	service.type = SDP_UUID32;
+	for (i = 0; i < uuid32_count; i++) {
+		uint32_t val32 = uuid32[3];
+		int k;
+
+		for (k = 2; k >= 0; k--)
+			val32 = (val32 << 8) + uuid32[k];
+
+		service.value.uuid32 = val32;
+		*uuids++ = bt_uuid2string(&service);
+		uuid32 += 4;
+	}
+
+	service.type = SDP_UUID128;
+	for (i = 0; i < uuid128_count; i++) {
+		int k;
+
+		for (k = 0; k < 16; k++)
+			service.value.uuid128.data[k] = uuid128[16 - k - 1];
+
+		*uuids++ = bt_uuid2string(&service);
+		uuid128 += 16;
+	}
+
+	return uuid_buf;
+}
+
 void adapter_emit_device_found(struct btd_adapter *adapter,
-				struct remote_dev_info *dev)
+				struct remote_dev_info *dev, uint8_t *eir_data)
 {
 	struct btd_device *device;
 	char peer_addr[18], local_addr[18];
@@ -2758,6 +2850,8 @@ void adapter_emit_device_found(struct btd_adapter *adapter,
 	dbus_bool_t paired = FALSE;
 	dbus_int16_t rssi = dev->rssi;
 	char *alias;
+	char **uuids = NULL;
+	size_t uuid_count = 0;
 
 	ba2str(&dev->bdaddr, peer_addr);
 	ba2str(&adapter->bdaddr, local_addr);
@@ -2777,6 +2871,10 @@ void adapter_emit_device_found(struct btd_adapter *adapter,
 	} else
 		alias = g_strdup(dev->alias);
 
+	/* Extract UUIDs from extended inquiry response if any*/
+	if (eir_data != NULL)
+		uuids = get_eir_uuids(eir_data, &uuid_count);
+
 	emit_device_found(adapter->path, paddr,
 			"Address", DBUS_TYPE_STRING, &paddr,
 			"Class", DBUS_TYPE_UINT32, &dev->class,
@@ -2786,15 +2884,17 @@ void adapter_emit_device_found(struct btd_adapter *adapter,
 			"Alias", DBUS_TYPE_STRING, &alias,
 			"LegacyPairing", DBUS_TYPE_BOOLEAN, &dev->legacy,
 			"Paired", DBUS_TYPE_BOOLEAN, &paired,
+			"UUIDs", DBUS_TYPE_ARRAY, &uuids, uuid_count,
 			NULL);
 
 	g_free(alias);
+	g_strfreev(uuids);
 }
 
 void adapter_update_found_devices(struct btd_adapter *adapter, bdaddr_t *bdaddr,
 				int8_t rssi, uint32_t class, const char *name,
 				const char *alias, gboolean legacy,
-				name_status_t name_status)
+				name_status_t name_status, uint8_t *eir_data)
 {
 	struct remote_dev_info *dev, match;
 
@@ -2833,7 +2933,7 @@ done:
 	adapter->found_devices = g_slist_sort(adapter->found_devices,
 						(GCompareFunc) dev_rssi_cmp);
 
-	adapter_emit_device_found(adapter, dev);
+	adapter_emit_device_found(adapter, dev, eir_data);
 }
 
 int adapter_remove_found_device(struct btd_adapter *adapter, bdaddr_t *bdaddr)
diff --git a/src/adapter.h b/src/adapter.h
index 8226514..a7eca0e 100644
--- a/src/adapter.h
+++ b/src/adapter.h
@@ -113,10 +113,10 @@ struct remote_dev_info *adapter_search_found_devices(struct btd_adapter *adapter
 void adapter_update_found_devices(struct btd_adapter *adapter, bdaddr_t *bdaddr,
 				int8_t rssi, uint32_t class, const char *name,
 				const char *alias, gboolean legacy,
-				name_status_t name_status);
+				name_status_t name_status, uint8_t *eir_data);
 int adapter_remove_found_device(struct btd_adapter *adapter, bdaddr_t *bdaddr);
 void adapter_emit_device_found(struct btd_adapter *adapter,
-				struct remote_dev_info *dev);
+				struct remote_dev_info *dev, uint8_t *eir_data);
 void adapter_update_oor_devices(struct btd_adapter *adapter);
 void adapter_mode_changed(struct btd_adapter *adapter, uint8_t scan_mode);
 void adapter_setname_complete(bdaddr_t *local, uint8_t status);
diff --git a/src/dbus-hci.c b/src/dbus-hci.c
index b83506f..6d27caa 100644
--- a/src/dbus-hci.c
+++ b/src/dbus-hci.c
@@ -515,7 +515,7 @@ void hcid_dbus_inquiry_result(bdaddr_t *local, bdaddr_t *peer, uint32_t class,
 	if (dev) {
 		adapter_update_found_devices(adapter, peer, rssi, class,
 						NULL, NULL, dev->legacy,
-						NAME_NOT_REQUIRED);
+						NAME_NOT_REQUIRED, data);
 		return;
 	}
 
@@ -566,7 +566,7 @@ void hcid_dbus_inquiry_result(bdaddr_t *local, bdaddr_t *peer, uint32_t class,
 
 	/* add in the list to track name sent/pending */
 	adapter_update_found_devices(adapter, peer, rssi, class, name, alias,
-					legacy, name_status);
+					legacy, name_status, data);
 
 	g_free(name);
 	g_free(alias);
@@ -642,7 +642,7 @@ void hcid_dbus_remote_name(bdaddr_t *local, bdaddr_t *peer, uint8_t status,
 	if (dev_info) {
 		g_free(dev_info->name);
 		dev_info->name = g_strdup(name);
-		adapter_emit_device_found(adapter, dev_info);
+		adapter_emit_device_found(adapter, dev_info, NULL);
 	}
 
 	if (device)
-- 
1.7.2

-- 
Inga Stotland
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

Re: [PATCH 7/7] Add service UUIDs from EIR to device properties in "Device Found" signal.

[Johan Hedberg]

Hi Inga,

On Wed, Aug 04, 2010, Inga Stotland wrote:
> ---
>  src/adapter.c  |  106 ++++++++++++++++++++++++++++++++++++++++++++++++++++++--
>  src/adapter.h  |    4 +-
>  src/dbus-hci.c |    6 ++--
>  3 files changed, 108 insertions(+), 8 deletions(-)

A few more things here:

> +static char **get_eir_uuids(uint8_t *eir_data, size_t *uuid_count)
> +{
> +	uint8_t len = 0;
> +	char **uuid_buf = NULL;
> +	char **uuids;
<snip>
> +	uuid_buf = g_new0(char *, total + 1);
> +
> +	if (!uuid_buf)
> +		return NULL;
> +
> +	uuids = uuid_buf;

First of all, g_new0 is guaranteed to return non-NULL. If something goes
wrong with the memory allocation abort() will be called and the process
will exit. So the NULL check is redundant. Secondly, I don't think you
need both of these variables (see further below).

> +	for (i = 0; i < uuid16_count; i++) {
> +		uint16_t val16 = uuid16[1];
> +
> +		val16 = (val16<<8) + uuid16[0];

Space missing before and after <<

> +		service.value.uuid16 = val16;
> +		*uuids++ = bt_uuid2string(&service);

Instead of incrementing the pointer you could just do

	uuids[i] = bt_uuid2string(&service);

That way the original pointer stays unchanged and you don't need the
second one.

Johan

[PATCH v8 0/7] Enhanced support for extended inquiry response

[Inga Stotland]

EIR needs to be updated whenever local SDP record database changes.
Previously, when an SDP record was added/removed/updated, EIR would 
be written in callback fired off HCI "Command Complete" event set off by
"Write Class Of Device" command. However, if the class of device did not
need to be updated, the EIR write was not happening either. This
implementation has been augmented to write new EIR when modification
in SDP database does not lead to updating of class of device.

Added support for UUID128 service descriptors in local EIR. 

Service UUIDs are written to "device properties" dictionary  when emitting
"Device Found" signal. This allows peek at available services offered by 
a remote device without establishing SDP connection.


-- 
Inga Stotland
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

[PATCH 7/7] Add service UUIDs from EIR to device properties in "Device Found" signal.

[Inga Stotland]

---
 src/adapter.c  |  100 ++++++++++++++++++++++++++++++++++++++++++++++++++++++--
 src/adapter.h  |    4 +-
 src/dbus-hci.c |    6 ++--
 3 files changed, 102 insertions(+), 8 deletions(-)

diff --git a/src/adapter.c b/src/adapter.c
index c142a4a..d191014 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -2749,8 +2749,94 @@ static void emit_device_found(const char *path, const char *address,
 	g_dbus_send_message(connection, signal);
 }
 
+static char **get_eir_uuids(uint8_t *eir_data, size_t *uuid_count)
+{
+	uint8_t len = 0;
+	char **uuids = NULL;
+	size_t total = 0;
+	size_t uuid16_count = 0;
+	size_t uuid32_count = 0;
+	size_t uuid128_count = 0;
+	uint8_t *uuid16;
+	uint8_t *uuid32;
+	uint8_t *uuid128;
+	uuid_t service;
+	int i;
+
+	while (len < EIR_DATA_LENGTH) {
+		uint8_t type = eir_data[1];
+		uint8_t field_len = eir_data[0];
+
+		switch (type) {
+		case EIR_UUID16_SOME:
+		case EIR_UUID16_ALL:
+			uuid16_count = field_len / 2;
+			uuid16 = &eir_data[2];
+			break;
+		case EIR_UUID32_SOME:
+		case EIR_UUID32_ALL:
+			uuid32_count = field_len / 4;
+			uuid32 = &eir_data[2];
+			break;
+		case EIR_UUID128_SOME:
+		case EIR_UUID128_ALL:
+			uuid128_count = field_len / 16;
+			uuid128 = &eir_data[2];
+			break;
+		}
+
+		len += field_len + 1;
+		eir_data += field_len + 1;
+	}
+
+	total = uuid16_count + uuid32_count + uuid128_count;
+	*uuid_count = total;
+
+	if (!total)
+		return NULL;
+
+	uuids = g_new0(char *, total + 1);
+
+	/* Generate uuids in SDP format (EIR data is Little Endian) */
+	service.type = SDP_UUID16;
+	for (i = 0; i < uuid16_count; i++) {
+		uint16_t val16 = uuid16[1];
+
+		val16 = (val16 << 8) + uuid16[0];
+		service.value.uuid16 = val16;
+		uuids[i] = bt_uuid2string(&service);
+		uuid16 += 2;
+	}
+
+	service.type = SDP_UUID32;
+	for (i = uuid16_count; i < uuid32_count + uuid16_count; i++) {
+		uint32_t val32 = uuid32[3];
+		int k;
+
+		for (k = 2; k >= 0; k--)
+			val32 = (val32 << 8) + uuid32[k];
+
+		service.value.uuid32 = val32;
+		uuids[i] = bt_uuid2string(&service);
+		uuid32 += 4;
+	}
+
+	service.type = SDP_UUID128;
+	for (i = uuid32_count + uuid16_count; i < total; i++) {
+		int k;
+
+		for (k = 0; k < 16; k++)
+			service.value.uuid128.data[k] = uuid128[16 - k - 1];
+
+		uuids[i] = bt_uuid2string(&service);
+		uuid128 += 16;
+	}
+
+	return uuids;
+}
+
 void adapter_emit_device_found(struct btd_adapter *adapter,
-				struct remote_dev_info *dev)
+				struct remote_dev_info *dev, uint8_t *eir_data)
 {
 	struct btd_device *device;
 	char peer_addr[18], local_addr[18];
@@ -2758,6 +2844,8 @@ void adapter_emit_device_found(struct btd_adapter *adapter,
 	dbus_bool_t paired = FALSE;
 	dbus_int16_t rssi = dev->rssi;
 	char *alias;
+	char **uuids = NULL;
+	size_t uuid_count = 0;
 
 	ba2str(&dev->bdaddr, peer_addr);
 	ba2str(&adapter->bdaddr, local_addr);
@@ -2777,6 +2865,10 @@ void adapter_emit_device_found(struct btd_adapter *adapter,
 	} else
 		alias = g_strdup(dev->alias);
 
+	/* Extract UUIDs from extended inquiry response if any*/
+	if (eir_data != NULL)
+		uuids = get_eir_uuids(eir_data, &uuid_count);
+
 	emit_device_found(adapter->path, paddr,
 			"Address", DBUS_TYPE_STRING, &paddr,
 			"Class", DBUS_TYPE_UINT32, &dev->class,
@@ -2786,15 +2878,17 @@ void adapter_emit_device_found(struct btd_adapter *adapter,
 			"Alias", DBUS_TYPE_STRING, &alias,
 			"LegacyPairing", DBUS_TYPE_BOOLEAN, &dev->legacy,
 			"Paired", DBUS_TYPE_BOOLEAN, &paired,
+			"UUIDs", DBUS_TYPE_ARRAY, &uuids, uuid_count,
 			NULL);
 
 	g_free(alias);
+	g_strfreev(uuids);
 }
 
 void adapter_update_found_devices(struct btd_adapter *adapter, bdaddr_t *bdaddr,
 				int8_t rssi, uint32_t class, const char *name,
 				const char *alias, gboolean legacy,
-				name_status_t name_status)
+				name_status_t name_status, uint8_t *eir_data)
 {
 	struct remote_dev_info *dev, match;
 
@@ -2833,7 +2927,7 @@ done:
 	adapter->found_devices = g_slist_sort(adapter->found_devices,
 						(GCompareFunc) dev_rssi_cmp);
 
-	adapter_emit_device_found(adapter, dev);
+	adapter_emit_device_found(adapter, dev, eir_data);
 }
 
 int adapter_remove_found_device(struct btd_adapter *adapter, bdaddr_t *bdaddr)
diff --git a/src/adapter.h b/src/adapter.h
index 8226514..a7eca0e 100644
--- a/src/adapter.h
+++ b/src/adapter.h
@@ -113,10 +113,10 @@ struct remote_dev_info *adapter_search_found_devices(struct btd_adapter *adapter
 void adapter_update_found_devices(struct btd_adapter *adapter, bdaddr_t *bdaddr,
 				int8_t rssi, uint32_t class, const char *name,
 				const char *alias, gboolean legacy,
-				name_status_t name_status);
+				name_status_t name_status, uint8_t *eir_data);
 int adapter_remove_found_device(struct btd_adapter *adapter, bdaddr_t *bdaddr);
 void adapter_emit_device_found(struct btd_adapter *adapter,
-				struct remote_dev_info *dev);
+				struct remote_dev_info *dev, uint8_t *eir_data);
 void adapter_update_oor_devices(struct btd_adapter *adapter);
 void adapter_mode_changed(struct btd_adapter *adapter, uint8_t scan_mode);
 void adapter_setname_complete(bdaddr_t *local, uint8_t status);
diff --git a/src/dbus-hci.c b/src/dbus-hci.c
index b83506f..6d27caa 100644
--- a/src/dbus-hci.c
+++ b/src/dbus-hci.c
@@ -515,7 +515,7 @@ void hcid_dbus_inquiry_result(bdaddr_t *local, bdaddr_t *peer, uint32_t class,
 	if (dev) {
 		adapter_update_found_devices(adapter, peer, rssi, class,
 						NULL, NULL, dev->legacy,
-						NAME_NOT_REQUIRED);
+						NAME_NOT_REQUIRED, data);
 		return;
 	}
 
@@ -566,7 +566,7 @@ void hcid_dbus_inquiry_result(bdaddr_t *local, bdaddr_t *peer, uint32_t class,
 
 	/* add in the list to track name sent/pending */
 	adapter_update_found_devices(adapter, peer, rssi, class, name, alias,
-					legacy, name_status);
+					legacy, name_status, data);
 
 	g_free(name);
 	g_free(alias);
@@ -642,7 +642,7 @@ void hcid_dbus_remote_name(bdaddr_t *local, bdaddr_t *peer, uint8_t status,
 	if (dev_info) {
 		g_free(dev_info->name);
 		dev_info->name = g_strdup(name);
-		adapter_emit_device_found(adapter, dev_info);
+		adapter_emit_device_found(adapter, dev_info, NULL);
 	}
 
 	if (device)
-- 
1.7.2

-- 
Inga Stotland
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

Re: [PATCH 7/7] Add service UUIDs from EIR to device properties in "Device Found" signal.

[Johan Hedberg]

Hi Inga,

On Wed, Aug 04, 2010, Inga Stotland wrote:
> ---
>  src/adapter.c  |  100 ++++++++++++++++++++++++++++++++++++++++++++++++++++++--
>  src/adapter.h  |    4 +-
>  src/dbus-hci.c |    6 ++--
>  3 files changed, 102 insertions(+), 8 deletions(-)

I've pushed the six other patches upstream, but I'm still a bit
concerned with this one. First there's a minor coding style issue:

> +static char **get_eir_uuids(uint8_t *eir_data, size_t *uuid_count)
> +{
> +	uint8_t len = 0;
> +	char **uuids = NULL;
> +	size_t total = 0;

Neither the uuids nor the total variable need to be initialized upon
declaration if you look at the function's code flow. In general
initialization upon declaration of variables is something that should be
avoided whenever not strictly needed since it can hide real issues that
the compiler would otherwise be able to catch.

Then, a more general concern about this function. It will receive data
as input that any nearby device that's discoverable has declared in
their EIR data. I.e. we need to be super strict about checking the
validity of the data and not make any assumptions about the correctness
of encoded field lengths etc. in order not to do buffer overflows. Have
you taken this into account when designing the function? Looking at it
it seems it might be possible to give it data that will cause some
buffer overflows (by e.g. placing a uuid list at the very end of the EIR
data with an invalid field length value).

Johan

Re: [PATCH 7/7] Add service UUIDs from EIR to device properties in "Device Found" signal.

[ingas]

Hi Johan,

> I've pushed the six other patches upstream, but I'm still a bit
> concerned with this one.
>

Thanks :)

>
> Then, a more general concern about this function. It will receive data
> as input that any nearby device that's discoverable has declared in
> their EIR data. I.e. we need to be super strict about checking the
> validity of the data and not make any assumptions about the correctness
> of encoded field lengths etc. in order not to do buffer overflows. Have
> you taken this into account when designing the function? Looking at it
> it seems it might be possible to give it data that will cause some
> buffer overflows (by e.g. placing a uuid list at the very end of the EIR
> data with an invalid field length value).
>

I agree. Adding few more checks there. Will send a new patch today.

Inga

[PATCH 7/7] Add service UUIDs from EIR to device properties in "Device Found" signal.

[Inga Stotland]

---
 src/adapter.c  |  108 ++++++++++++++++++++++++++++++++++++++++++++++++++++++--
 src/adapter.h  |    4 +-
 src/dbus-hci.c |    6 ++--
 3 files changed, 110 insertions(+), 8 deletions(-)

diff --git a/src/adapter.c b/src/adapter.c
index c142a4a..b2b5be8 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -2749,8 +2749,102 @@ static void emit_device_found(const char *path, const char *address,
 	g_dbus_send_message(connection, signal);
 }
 
+static char **get_eir_uuids(uint8_t *eir_data, size_t *uuid_count)
+{
+	uint16_t len = 0;
+	char **uuids;
+	size_t total;
+	size_t uuid16_count = 0;
+	size_t uuid32_count = 0;
+	size_t uuid128_count = 0;
+	uint8_t *uuid16;
+	uint8_t *uuid32;
+	uint8_t *uuid128;
+	uuid_t service;
+	int i;
+
+	while (len < EIR_DATA_LENGTH - 1) {
+		uint8_t type = eir_data[1];
+		uint8_t field_len = eir_data[0];
+
+		/* Check for the end of EIR */
+		if (field_len == 0)
+			break;
+
+		switch (type) {
+		case EIR_UUID16_SOME:
+		case EIR_UUID16_ALL:
+			uuid16_count = field_len / 2;
+			uuid16 = &eir_data[2];
+			break;
+		case EIR_UUID32_SOME:
+		case EIR_UUID32_ALL:
+			uuid32_count = field_len / 4;
+			uuid32 = &eir_data[2];
+			break;
+		case EIR_UUID128_SOME:
+		case EIR_UUID128_ALL:
+			uuid128_count = field_len / 16;
+			uuid128 = &eir_data[2];
+			break;
+		}
+
+		len += field_len + 1;
+		eir_data += field_len + 1;
+	}
+
+	/* Bail out if got incorrect length */
+	if (len > EIR_DATA_LENGTH)
+		return NULL;
+
+	total = uuid16_count + uuid32_count + uuid128_count;
+	*uuid_count = total;
+
+	if (!total)
+		return NULL;
+
+	uuids = g_new0(char *, total + 1);
+
+	/* Generate uuids in SDP format (EIR data is Little Endian) */
+	service.type = SDP_UUID16;
+	for (i = 0; i < uuid16_count; i++) {
+		uint16_t val16 = uuid16[1];
+
+		val16 = (val16 << 8) + uuid16[0];
+		service.value.uuid16 = val16;
+		uuids[i] = bt_uuid2string(&service);
+		uuid16 += 2;
+	}
+
+	service.type = SDP_UUID32;
+	for (i = uuid16_count; i < uuid32_count + uuid16_count; i++) {
+		uint32_t val32 = uuid32[3];
+		int k;
+
+		for (k = 2; k >= 0; k--)
+			val32 = (val32 << 8) + uuid32[k];
+
+		service.value.uuid32 = val32;
+		uuids[i] = bt_uuid2string(&service);
+		uuid32 += 4;
+	}
+
+	service.type = SDP_UUID128;
+	for (i = uuid32_count + uuid16_count; i < total; i++) {
+		int k;
+
+		for (k = 0; k < 16; k++)
+			service.value.uuid128.data[k] = uuid128[16 - k - 1];
+
+		uuids[i] = bt_uuid2string(&service);
+		uuid128 += 16;
+	}
+
+	return uuids;
+}
+
 void adapter_emit_device_found(struct btd_adapter *adapter,
-				struct remote_dev_info *dev)
+				struct remote_dev_info *dev, uint8_t *eir_data)
 {
 	struct btd_device *device;
 	char peer_addr[18], local_addr[18];
@@ -2758,6 +2852,8 @@ void adapter_emit_device_found(struct btd_adapter *adapter,
 	dbus_bool_t paired = FALSE;
 	dbus_int16_t rssi = dev->rssi;
 	char *alias;
+	char **uuids = NULL;
+	size_t uuid_count = 0;
 
 	ba2str(&dev->bdaddr, peer_addr);
 	ba2str(&adapter->bdaddr, local_addr);
@@ -2777,6 +2873,10 @@ void adapter_emit_device_found(struct btd_adapter *adapter,
 	} else
 		alias = g_strdup(dev->alias);
 
+	/* Extract UUIDs from extended inquiry response if any*/
+	if (eir_data != NULL)
+		uuids = get_eir_uuids(eir_data, &uuid_count);
+
 	emit_device_found(adapter->path, paddr,
 			"Address", DBUS_TYPE_STRING, &paddr,
 			"Class", DBUS_TYPE_UINT32, &dev->class,
@@ -2786,15 +2886,17 @@ void adapter_emit_device_found(struct btd_adapter *adapter,
 			"Alias", DBUS_TYPE_STRING, &alias,
 			"LegacyPairing", DBUS_TYPE_BOOLEAN, &dev->legacy,
 			"Paired", DBUS_TYPE_BOOLEAN, &paired,
+			"UUIDs", DBUS_TYPE_ARRAY, &uuids, uuid_count,
 			NULL);
 
 	g_free(alias);
+	g_strfreev(uuids);
 }
 
 void adapter_update_found_devices(struct btd_adapter *adapter, bdaddr_t *bdaddr,
 				int8_t rssi, uint32_t class, const char *name,
 				const char *alias, gboolean legacy,
-				name_status_t name_status)
+				name_status_t name_status, uint8_t *eir_data)
 {
 	struct remote_dev_info *dev, match;
 
@@ -2833,7 +2935,7 @@ done:
 	adapter->found_devices = g_slist_sort(adapter->found_devices,
 						(GCompareFunc) dev_rssi_cmp);
 
-	adapter_emit_device_found(adapter, dev);
+	adapter_emit_device_found(adapter, dev, eir_data);
 }
 
 int adapter_remove_found_device(struct btd_adapter *adapter, bdaddr_t *bdaddr)
diff --git a/src/adapter.h b/src/adapter.h
index 8226514..a7eca0e 100644
--- a/src/adapter.h
+++ b/src/adapter.h
@@ -113,10 +113,10 @@ struct remote_dev_info *adapter_search_found_devices(struct btd_adapter *adapter
 void adapter_update_found_devices(struct btd_adapter *adapter, bdaddr_t *bdaddr,
 				int8_t rssi, uint32_t class, const char *name,
 				const char *alias, gboolean legacy,
-				name_status_t name_status);
+				name_status_t name_status, uint8_t *eir_data);
 int adapter_remove_found_device(struct btd_adapter *adapter, bdaddr_t *bdaddr);
 void adapter_emit_device_found(struct btd_adapter *adapter,
-				struct remote_dev_info *dev);
+				struct remote_dev_info *dev, uint8_t *eir_data);
 void adapter_update_oor_devices(struct btd_adapter *adapter);
 void adapter_mode_changed(struct btd_adapter *adapter, uint8_t scan_mode);
 void adapter_setname_complete(bdaddr_t *local, uint8_t status);
diff --git a/src/dbus-hci.c b/src/dbus-hci.c
index b83506f..6d27caa 100644
--- a/src/dbus-hci.c
+++ b/src/dbus-hci.c
@@ -515,7 +515,7 @@ void hcid_dbus_inquiry_result(bdaddr_t *local, bdaddr_t *peer, uint32_t class,
 	if (dev) {
 		adapter_update_found_devices(adapter, peer, rssi, class,
 						NULL, NULL, dev->legacy,
-						NAME_NOT_REQUIRED);
+						NAME_NOT_REQUIRED, data);
 		return;
 	}
 
@@ -566,7 +566,7 @@ void hcid_dbus_inquiry_result(bdaddr_t *local, bdaddr_t *peer, uint32_t class,
 
 	/* add in the list to track name sent/pending */
 	adapter_update_found_devices(adapter, peer, rssi, class, name, alias,
-					legacy, name_status);
+					legacy, name_status, data);
 
 	g_free(name);
 	g_free(alias);
@@ -642,7 +642,7 @@ void hcid_dbus_remote_name(bdaddr_t *local, bdaddr_t *peer, uint8_t status,
 	if (dev_info) {
 		g_free(dev_info->name);
 		dev_info->name = g_strdup(name);
-		adapter_emit_device_found(adapter, dev_info);
+		adapter_emit_device_found(adapter, dev_info, NULL);
 	}
 
 	if (device)
-- 
1.7.2

-- 
Inga Stotland
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

Re: [PATCH 7/7] Add service UUIDs from EIR to device properties in "Device Found" signal.

[Johan Hedberg]

Hi Inga,

On Thu, Aug 05, 2010, Inga Stotland wrote:
> +	while (len < EIR_DATA_LENGTH - 1) {
> +		uint8_t type = eir_data[1];
> +		uint8_t field_len = eir_data[0];
> +
> +		/* Check for the end of EIR */
> +		if (field_len == 0)
> +			break;

Shouldn't there also be another check here:

		/* Bail out if field_len claims to reach beyond the EIR
		 * data end */
		if (len + field_len + 1 >= EIR_DATA_LENGTH)
			break;

Johan

Re: [PATCH 7/7] Add service UUIDs from EIR to device properties in "Device Found" signal.

[Johan Hedberg]

Hi,

On Fri, Aug 06, 2010, Johan Hedberg wrote:
> 		/* Bail out if field_len claims to reach beyond the EIR
> 		 * data end */
> 		if (len + field_len + 1 >= EIR_DATA_LENGTH)
> 			break;

Sorry, that should actually be > and not >= since it's fine if the end
of the field coincides with the end of EIR data.

Johan

Re: [PATCH 7/7] Add service UUIDs from EIR to device properties in "Device Found" signal.

[ingas]

Hi Johan,

> Hi Inga,
>
> On Thu, Aug 05, 2010, Inga Stotland wrote:
>> +	while (len < EIR_DATA_LENGTH - 1) {
>> +		uint8_t type = eir_data[1];
>> +		uint8_t field_len = eir_data[0];
>> +
>> +		/* Check for the end of EIR */
>> +		if (field_len == 0)
>> +			break;
>
> Shouldn't there also be another check here:
>
> 		/* Bail out if field_len claims to reach beyond the EIR
> 		 * data end */
> 		if (len + field_len + 1 >= EIR_DATA_LENGTH)
> 			break;
>

After reading in eir_data[0] & eir_data[1] (and those reads are valid due
to passing the "while (len < EIR_DATA_LENGTH - 1)" check above) there are
no more memory accesses in the loop. And if we do end up reading in field
length that's bogus, we fail the "while" check on next iteration, exit the
loop, fail the "(len > EIR_DATA_LENGTH)" and bail out of the routine with
NULL return value.

Actually, I originally had the check for field length inside the loop, but
then moved it outside for efficiency reasons.

Thanks,

Inga


--
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.


>

Re: [PATCH 7/7] Add service UUIDs from EIR to device properties in "Device Found" signal.

[Johan Hedberg]

Hi Inga,

On Fri, Aug 06, 2010, ingas@codeaurora.org wrote:
> > On Thu, Aug 05, 2010, Inga Stotland wrote:
> >> +	while (len < EIR_DATA_LENGTH - 1) {
> >> +		uint8_t type = eir_data[1];
> >> +		uint8_t field_len = eir_data[0];
> >> +
> >> +		/* Check for the end of EIR */
> >> +		if (field_len == 0)
> >> +			break;
> >
> > Shouldn't there also be another check here:
> >
> > 		/* Bail out if field_len claims to reach beyond the EIR
> > 		 * data end */
> > 		if (len + field_len + 1 >= EIR_DATA_LENGTH)
> > 			break;
> >
> 
> After reading in eir_data[0] & eir_data[1] (and those reads are valid due
> to passing the "while (len < EIR_DATA_LENGTH - 1)" check above) there are
> no more memory accesses in the loop. And if we do end up reading in field
> length that's bogus, we fail the "while" check on next iteration, exit the
> loop, fail the "(len > EIR_DATA_LENGTH)" and bail out of the routine with
> NULL return value.

Yep, you're right. What got me unnerved was that you still set pointers
to potentially out-of-bounds data in the switch statement, but as you
say the if check after the switch statement ensures that the pointers
don't get accessed if something went beyond the EIR data length.

There's still however one issue (I only now tried to compile the patch):

src/adapter.c: In function ‘get_eir_uuids’:
src/adapter.c:2810: error: comparison between signed and unsigned integer expressions
src/adapter.c:2820: error: comparison between signed and unsigned integer expressions
src/adapter.c:2833: error: comparison between signed and unsigned integer expressions
make[1]: *** [src/adapter.o] Error 1

Could you please fix it and always in the future ensure that the code
compiles cleanly when configured with ./bootstrap-configure. Also, could
you make the commit message more descriptive. The summary line should be
a very short summary of what the patch is about and the more detailed
description should be in the message body.

Johan

Re: [PATCH 7/7] Add service UUIDs from EIR to device properties in "Device Found" signal.

[ingas]

>
> There's still however one issue (I only now tried to compile the patch):
>
> src/adapter.c: In function ‘get_eir_uuids’:
> src/adapter.c:2810: error: comparison between signed and unsigned integer
> expressions
> src/adapter.c:2820: error: comparison between signed and unsigned integer
> expressions
> src/adapter.c:2833: error: comparison between signed and unsigned integer
> expressions
> make[1]: *** [src/adapter.o] Error 1
>
> Could you please fix it and always in the future ensure that the code
> compiles cleanly when configured with ./bootstrap-configure. Also, could
> you make the commit message more descriptive. The summary line should be
> a very short summary of what the patch is about and the more detailed
> description should be in the message body.
>

Oops, sorry about that: I was using bootstrap and configure as separate
scripts. Apparently the warning level is not set high enough in there.
Will fix and resubmit.

Regards,

Inga

[PATCH 7/7] Add service UUIDs from EIR to device properties in "Device Found" signal.

[Inga Stotland]

If service UUIDs are present in EIR, they are written to
"device properties" dictionary  when emitting "Device Found" signal.
This allows peek at available services offered by a remote device
without establishing SDP connection.
---
 src/adapter.c  |  108 ++++++++++++++++++++++++++++++++++++++++++++++++++++++--
 src/adapter.h  |    4 +-
 src/dbus-hci.c |    6 ++--
 3 files changed, 110 insertions(+), 8 deletions(-)

diff --git a/src/adapter.c b/src/adapter.c
index c142a4a..5746cbc 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -2749,8 +2749,102 @@ static void emit_device_found(const char *path, const char *address,
 	g_dbus_send_message(connection, signal);
 }
 
+static char **get_eir_uuids(uint8_t *eir_data, size_t *uuid_count)
+{
+	uint16_t len = 0;
+	char **uuids;
+	size_t total;
+	size_t uuid16_count = 0;
+	size_t uuid32_count = 0;
+	size_t uuid128_count = 0;
+	uint8_t *uuid16;
+	uint8_t *uuid32;
+	uint8_t *uuid128;
+	uuid_t service;
+	unsigned int i;
+
+	while (len < EIR_DATA_LENGTH - 1) {
+		uint8_t type = eir_data[1];
+		uint8_t field_len = eir_data[0];
+
+		/* Check for the end of EIR */
+		if (field_len == 0)
+			break;
+
+		switch (type) {
+		case EIR_UUID16_SOME:
+		case EIR_UUID16_ALL:
+			uuid16_count = field_len / 2;
+			uuid16 = &eir_data[2];
+			break;
+		case EIR_UUID32_SOME:
+		case EIR_UUID32_ALL:
+			uuid32_count = field_len / 4;
+			uuid32 = &eir_data[2];
+			break;
+		case EIR_UUID128_SOME:
+		case EIR_UUID128_ALL:
+			uuid128_count = field_len / 16;
+			uuid128 = &eir_data[2];
+			break;
+		}
+
+		len += field_len + 1;
+		eir_data += field_len + 1;
+	}
+
+	/* Bail out if got incorrect length */
+	if (len > EIR_DATA_LENGTH)
+		return NULL;
+
+	total = uuid16_count + uuid32_count + uuid128_count;
+	*uuid_count = total;
+
+	if (!total)
+		return NULL;
+
+	uuids = g_new0(char *, total + 1);
+
+	/* Generate uuids in SDP format (EIR data is Little Endian) */
+	service.type = SDP_UUID16;
+	for (i = 0; i < uuid16_count; i++) {
+		uint16_t val16 = uuid16[1];
+
+		val16 = (val16 << 8) + uuid16[0];
+		service.value.uuid16 = val16;
+		uuids[i] = bt_uuid2string(&service);
+		uuid16 += 2;
+	}
+
+	service.type = SDP_UUID32;
+	for (i = uuid16_count; i < uuid32_count + uuid16_count; i++) {
+		uint32_t val32 = uuid32[3];
+		int k;
+
+		for (k = 2; k >= 0; k--)
+			val32 = (val32 << 8) + uuid32[k];
+
+		service.value.uuid32 = val32;
+		uuids[i] = bt_uuid2string(&service);
+		uuid32 += 4;
+	}
+
+	service.type = SDP_UUID128;
+	for (i = uuid32_count + uuid16_count; i < total; i++) {
+		int k;
+
+		for (k = 0; k < 16; k++)
+			service.value.uuid128.data[k] = uuid128[16 - k - 1];
+
+		uuids[i] = bt_uuid2string(&service);
+		uuid128 += 16;
+	}
+
+	return uuids;
+}
+
 void adapter_emit_device_found(struct btd_adapter *adapter,
-				struct remote_dev_info *dev)
+				struct remote_dev_info *dev, uint8_t *eir_data)
 {
 	struct btd_device *device;
 	char peer_addr[18], local_addr[18];
@@ -2758,6 +2852,8 @@ void adapter_emit_device_found(struct btd_adapter *adapter,
 	dbus_bool_t paired = FALSE;
 	dbus_int16_t rssi = dev->rssi;
 	char *alias;
+	char **uuids = NULL;
+	size_t uuid_count = 0;
 
 	ba2str(&dev->bdaddr, peer_addr);
 	ba2str(&adapter->bdaddr, local_addr);
@@ -2777,6 +2873,10 @@ void adapter_emit_device_found(struct btd_adapter *adapter,
 	} else
 		alias = g_strdup(dev->alias);
 
+	/* Extract UUIDs from extended inquiry response if any*/
+	if (eir_data != NULL)
+		uuids = get_eir_uuids(eir_data, &uuid_count);
+
 	emit_device_found(adapter->path, paddr,
 			"Address", DBUS_TYPE_STRING, &paddr,
 			"Class", DBUS_TYPE_UINT32, &dev->class,
@@ -2786,15 +2886,17 @@ void adapter_emit_device_found(struct btd_adapter *adapter,
 			"Alias", DBUS_TYPE_STRING, &alias,
 			"LegacyPairing", DBUS_TYPE_BOOLEAN, &dev->legacy,
 			"Paired", DBUS_TYPE_BOOLEAN, &paired,
+			"UUIDs", DBUS_TYPE_ARRAY, &uuids, uuid_count,
 			NULL);
 
 	g_free(alias);
+	g_strfreev(uuids);
 }
 
 void adapter_update_found_devices(struct btd_adapter *adapter, bdaddr_t *bdaddr,
 				int8_t rssi, uint32_t class, const char *name,
 				const char *alias, gboolean legacy,
-				name_status_t name_status)
+				name_status_t name_status, uint8_t *eir_data)
 {
 	struct remote_dev_info *dev, match;
 
@@ -2833,7 +2935,7 @@ done:
 	adapter->found_devices = g_slist_sort(adapter->found_devices,
 						(GCompareFunc) dev_rssi_cmp);
 
-	adapter_emit_device_found(adapter, dev);
+	adapter_emit_device_found(adapter, dev, eir_data);
 }
 
 int adapter_remove_found_device(struct btd_adapter *adapter, bdaddr_t *bdaddr)
diff --git a/src/adapter.h b/src/adapter.h
index 8226514..a7eca0e 100644
--- a/src/adapter.h
+++ b/src/adapter.h
@@ -113,10 +113,10 @@ struct remote_dev_info *adapter_search_found_devices(struct btd_adapter *adapter
 void adapter_update_found_devices(struct btd_adapter *adapter, bdaddr_t *bdaddr,
 				int8_t rssi, uint32_t class, const char *name,
 				const char *alias, gboolean legacy,
-				name_status_t name_status);
+				name_status_t name_status, uint8_t *eir_data);
 int adapter_remove_found_device(struct btd_adapter *adapter, bdaddr_t *bdaddr);
 void adapter_emit_device_found(struct btd_adapter *adapter,
-				struct remote_dev_info *dev);
+				struct remote_dev_info *dev, uint8_t *eir_data);
 void adapter_update_oor_devices(struct btd_adapter *adapter);
 void adapter_mode_changed(struct btd_adapter *adapter, uint8_t scan_mode);
 void adapter_setname_complete(bdaddr_t *local, uint8_t status);
diff --git a/src/dbus-hci.c b/src/dbus-hci.c
index b83506f..6d27caa 100644
--- a/src/dbus-hci.c
+++ b/src/dbus-hci.c
@@ -515,7 +515,7 @@ void hcid_dbus_inquiry_result(bdaddr_t *local, bdaddr_t *peer, uint32_t class,
 	if (dev) {
 		adapter_update_found_devices(adapter, peer, rssi, class,
 						NULL, NULL, dev->legacy,
-						NAME_NOT_REQUIRED);
+						NAME_NOT_REQUIRED, data);
 		return;
 	}
 
@@ -566,7 +566,7 @@ void hcid_dbus_inquiry_result(bdaddr_t *local, bdaddr_t *peer, uint32_t class,
 
 	/* add in the list to track name sent/pending */
 	adapter_update_found_devices(adapter, peer, rssi, class, name, alias,
-					legacy, name_status);
+					legacy, name_status, data);
 
 	g_free(name);
 	g_free(alias);
@@ -642,7 +642,7 @@ void hcid_dbus_remote_name(bdaddr_t *local, bdaddr_t *peer, uint8_t status,
 	if (dev_info) {
 		g_free(dev_info->name);
 		dev_info->name = g_strdup(name);
-		adapter_emit_device_found(adapter, dev_info);
+		adapter_emit_device_found(adapter, dev_info, NULL);
 	}
 
 	if (device)
-- 
1.7.2

-- 
Inga Stotland
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

Re: [PATCH 7/7] Add service UUIDs from EIR to device properties in "Device Found" signal.

[Johan Hedberg]

Hi Inga,

On Fri, Aug 06, 2010, Inga Stotland wrote:
> If service UUIDs are present in EIR, they are written to
> "device properties" dictionary  when emitting "Device Found" signal.
> This allows peek at available services offered by a remote device
> without establishing SDP connection.
> ---
>  src/adapter.c  |  108 ++++++++++++++++++++++++++++++++++++++++++++++++++++++--
>  src/adapter.h  |    4 +-
>  src/dbus-hci.c |    6 ++--
>  3 files changed, 110 insertions(+), 8 deletions(-)

The patch is now upstream. Thanks!

Johan