Skip to contentSkip to conversation

Update gatt_service_add size assert for dealing with 128-bit uuids

PATCH In review Dirk-Jan C. Binnema 2 participants 4 messages Jul 29, 2013 – Jul 30, 2013 Bluetooth drivers

lore
Share & tools
lore

Dirk-Jan C. Binnema

author
#0 lore raw

PATCH 1/3 Update gatt_service_add size assert for dealing with 128-bit uuids

From: "Dirk-Jan C. Binnema" <[email protected]>

To paraphrase Anderson Lizardo: "h" was overflowing for 128-bit services because the handle is incremented after each attribute is added so at the end we had: 0xffff + 1 == 0 (h is uint16_t).

As it seems more complex to try to remove the overflow without affecting the logic too much, we update the assertion.

---
 attrib/gatt-service.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
attrib/gatt-service.c +1 -1
index 4b02d39..1b84c89 100644
@@ -341,7 +341,7 @@ gboolean gatt_service_add(struct btd_adapter *adapter, uint16_t uuid,
 	}
 
 	g_assert(size < USHRT_MAX);
-	g_assert(h - start_handle == (uint16_t) size);
+	g_assert(h == 0 || (h - start_handle == (uint16_t) size));
 	g_slist_free_full(chrs, free_gatt_info);
 
 	return TRUE;
Signature · 1 line
1.8.1.4
author
#1 lore raw

PATCH 2/3 Rename GATT_OPT_CHR_UUID into GATT_OPT_CHR_UUID16

From: "Dirk-Jan C. Binnema" <[email protected]>

Rename the option for all in-tree users. This frees up GATT_OPT_CHR_UUID to be used for UUIDs of type bt_uuid_t* (in a next patch).

---
 attrib/gatt-service.c         |  4 ++--
 attrib/gatt-service.h         |  2 +-
 plugins/gatt-example.c        |  2 +-
 profiles/alert/server.c       | 16 ++++++++--------
 profiles/proximity/immalert.c |  2 +-
 profiles/proximity/linkloss.c |  2 +-
 profiles/time/server.c        |  8 ++++----
 7 files changed, 18 insertions(+), 18 deletions(-)
