[PATCH] attrib/gatt_service_add: support 128-bit uuids for characteristics

[PATCH] attrib/gatt_service_add: support 128-bit uuids for characteristics

[Dirk-Jan C. Binnema]

From: "Dirk-Jan C. Binnema" <djcb.bulk@gmail.com>

Add GATT_OPT_CHR_BT_UUID_T, which does what GATT_OPT_CHR_UUID does, but
instead takes a bt_uuid_t*, so 128-bit uuids are supported as well.

GATT_OPT_CHR_UUID is kept around for backward compatibility.

Signed-off-by: Dirk-Jan C. Binnema <djcb@djcbsoftware.nl>
---
 attrib/gatt-service.c | 19 ++++++++++++-------
 attrib/gatt-service.h |  1 +
 2 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/attrib/gatt-service.c b/attrib/gatt-service.c
index 4b02d39..bc17778 100644
--- a/attrib/gatt-service.c
+++ b/attrib/gatt-service.c
@@ -73,6 +73,11 @@ static GSList *parse_opts(gatt_option opt1, va_list args)
 			/* characteristic declaration and value */
 			info->num_attrs += 2;
 			break;
+		case GATT_OPT_CHR_BT_UUID_T:
+			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 +113,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_UUID || opt == GATT_OPT_CHR_BT_UUID_T) {
 			info = g_new0(struct gatt_info, 1);
 			l = g_slist_append(l, info);
 		}