attrib/gatt-service.c +2 -2
index 1b84c89..6ae3902 100644
@@ -68,7 +68,7 @@ static GSList *parse_opts(gatt_option opt1, va_list args)
 
 	while (opt != GATT_OPT_INVALID) {
 		switch (opt) {
-		case GATT_OPT_CHR_UUID:
+		case GATT_OPT_CHR_UUID16:
 			bt_uuid16_create(&info->uuid, va_arg(args, int));
 			/* characteristic declaration and value */
 			info->num_attrs += 2;

@@ -108,7 +108,7 @@ static GSList *parse_opts(gatt_option opt1, va_list args)
 		}
 
 		opt = va_arg(args, gatt_option);
-		if (opt == GATT_OPT_CHR_UUID) {
+		if (opt == GATT_OPT_CHR_UUID16) {
 			info = g_new0(struct gatt_info, 1);
 			l = g_slist_append(l, info);
 		}
attrib/gatt-service.h +1 -1
index b810e2e..3964c2b 100644
@@ -24,7 +24,7 @@
 
 typedef enum {
 	GATT_OPT_INVALID = 0,
-	GATT_OPT_CHR_UUID,
+	GATT_OPT_CHR_UUID16,
 	GATT_OPT_CHR_PROPS,
 	GATT_OPT_CHR_VALUE_CB,
 	GATT_OPT_CHR_AUTHENTICATION,
plugins/gatt-example.c +1 -1
index 37972e8..b926947 100644
@@ -114,7 +114,7 @@ static gboolean register_battery_service(struct btd_adapter *adapter)
 
 	return gatt_service_add(adapter, GATT_PRIM_SVC_UUID, &uuid,
 			/* battery state characteristic */
-			GATT_OPT_CHR_UUID, BATTERY_STATE_UUID,
+			GATT_OPT_CHR_UUID16, BATTERY_STATE_UUID,
 			GATT_OPT_CHR_PROPS, ATT_CHAR_PROPER_READ |
 							ATT_CHAR_PROPER_NOTIFY,
 			GATT_OPT_CHR_VALUE_CB, ATTRIB_READ,
profiles/alert/server.c +8 -8
index 8da1928..4565470 100644
@@ -789,7 +789,7 @@ static void register_phone_alert_service(struct alert_adapter *al_adapter)
 	/* Phone Alert Status Service */
 	gatt_service_add(al_adapter->adapter, GATT_PRIM_SVC_UUID, &uuid,
 			/* Alert Status characteristic */
-			GATT_OPT_CHR_UUID, ALERT_STATUS_CHR_UUID,
+			GATT_OPT_CHR_UUID16, ALERT_STATUS_CHR_UUID,
 			GATT_OPT_CHR_PROPS, ATT_CHAR_PROPER_READ |
 							ATT_CHAR_PROPER_NOTIFY,
 			GATT_OPT_CHR_VALUE_CB, ATTRIB_READ,

@@ -799,12 +799,12 @@ static void register_phone_alert_service(struct alert_adapter *al_adapter)
 			GATT_OPT_CHR_VALUE_GET_HANDLE,
 			&al_adapter->hnd_value[NOTIFY_ALERT_STATUS],
 			/* Ringer Control Point characteristic */
-			GATT_OPT_CHR_UUID, RINGER_CP_CHR_UUID,
+			GATT_OPT_CHR_UUID16, RINGER_CP_CHR_UUID,
 			GATT_OPT_CHR_PROPS, ATT_CHAR_PROPER_WRITE_WITHOUT_RESP,
 			GATT_OPT_CHR_VALUE_CB, ATTRIB_WRITE,
 			ringer_cp_write, NULL,
 			/* Ringer Setting characteristic */
-			GATT_OPT_CHR_UUID, RINGER_SETTING_CHR_UUID,
+			GATT_OPT_CHR_UUID16, RINGER_SETTING_CHR_UUID,
 			GATT_OPT_CHR_PROPS, ATT_CHAR_PROPER_READ |
 							ATT_CHAR_PROPER_NOTIFY,
 			GATT_OPT_CHR_VALUE_CB, ATTRIB_READ,

@@ -892,35 +892,35 @@ static void register_alert_notif_service(struct alert_adapter *al_adapter)
 	/* Alert Notification Service */
 	gatt_service_add(al_adapter->adapter, GATT_PRIM_SVC_UUID, &uuid,
 			/* Supported New Alert Category */
-			GATT_OPT_CHR_UUID, SUPP_NEW_ALERT_CAT_CHR_UUID,
+			GATT_OPT_CHR_UUID16, SUPP_NEW_ALERT_CAT_CHR_UUID,
 			GATT_OPT_CHR_PROPS, ATT_CHAR_PROPER_READ,
 			GATT_OPT_CHR_VALUE_CB, ATTRIB_READ,
 			supp_new_alert_cat_read, al_adapter->adapter,
 			GATT_OPT_CHR_VALUE_GET_HANDLE,
 			&al_adapter->supp_new_alert_cat_handle,
 			/* New Alert */
-			GATT_OPT_CHR_UUID, NEW_ALERT_CHR_UUID,
+			GATT_OPT_CHR_UUID16, NEW_ALERT_CHR_UUID,
 			GATT_OPT_CHR_PROPS, ATT_CHAR_PROPER_NOTIFY,
 			GATT_OPT_CCC_GET_HANDLE,
 			&al_adapter->hnd_ccc[NOTIFY_NEW_ALERT],
 			GATT_OPT_CHR_VALUE_GET_HANDLE,
 			&al_adapter->hnd_value[NOTIFY_NEW_ALERT],
 			/* Supported Unread Alert Category */
-			GATT_OPT_CHR_UUID, SUPP_UNREAD_ALERT_CAT_CHR_UUID,
+			GATT_OPT_CHR_UUID16, SUPP_UNREAD_ALERT_CAT_CHR_UUID,
 			GATT_OPT_CHR_PROPS, ATT_CHAR_PROPER_READ,
 			GATT_OPT_CHR_VALUE_CB, ATTRIB_READ,
 			supp_unread_alert_cat_read, al_adapter->adapter,
 			GATT_OPT_CHR_VALUE_GET_HANDLE,
 			&al_adapter->supp_unread_alert_cat_handle,
 			/* Unread Alert Status */
-			GATT_OPT_CHR_UUID, UNREAD_ALERT_CHR_UUID,
+			GATT_OPT_CHR_UUID16, UNREAD_ALERT_CHR_UUID,
 			GATT_OPT_CHR_PROPS, ATT_CHAR_PROPER_NOTIFY,
 			GATT_OPT_CCC_GET_HANDLE,
 			&al_adapter->hnd_ccc[NOTIFY_UNREAD_ALERT],
 			GATT_OPT_CHR_VALUE_GET_HANDLE,
 			&al_adapter->hnd_value[NOTIFY_UNREAD_ALERT],
 			/* Alert Notification Control Point */
-			GATT_OPT_CHR_UUID, ALERT_NOTIF_CP_CHR_UUID,
+			GATT_OPT_CHR_UUID16, ALERT_NOTIF_CP_CHR_UUID,
 			GATT_OPT_CHR_PROPS, ATT_CHAR_PROPER_WRITE,
 			GATT_OPT_CHR_VALUE_CB, ATTRIB_WRITE,
 			alert_notif_cp_write, NULL,
profiles/proximity/immalert.c +1 -1
index 06e8eb8..2f8d4e7 100644
@@ -249,7 +249,7 @@ void imm_alert_register(struct btd_adapter *adapter)
 	svc_added = gatt_service_add(adapter,
 				GATT_PRIM_SVC_UUID, &uuid,
 				/* Alert level characteristic */
-				GATT_OPT_CHR_UUID, ALERT_LEVEL_CHR_UUID,
+				GATT_OPT_CHR_UUID16, ALERT_LEVEL_CHR_UUID,
 				GATT_OPT_CHR_PROPS,
 					ATT_CHAR_PROPER_WRITE_WITHOUT_RESP,
 				GATT_OPT_CHR_VALUE_CB, ATTRIB_WRITE,
profiles/proximity/linkloss.c +1 -1
index cb87b12..a7ed96c 100644
@@ -290,7 +290,7 @@ void link_loss_register(struct btd_adapter *adapter)
 	svc_added = gatt_service_add(adapter,
 			GATT_PRIM_SVC_UUID, &uuid,
 			/* Alert level characteristic */
-			GATT_OPT_CHR_UUID, ALERT_LEVEL_CHR_UUID,
+			GATT_OPT_CHR_UUID16, ALERT_LEVEL_CHR_UUID,
 			GATT_OPT_CHR_PROPS,
 				ATT_CHAR_PROPER_READ | ATT_CHAR_PROPER_WRITE,
 			GATT_OPT_CHR_VALUE_CB, ATTRIB_READ,
profiles/time/server.c +4 -4
index 518a29f..178d4d2 100644
@@ -152,14 +152,14 @@ static gboolean register_current_time_service(struct btd_adapter *adapter)
 	/* Current Time service */
 	return gatt_service_add(adapter, GATT_PRIM_SVC_UUID, &uuid,
 				/* CT Time characteristic */
-				GATT_OPT_CHR_UUID, CT_TIME_CHR_UUID,
+				GATT_OPT_CHR_UUID16, CT_TIME_CHR_UUID,
 				GATT_OPT_CHR_PROPS, ATT_CHAR_PROPER_READ |
 							ATT_CHAR_PROPER_NOTIFY,
 				GATT_OPT_CHR_VALUE_CB, ATTRIB_READ,
 						current_time_read, adapter,
 
 				/* Local Time Information characteristic */
-				GATT_OPT_CHR_UUID, LOCAL_TIME_INFO_CHR_UUID,
+				GATT_OPT_CHR_UUID16, LOCAL_TIME_INFO_CHR_UUID,
 				GATT_OPT_CHR_PROPS, ATT_CHAR_PROPER_READ,
 				GATT_OPT_CHR_VALUE_CB, ATTRIB_READ,
 						local_time_info_read, adapter,

@@ -215,14 +215,14 @@ static gboolean register_ref_time_update_service(struct btd_adapter *adapter)
 	/* Reference Time Update service */
 	return gatt_service_add(adapter, GATT_PRIM_SVC_UUID, &uuid,
 				/* Time Update control point */
-				GATT_OPT_CHR_UUID, TIME_UPDATE_CTRL_CHR_UUID,
+				GATT_OPT_CHR_UUID16, TIME_UPDATE_CTRL_CHR_UUID,
 				GATT_OPT_CHR_PROPS,
 					ATT_CHAR_PROPER_WRITE_WITHOUT_RESP,
 				GATT_OPT_CHR_VALUE_CB, ATTRIB_WRITE,
 						time_update_control, adapter,
 
 				/* Time Update status */
-				GATT_OPT_CHR_UUID, TIME_UPDATE_STAT_CHR_UUID,
+				GATT_OPT_CHR_UUID16, TIME_UPDATE_STAT_CHR_UUID,
 				GATT_OPT_CHR_PROPS, ATT_CHAR_PROPER_READ,
 				GATT_OPT_CHR_VALUE_CB, ATTRIB_READ,
 						time_update_status, adapter,
Signature · 1 line
1.8.1.4
author
#2 lore raw

PATCH 3/3 Add support for 128-bit characteristic uuids in gatt_service_add

In reply to Dirk-Jan C. Binnema

From: "Dirk-Jan C. Binnema" <[email protected]>

Custom GATT services typically use 128-bits UUIDs; this patch allows for using gatt_service_add to define such services by adding a new option GATT_OPT_CHR_UUID, which allows for passing bt_uuid_t* values. Note, a previous patch renamed the old GATT_OPT_CHR_UUID into GATT_OPT_CHR_UUID16.

---
 attrib/gatt-service.c | 17 ++++++++++++-----
 attrib/gatt-service.h |  6 ++++++
 2 files changed, 18 insertions(+), 5 deletions(-)
attrib/gatt-service.c +12 -5
index 6ae3902..4cd300f 100644
@@ -73,6 +73,12 @@ static GSList *parse_opts(gatt_option opt1, va_list args)
 			/* characteristic declaration and value */
 			info->num_attrs += 2;
 			break;
+		case GATT_OPT_CHR_UUID:
+			memcpy(&info->uuid, va_arg(args, bt_uuid_t *),
+				sizeof(bt_uuid_t));
+			/* characteristic declaration and value */
+			info->num_attrs += 2;
+			break;
 		case GATT_OPT_CHR_PROPS:
 			info->props = va_arg(args, int);
 

@@ -108,7 +114,7 @@ static GSList *parse_opts(gatt_option opt1, va_list args)
 		}
 
 		opt = va_arg(args, gatt_option);
-		if (opt == GATT_OPT_CHR_UUID16) {
+		if (opt == GATT_OPT_CHR_UUID16 || opt == GATT_OPT_CHR_UUID) {
 			info = g_new0(struct gatt_info, 1);
 			l = g_slist_append(l, info);
 		}

@@ -183,10 +189,11 @@ static gboolean add_characteristic(struct btd_adapter *adapter,
 	uint16_t h = *handle;
 	struct attribute *a;
 	bt_uuid_t bt_uuid;
-	uint8_t atval[5];
+	uint8_t atval[ATT_MAX_VALUE_LEN];
 	GSList *l;
 
-	if (!info->uuid.value.u16 || !info->props) {
+	if ((info->uuid.type != BT_UUID16 && info->uuid.type != BT_UUID128) ||
+		!info->props) {
 		error("Characteristic UUID or properties are missing");
 		return FALSE;
 	}

@@ -222,9 +229,9 @@ static gboolean add_characteristic(struct btd_adapter *adapter,
 	bt_uuid16_create(&bt_uuid, GATT_CHARAC_UUID);
 	atval[0] = info->props;
 	att_put_u16(h + 1, &atval[1]);
-	att_put_u16(info->uuid.value.u16, &atval[3]);
+	att_put_uuid(info->uuid, &atval[3]);
 	if (attrib_db_add(adapter, h++, &bt_uuid, ATT_NONE, ATT_NOT_PERMITTED,
-						atval, sizeof(atval)) == NULL)
+				atval, 3 + info->uuid.type / 8) == NULL)
 		return FALSE;
 
 	/* characteristic value */
attrib/gatt-service.h +6 -0
index 3964c2b..728d3a8 100644
@@ -24,7 +24,13 @@
 
 typedef enum {
 	GATT_OPT_INVALID = 0,
+
+	/* bt_uuid_t* value */
+	GATT_OPT_CHR_UUID,
+
+	/* a uint16 value */
 	GATT_OPT_CHR_UUID16,
+
 	GATT_OPT_CHR_PROPS,
 	GATT_OPT_CHR_VALUE_CB,
 	GATT_OPT_CHR_AUTHENTICATION,
Signature · 1 line
1.8.1.4
#3 lore raw
In reply to Dirk-Jan C. Binnema

Hi Dirk,

Quoted 9 lines · Dirk-Jan C. Binnema

To paraphrase Anderson Lizardo: "h" was overflowing for 128-bit services because the handle is incremented after each attribute is added so at the end we had: 0xffff + 1 == 0 (h is uint16_t).

As it seems more complex to try to remove the overflow without affecting the logic too much, we update the assertion.

---
 attrib/gatt-service.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

I made a couple of more cosmetic cleanups and after that pushed these patches upstream. Thanks.

Some things to keep in mind for future patches:

- Keep your commit message line lengths max 72-74 (both git log and git shortlog indent them and we don't want to run past a 80 char terminal)
- Prefix the summary lines appropriately (in this case "attrib: ...")
- Split lines are indented as much as possible while keeping them under 80 characters (i.e. not just past the opening parenthesis)

Johan