@@ -177,16 +182,16 @@ static int find_callback(gconstpointer a, gconstpointer b)
 }
 
 static gboolean add_characteristic(struct btd_adapter *adapter,
-				uint16_t *handle, struct gatt_info *info)
+				   uint16_t *handle, struct gatt_info *info)
 {
 	int read_req, write_req;
 	uint16_t h = *handle;
 	struct attribute *a;
 	bt_uuid_t bt_uuid;
-	uint8_t atval[5];
+	uint8_t atval[131];
 	GSList *l;
 
-	if (!info->uuid.value.u16 || !info->props) {
+	if ((!info->uuid.value.u16 && info->uuid.type != BT_UUID128) || !info->props) {
 		error("Characteristic UUID or properties are missing");
 		return FALSE;
 	}
@@ -222,9 +227,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 */
@@ -341,7 +346,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((uint16_t)(h - start_handle) == (uint16_t) size);
 	g_slist_free_full(chrs, free_gatt_info);
 
 	return TRUE;
diff --git a/attrib/gatt-service.h b/attrib/gatt-service.h
index b810e2e..5c3e15d 100644
--- a/attrib/gatt-service.h
+++ b/attrib/gatt-service.h
@@ -25,6 +25,7 @@
 typedef enum {
 	GATT_OPT_INVALID = 0,
 	GATT_OPT_CHR_UUID,
+	GATT_OPT_CHR_BT_UUID_T,
 	GATT_OPT_CHR_PROPS,
 	GATT_OPT_CHR_VALUE_CB,
 	GATT_OPT_CHR_AUTHENTICATION,
-- 
1.8.3.2

Re: [PATCH] attrib/gatt_service_add: support 128-bit uuids for characteristics

[Johan Hedberg]

Hi Dirk-Jan,

On Tue, Jul 23, 2013, Dirk-Jan C. Binnema wrote:
> +		case GATT_OPT_CHR_BT_UUID_T:
> +			memcpy(&info->uuid, va_arg(args, bt_uuid_t *), sizeof(bt_uuid_t));

The above looks like a too long line to me. Please stick to under 80
characters.

>  static gboolean add_characteristic(struct btd_adapter *adapter,
> -				uint16_t *handle, struct gatt_info *info)
> +				   uint16_t *handle, struct gatt_info *info)

The above coding style change seems unrelated to this patch.

> -	if (!info->uuid.value.u16 || !info->props) {
> +	if ((!info->uuid.value.u16 && info->uuid.type != BT_UUID128) || !info->props) {

Looks like a too long line as well.

info->uuid.value is a union so I don't think it's safe to check any
particular member before you've checked uuid.type. However, isn't what
you want above to check that you have a BT_UUID16 or BT_UUID128? I.e.
there should be no need to check for info->uuid.value.u16 directly.

>  	if (attrib_db_add(adapter, h++, &bt_uuid, ATT_NONE, ATT_NOT_PERMITTED,
> -						atval, sizeof(atval)) == NULL)
> +			  atval, 3 + info->uuid.type/8) == NULL)

It looks like the above change introduces a mix of spaces and tabs for
indentation. The user space style is to use only tabs (indent as much as
you can as long as you don't go over 80 characters).

> -	g_assert(h - start_handle == (uint16_t) size);
> +	g_assert((uint16_t)(h - start_handle) == (uint16_t) size);

Be consistent with the coding style here: type casts should have a space
between the cast and the variable.

>  typedef enum {
>  	GATT_OPT_INVALID = 0,
>  	GATT_OPT_CHR_UUID,
> +	GATT_OPT_CHR_BT_UUID_T,
>  	GATT_OPT_CHR_PROPS,
>  	GATT_OPT_CHR_VALUE_CB,
>  	GATT_OPT_CHR_AUTHENTICATION,

To be honest, I'm not 100% sure this is the best name for the new
option, but I can't really think of anything much better either.
Renaming the existing one to UUID_STR and the new one to UUID (like the
old one was) would be one option, but it's one more patch and more code
changes.

Johan

Re: [PATCH] attrib/gatt_service_add: support 128-bit uuids for characteristics

[Dirk-Jan C. Binnema]

Hi Johan,

Thanks for the quick review! I've updated the too-long lines and taught
emacs to avoid mixing tabs/space in bluez code.

On 2013-07-24 07:57, johan.hedberg@gmail.com wrote:

 > info->uuid.value is a union so I don't think it's safe to check any
 > particular member before you've checked uuid.type. However, isn't what
 > you want above to check that you have a BT_UUID16 or BT_UUID128? I.e.
 > there should be no need to check for info->uuid.value.u16 directly.

Makes sense; note that the value check was in the original code, which I
didn't want to touch. In any case, I've changed it now.

 > Be consistent with the coding style here: type casts should have a space
 > between the cast and the variable.

 > >  typedef enum {
 > >  	GATT_OPT_INVALID = 0,
 > >  	GATT_OPT_CHR_UUID,
 > > +	GATT_OPT_CHR_BT_UUID_T,
 > >  	GATT_OPT_CHR_PROPS,
 > >  	GATT_OPT_CHR_VALUE_CB,
 > >  	GATT_OPT_CHR_AUTHENTICATION,

 > To be honest, I'm not 100% sure this is the best name for the new
 > option, but I can't really think of anything much better either.
 > Renaming the existing one to UUID_STR and the new one to UUID (like the
 > old one was) would be one option, but it's one more patch and more code
 > changes.

Indeed, couldn't come up with a better name myself :/... The old one
takes a uint16_t, so maybe we could call it UUID_16, and perhaps in the
future add UUID_STR for string-encoded 128-bit uuids.

Anyway, I'll send an updated patch.

Cheers,
Dirk.
 

-- 
Dirk-Jan C. Binnema                  Helsinki, Finland
e:djcb@djcbsoftware.nl           w:www.djcbsoftware.nl
pgp: D09C E664 897D 7D39 5047 A178 E96A C7A1 017D DA3C

[PATCH] attrib/gatt_service_add: support 128-bit uuids for characteristics

[Dirk-Jan C. Binnema]

From: "Dirk-Jan C. Binnema" <djcb@djcbsoftware.nl>

Add GATT_OPT_CHR_BT_UUID_T, which does what GATT_OPT_CHR_UUID does, but
instead takes a bt_uuid_t*, so 128-bit uuids are supported as well.

GATT_OPT_CHR_UUID is kept around for backward compatibility.
---
 attrib/gatt-service.c | 19 +++++++++++++------
 attrib/gatt-service.h |  1 +
 2 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/attrib/gatt-service.c b/attrib/gatt-service.c
index 4b02d39..b9e5a70 100644
--- a/attrib/gatt-service.c
+++ b/attrib/gatt-service.c
@@ -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_BT_UUID_T:
+			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_UUID) {
+		if (opt == GATT_OPT_CHR_UUID || opt == GATT_OPT_CHR_BT_UUID_T) {
 			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[131];
 	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 */
@@ -341,7 +348,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((uint16_t) (h - start_handle) == (uint16_t) size);
 	g_slist_free_full(chrs, free_gatt_info);
 
 	return TRUE;
diff --git a/attrib/gatt-service.h b/attrib/gatt-service.h
index b810e2e..5c3e15d 100644
--- a/attrib/gatt-service.h
+++ b/attrib/gatt-service.h
@@ -25,6 +25,7 @@
 typedef enum {
 	GATT_OPT_INVALID = 0,
 	GATT_OPT_CHR_UUID,
+	GATT_OPT_CHR_BT_UUID_T,
 	GATT_OPT_CHR_PROPS,
 	GATT_OPT_CHR_VALUE_CB,
 	GATT_OPT_CHR_AUTHENTICATION,
-- 
1.8.3.2

Re: [PATCH] attrib/gatt_service_add: support 128-bit uuids for characteristics

[Anderson Lizardo]

Hi Dirk-Jan,

On Wed, Jul 24, 2013 at 3:32 AM, Dirk-Jan C. Binnema
<djcb.bulk@gmail.com> wrote:
> @@ -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[131];

Where "131" comes from? A UUID has 128 bits (i.e. 16 bytes). Anyway, I
suggest using atval[ATT_MAX_VALUE_LEN] and not worry about the size
here, as long as you pass the exact site to atttrib_db_add(), as you
do below.

> @@ -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)

Add spaces around "/" here, i.e: 3 + info->uuid.type / 8

>                 return FALSE;
>
>         /* characteristic value */
> @@ -341,7 +348,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((uint16_t) (h - start_handle) == (uint16_t) size);

This actually seems unrelated to the patch. Can you put it into a
separate patch with the compilation error you got?

>         g_slist_free_full(chrs, free_gatt_info);
>
>         return TRUE;
> [...]
> diff --git a/attrib/gatt-service.h b/attrib/gatt-service.h
> index b810e2e..5c3e15d 100644
> --- a/attrib/gatt-service.h
> +++ b/attrib/gatt-service.h
> @@ -25,6 +25,7 @@
>  typedef enum {
>         GATT_OPT_INVALID = 0,
>         GATT_OPT_CHR_UUID,
> +       GATT_OPT_CHR_BT_UUID_T,

My suggestion would be to rename the old one to GATT_OPT_CHR_UUID16
and this could be GATT_OPT_CHR_UUID. But as Johan said, this could be
in a separate patch.

>         GATT_OPT_CHR_PROPS,
>         GATT_OPT_CHR_VALUE_CB,
>         GATT_OPT_CHR_AUTHENTICATION,

Best Regards,
-- 
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil

Re: [PATCH] attrib/gatt_service_add: support 128-bit uuids for characteristics

[Dirk-Jan C. Binnema]

Hi Lizardo,

Thanks for the comments!

On 2013-07-24 13:55, anderson.lizardo@openbossa.org wrote:

 > Hi Dirk-Jan,

 > On Wed, Jul 24, 2013 at 3:32 AM, Dirk-Jan C. Binnema
 > <djcb.bulk@gmail.com> wrote:
 > > @@ -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[131];

 > Where "131" comes from? A UUID has 128 bits (i.e. 16 bytes). Anyway, I
 > suggest using atval[ATT_MAX_VALUE_LEN] and not worry about the size
 > here, as long as you pass the exact site to atttrib_db_add(), as you
 > do below.

Well, a little bit below that, there is:
    att_put_uuid(info->uuid, &atval[3]);
so I guessed I need to reserve the three extra bytes; this is also why
it was 5 before, I think. Or?

ATT_MAX_VALUE_LEN is 512, but it seems to be the maximum size of a
value, rather than the UUID. It's more than enough at least :)

 > > -       g_assert(h - start_handle == (uint16_t) size);
 > > +       g_assert((uint16_t) (h - start_handle) == (uint16_t) size);

 > This actually seems unrelated to the patch. Can you put it into a
 > separate patch with the compilation error you got?

When using 128-bit UUIDs, the start handle ultimately comes from
find_uuid128_avail, which returns 0xffff - nitems + 1 (for the first
item). If I don't explicitly cast it, h - start_handle will be negative
number. So, my patch doesn't really work without this.
 
 > My suggestion would be to rename the old one to GATT_OPT_CHR_UUID16
 > and this could be GATT_OPT_CHR_UUID. But as Johan said, this could be
 > in a separate patch.

 > >         GATT_OPT_CHR_PROPS,
 > >         GATT_OPT_CHR_VALUE_CB,
 > >         GATT_OPT_CHR_AUTHENTICATION,

That makes sense -- but as the newcomer here, I'll happily let the
others reach some consensus on what the best naming would be, and I'll
make a patch after that :-)

Cheers,
Dirk.
 
-- 
Dirk-Jan C. Binnema                  Helsinki, Finland
e:djcb@djcbsoftware.nl           w:www.djcbsoftware.nl
pgp: D09C E664 897D 7D39 5047 A178 E96A C7A1 017D DA3C

Re: [PATCH] attrib/gatt_service_add: support 128-bit uuids for characteristics

[Johan Hedberg]

Hi Dirk,

On Wed, Jul 24, 2013, Dirk-Jan C. Binnema wrote:
>  > My suggestion would be to rename the old one to GATT_OPT_CHR_UUID16
>  > and this could be GATT_OPT_CHR_UUID. But as Johan said, this could be
>  > in a separate patch.
> 
>  > >         GATT_OPT_CHR_PROPS,
>  > >         GATT_OPT_CHR_VALUE_CB,
>  > >         GATT_OPT_CHR_AUTHENTICATION,
> 
> That makes sense -- but as the newcomer here, I'll happily let the
> others reach some consensus on what the best naming would be, and I'll
> make a patch after that :-)

GATT_OPT_CHR_UUID16 and GATT_OPT_CHR_UUID sound fine to me so you can
proceed with those.

Johan

Re: [PATCH] attrib/gatt_service_add: support 128-bit uuids for characteristics

[Anderson Lizardo]

Hi Dirk-Jan,

On Wed, Jul 24, 2013 at 12:43 PM, Dirk-Jan C. Binnema
<djcb.bulk@gmail.com> wrote:
>  > On Wed, Jul 24, 2013 at 3:32 AM, Dirk-Jan C. Binnema
>  > <djcb.bulk@gmail.com> wrote:
>  > > @@ -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[131];
>
>  > Where "131" comes from? A UUID has 128 bits (i.e. 16 bytes). Anyway, I
>  > suggest using atval[ATT_MAX_VALUE_LEN] and not worry about the size
>  > here, as long as you pass the exact site to atttrib_db_add(), as you
>  > do below.
>
> Well, a little bit below that, there is:
>     att_put_uuid(info->uuid, &atval[3]);
> so I guessed I need to reserve the three extra bytes; this is also why
> it was 5 before, I think. Or?

Yes, but 128-bit == 16 bytes, so it should have been 19... but using
ATT_MAX_VALUE_LEN is better so you don't have to worry about the exact
buffer size anyway (it just needs to fit the data).

>
> ATT_MAX_VALUE_LEN is 512, but it seems to be the maximum size of a
> value, rather than the UUID. It's more than enough at least :)

The UUID is being saved into an attribute, and attributes have maximum
value size of 512, that's why I proposed using this define instead of
a hardcoded size.

>
>  > > -       g_assert(h - start_handle == (uint16_t) size);
>  > > +       g_assert((uint16_t) (h - start_handle) == (uint16_t) size);
>
>  > This actually seems unrelated to the patch. Can you put it into a
>  > separate patch with the compilation error you got?
>
> When using 128-bit UUIDs, the start handle ultimately comes from
> find_uuid128_avail, which returns 0xffff - nitems + 1 (for the first
> item). If I don't explicitly cast it, h - start_handle will be negative
> number. So, my patch doesn't really work without this.

Ok, the problem is clearer now. "h" is overflowing for 128-bit
services because the handle is incremented after each attribute is
added so at the end we have: 0xffff + 1 == 0 (h is uint16_t).

As it seems more complex to try to remove the overflow without
affecting the logic too much, can you try the following change
instead:

-       g_assert(h - start_handle == (uint16_t) size);
+       g_assert(h == 0 || (h - start_handle == (uint16_t) size));

In my opinion, the cast you added is dangerous because it will hide
the programming bugs that this g_assert() was supposed to catch.

Given this is a bug on the existing code and not part of the
implementation of GATT_OPT_CHR_UUID, please put it into a separate
patch. This means you should send 2 patches, the first with this fix
and the second with the remaining changes.

>
>  > My suggestion would be to rename the old one to GATT_OPT_CHR_UUID16
>  > and this could be GATT_OPT_CHR_UUID. But as Johan said, this could be
>  > in a separate patch.
>
>  > >         GATT_OPT_CHR_PROPS,
>  > >         GATT_OPT_CHR_VALUE_CB,
>  > >         GATT_OPT_CHR_AUTHENTICATION,
>
> That makes sense -- but as the newcomer here, I'll happily let the
> others reach some consensus on what the best naming would be, and I'll
> make a patch after that :-)

Johan agreed with the name, so feel free to change the existing
occurrences on the code to use CHR_UUID16 instead.

Best Regards,
-- 
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil

[PATCH 1/2] Add support for 128-bit UUIDs for characteristics.

[Dirk-Jan C. Binnema]

From: "Dirk-Jan C. Binnema" <djcb@djcbsoftware.nl>

---
 attrib/gatt-service.c | 19 +++++++++++++------
 attrib/gatt-service.h |  1 +
 2 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/attrib/gatt-service.c b/attrib/gatt-service.c
index 4b02d39..e4ca78c 100644
--- a/attrib/gatt-service.c
+++ b/attrib/gatt-service.c
@@ -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_BT_UUID_T:
+			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_UUID) {
+		if (opt == GATT_OPT_CHR_UUID || opt == GATT_OPT_CHR_BT_UUID_T) {
 			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[131];
 	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 */
@@ -341,7 +348,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((uint16_t) (h - start_handle) == (uint16_t) size);
 	g_slist_free_full(chrs, free_gatt_info);
 
 	return TRUE;
diff --git a/attrib/gatt-service.h b/attrib/gatt-service.h
index b810e2e..5c3e15d 100644
--- a/attrib/gatt-service.h
+++ b/attrib/gatt-service.h
@@ -25,6 +25,7 @@
 typedef enum {
 	GATT_OPT_INVALID = 0,
 	GATT_OPT_CHR_UUID,
+	GATT_OPT_CHR_BT_UUID_T,
 	GATT_OPT_CHR_PROPS,
 	GATT_OPT_CHR_VALUE_CB,
 	GATT_OPT_CHR_AUTHENTICATION,
-- 
1.8.3.2

[PATCH 2/2] Use GATT_OPT_CHR_UUID for bt_uuid_t*, GATT_OPT_CHR_UUID16 for uint16 chrs

[Dirk-Jan C. Binnema]

From: "Dirk-Jan C. Binnema" <djcb@djcbsoftware.nl>

This renames GATT_OPT_CHR_UUID into GATT_OPT_CHR_UUID16, and renames the
(temporary) GATT_OPT_CHR_BT_UUID_T into GATT_OPT_CHR_UUID. Also, updates all
in-tree users.
---
 attrib/gatt-service.c         |  6 +++---
 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, 19 insertions(+), 19 deletions(-)

diff --git a/attrib/gatt-service.c b/attrib/gatt-service.c
index e4ca78c..86e0093 100644
--- a/attrib/gatt-service.c
+++ b/attrib/gatt-service.c
@@ -68,12 +68,12 @@ 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;
 			break;
-		case GATT_OPT_CHR_BT_UUID_T:
+		case GATT_OPT_CHR_UUID:
 			memcpy(&info->uuid, va_arg(args, bt_uuid_t *),
 				sizeof(bt_uuid_t));
 			/* characteristic declaration and value */
@@ -114,7 +114,7 @@ static GSList *parse_opts(gatt_option opt1, va_list args)
 		}
 
 		opt = va_arg(args, gatt_option);
-		if (opt == GATT_OPT_CHR_UUID || opt == GATT_OPT_CHR_BT_UUID_T) {
+		if (opt == GATT_OPT_CHR_UUID16 || opt == GATT_OPT_CHR_UUID) {
 			info = g_new0(struct gatt_info, 1);
 			l = g_slist_append(l, info);
 		}
diff --git a/attrib/gatt-service.h b/attrib/gatt-service.h
index 5c3e15d..2c05c7d 100644
--- a/attrib/gatt-service.h
+++ b/attrib/gatt-service.h
@@ -24,8 +24,8 @@
 
 typedef enum {
 	GATT_OPT_INVALID = 0,
+	GATT_OPT_CHR_UUID16,
 	GATT_OPT_CHR_UUID,
-	GATT_OPT_CHR_BT_UUID_T,
 	GATT_OPT_CHR_PROPS,
 	GATT_OPT_CHR_VALUE_CB,
 	GATT_OPT_CHR_AUTHENTICATION,
diff --git a/plugins/gatt-example.c b/plugins/gatt-example.c
index 37972e8..b926947 100644
--- a/plugins/gatt-example.c
+++ b/plugins/gatt-example.c
@@ -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,
diff --git a/profiles/alert/server.c b/profiles/alert/server.c
index 8da1928..4565470 100644
--- a/profiles/alert/server.c
+++ b/profiles/alert/server.c
@@ -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,
diff --git a/profiles/proximity/immalert.c b/profiles/proximity/immalert.c
index 06e8eb8..2f8d4e7 100644
--- a/profiles/proximity/immalert.c
+++ b/profiles/proximity/immalert.c
@@ -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,
diff --git a/profiles/proximity/linkloss.c b/profiles/proximity/linkloss.c
index cb87b12..a7ed96c 100644
--- a/profiles/proximity/linkloss.c
+++ b/profiles/proximity/linkloss.c
@@ -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,
diff --git a/profiles/time/server.c b/profiles/time/server.c
index 518a29f..178d4d2 100644
--- a/profiles/time/server.c
+++ b/profiles/time/server.c
@@ -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,
-- 
1.8.3.2

Re: [PATCH] attrib/gatt_service_add: support 128-bit uuids for characteristics

[Dirk-Jan C. Binnema]

Hi,

Was a bit too quick with my updated patch! Anyway...

On 2013-07-24 20:38, anderson.lizardo@openbossa.org wrote:

 > Yes, but 128-bit == 16 bytes, so it should have been 19... but using
 > ATT_MAX_VALUE_LEN is better so you don't have to worry about the exact
 > buffer size anyway (it just needs to fit the data).

 > > ATT_MAX_VALUE_LEN is 512, but it seems to be the maximum size of a
 > > value, rather than the UUID. It's more than enough at least :)

 > The UUID is being saved into an attribute, and attributes have maximum
 > value size of 512, that's why I proposed using this define instead of
 > a hardcoded size.

Oh, I see your point. I'll update the patch.

 > >  > > -       g_assert(h - start_handle == (uint16_t) size);
 > >  > > +       g_assert((uint16_t) (h - start_handle) == (uint16_t) size);
 > >
 > >  > This actually seems unrelated to the patch. Can you put it into a
 > >  > separate patch with the compilation error you got?
 > >
 > > When using 128-bit UUIDs, the start handle ultimately comes from
 > > find_uuid128_avail, which returns 0xffff - nitems + 1 (for the first
 > > item). If I don't explicitly cast it, h - start_handle will be negative
 > > number. So, my patch doesn't really work without this.

 > Ok, the problem is clearer now. "h" is overflowing for 128-bit
 > services because the handle is incremented after each attribute is
 > added so at the end we have: 0xffff + 1 == 0 (h is uint16_t).

 > As it seems more complex to try to remove the overflow without
 > affecting the logic too much, can you try the following change
 > instead:

 > -       g_assert(h - start_handle == (uint16_t) size);
 > +       g_assert(h == 0 || (h - start_handle == (uint16_t) size));

 > In my opinion, the cast you added is dangerous because it will hide
 > the programming bugs that this g_assert() was supposed to catch.

 Sure, will do that, too.
 
 > Given this is a bug on the existing code and not part of the
 > implementation of GATT_OPT_CHR_UUID, please put it into a separate
 > patch. This means you should send 2 patches, the first with this fix
 > and the second with the remaining changes.

Okidoki. I actually split the original patch in two, so their separately
applicable.

Cheers,
Dirk.
  

-- 
Dirk-Jan C. Binnema                  Helsinki, Finland
e:djcb@djcbsoftware.nl           w:www.djcbsoftware.nl
pgp: D09C E664 897D 7D39 5047 A178 E96A C7A1 017D DA3C

[PATCH 1/3] gatt-service: fix assertion in gatt_service_add for 128-bit uuids

[Dirk-Jan C. Binnema]

From: "Dirk-Jan C. Binnema" <djcb@djcbsoftware.nl>

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

diff --git a/attrib/gatt-service.c b/attrib/gatt-service.c
index 4b02d39..1b84c89 100644
--- a/attrib/gatt-service.c
+++ b/attrib/gatt-service.c
@@ -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;
-- 
1.8.3.2

[PATCH 2/3] gatt_service_add: add support for 128-bit uuids for characteristics

[Dirk-Jan C. Binnema]

From: "Dirk-Jan C. Binnema" <djcb@djcbsoftware.nl>

---
 attrib/gatt-service.c | 17 ++++++++++++-----
 attrib/gatt-service.h |  1 +
 2 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/attrib/gatt-service.c b/attrib/gatt-service.c
index 1b84c89..3d9414d 100644
--- a/attrib/gatt-service.c
+++ b/attrib/gatt-service.c
@@ -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_BT_UUID_T:
+			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_UUID) {
+		if (opt == GATT_OPT_CHR_UUID || opt == GATT_OPT_CHR_BT_UUID_T) {
 			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 */
diff --git a/attrib/gatt-service.h b/attrib/gatt-service.h
index b810e2e..5c3e15d 100644
--- a/attrib/gatt-service.h
+++ b/attrib/gatt-service.h
@@ -25,6 +25,7 @@
 typedef enum {
 	GATT_OPT_INVALID = 0,
 	GATT_OPT_CHR_UUID,
+	GATT_OPT_CHR_BT_UUID_T,
 	GATT_OPT_CHR_PROPS,
 	GATT_OPT_CHR_VALUE_CB,
 	GATT_OPT_CHR_AUTHENTICATION,
-- 
1.8.3.2

[PATCH 3/3] Use GATT_OPT_CHR_UUID for bt_uuid_t*, GATT_OPT_CHR_UUID16 for uint16 chrs

[Dirk-Jan C. Binnema]

From: "Dirk-Jan C. Binnema" <djcb@djcbsoftware.nl>

This renames GATT_OPT_CHR_UUID into GATT_OPT_CHR_UUID16, and renames the
(temporary) GATT_OPT_CHR_BT_UUID_T into GATT_OPT_CHR_UUID. Also, updates all
in-tree users.
---
 attrib/gatt-service.c         |  6 +++---
 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, 19 insertions(+), 19 deletions(-)

diff --git a/attrib/gatt-service.c b/attrib/gatt-service.c
index 3d9414d..4cd300f 100644
--- a/attrib/gatt-service.c
+++ b/attrib/gatt-service.c
@@ -68,12 +68,12 @@ 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;
 			break;
-		case GATT_OPT_CHR_BT_UUID_T:
+		case GATT_OPT_CHR_UUID:
 			memcpy(&info->uuid, va_arg(args, bt_uuid_t *),
 				sizeof(bt_uuid_t));
 			/* characteristic declaration and value */
@@ -114,7 +114,7 @@ static GSList *parse_opts(gatt_option opt1, va_list args)
 		}
 
 		opt = va_arg(args, gatt_option);
-		if (opt == GATT_OPT_CHR_UUID || opt == GATT_OPT_CHR_BT_UUID_T) {
+		if (opt == GATT_OPT_CHR_UUID16 || opt == GATT_OPT_CHR_UUID) {
 			info = g_new0(struct gatt_info, 1);
 			l = g_slist_append(l, info);
 		}
diff --git a/attrib/gatt-service.h b/attrib/gatt-service.h
index 5c3e15d..2c05c7d 100644
--- a/attrib/gatt-service.h
+++ b/attrib/gatt-service.h
@@ -24,8 +24,8 @@
 
 typedef enum {
 	GATT_OPT_INVALID = 0,
+	GATT_OPT_CHR_UUID16,
 	GATT_OPT_CHR_UUID,
-	GATT_OPT_CHR_BT_UUID_T,
 	GATT_OPT_CHR_PROPS,
 	GATT_OPT_CHR_VALUE_CB,
 	GATT_OPT_CHR_AUTHENTICATION,
diff --git a/plugins/gatt-example.c b/plugins/gatt-example.c
index 37972e8..b926947 100644
--- a/plugins/gatt-example.c
+++ b/plugins/gatt-example.c
@@ -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,
diff --git a/profiles/alert/server.c b/profiles/alert/server.c
index 8da1928..4565470 100644
--- a/profiles/alert/server.c
+++ b/profiles/alert/server.c
@@ -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,
diff --git a/profiles/proximity/immalert.c b/profiles/proximity/immalert.c
index 06e8eb8..2f8d4e7 100644
--- a/profiles/proximity/immalert.c
+++ b/profiles/proximity/immalert.c
@@ -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,
diff --git a/profiles/proximity/linkloss.c b/profiles/proximity/linkloss.c
index cb87b12..a7ed96c 100644
--- a/profiles/proximity/linkloss.c
+++ b/profiles/proximity/linkloss.c
@@ -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,
diff --git a/profiles/time/server.c b/profiles/time/server.c
index 518a29f..178d4d2 100644
--- a/profiles/time/server.c
+++ b/profiles/time/server.c
@@ -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,
-- 
1.8.3.2