2011-10-24 09:20:04

by Andrei Emeltchenko

[permalink] [raw]
Subject: [PATCH bluez] add get_le/get_be helpers

From: Andrei Emeltchenko <[email protected]>

Helpers to access LE / BE values. In bluetooth there is a mixture
of LE / BE network byte order.
---
lib/bluetooth.h | 64 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 64 insertions(+), 0 deletions(-)

diff --git a/lib/bluetooth.h b/lib/bluetooth.h
index b0680e2..5bd4f03 100644
--- a/lib/bluetooth.h
+++ b/lib/bluetooth.h
@@ -125,6 +125,70 @@ do { \
__p->__v = (val); \
} while(0)

+#if __BYTE_ORDER == __LITTLE_ENDIAN
+static inline uint64_t bt_get_le64(void *ptr)
+{
+ return bt_get_unaligned((uint64_t *) ptr);
+}
+
+static inline uint64_t bt_get_be64(void *ptr)
+{
+ return bswap_64(bt_get_unaligned((uint64_t *) ptr));
+}
+
+static inline uint32_t bt_get_le32(void *ptr)
+{
+ return bt_get_unaligned((uint32_t *) ptr);
+}
+
+static inline uint32_t bt_get_be32(void *ptr)
+{
+ return bswap_32(bt_get_unaligned((uint32_t *) ptr));
+}
+
+static inline uint16_t bt_get_le16(void *ptr)
+{
+ return bt_get_unaligned((uint16_t *) ptr);
+}
+
+static inline uint16_t bt_get_be16(void *ptr)
+{
+ return bswap_16(bt_get_unaligned((uint16_t *) ptr));
+}
+#elif __BYTE_ORDER == __BIG_ENDIAN
+static inline uint64_t bt_get_le64(void *ptr)
+{
+ return bswap_64(bt_get_unaligned((uint64_t *) ptr));
+}
+
+static inline uint64_t bt_get_be64(void *ptr)
+{
+ return bt_get_unaligned((uint64_t *) ptr);
+}
+
+static inline uint32_t bt_get_le32(void *ptr)
+{
+ return bswap_32(bt_get_unaligned((uint32_t *) ptr));
+}
+
+static inline uint32_t bt_get_be32(void *ptr)
+{
+ return bt_get_unaligned((uint32_t *) ptr);
+}
+
+static inline uint16_t bt_get_le16(void *ptr)
+{
+ return bswap_16(bt_get_unaligned((uint16_t *) ptr));
+}
+
+static inline uint16_t bt_get_be16(void *ptr)
+{
+ return bt_get_unaligned((uint16_t *) ptr);
+}
+#else
+#error "Unknown byte order"
+#endif
+
/* BD Address */
typedef struct {
uint8_t b[6];
--
1.7.4.1



2011-10-25 09:22:03

by Johan Hedberg

[permalink] [raw]
Subject: Re: [PATCH bluez] add get_le/get_be helpers

Hi Lucas,

On Tue, Oct 25, 2011, Lucas De Marchi wrote:
> On Tue, Oct 25, 2011 at 6:52 AM, Johan Hedberg <[email protected]> wrote:
> > Hi Andrei,
> >
> > On Mon, Oct 24, 2011, Emeltchenko Andrei wrote:
> >> Helpers to access LE / BE values. In bluetooth there is a mixture
> >> of LE / BE network byte order.
> >> ---
> >> ?lib/bluetooth.h | ? 64 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >> ?1 files changed, 64 insertions(+), 0 deletions(-)
> >
> > Applied. Thanks.
>
> Should we convert code to use these new functions?

Yes, please.

Johan

2011-10-25 09:34:29

by Lucas De Marchi

[permalink] [raw]
Subject: Re: [PATCH bluez] add get_le/get_be helpers

Hi Johan,

On Tue, Oct 25, 2011 at 6:52 AM, Johan Hedberg <[email protected]> wrote:
> Hi Andrei,
>
> On Mon, Oct 24, 2011, Emeltchenko Andrei wrote:
>> Helpers to access LE / BE values. In bluetooth there is a mixture
>> of LE / BE network byte order.
>> ---
>> ?lib/bluetooth.h | ? 64 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>> ?1 files changed, 64 insertions(+), 0 deletions(-)
>
> Applied. Thanks.

Should we convert code to use these new functions?


Lucas De Marchi

2011-10-25 08:52:42

by Johan Hedberg

[permalink] [raw]
Subject: Re: [PATCH bluez] add get_le/get_be helpers

Hi Andrei,

On Mon, Oct 24, 2011, Emeltchenko Andrei wrote:
> Helpers to access LE / BE values. In bluetooth there is a mixture
> of LE / BE network byte order.
> ---
> lib/bluetooth.h | 64 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 files changed, 64 insertions(+), 0 deletions(-)

Applied. Thanks.

Please start your commit messages with a capital letter in the future so
I don't have to keep fixing them manually.

Johan

2011-10-24 12:06:37

by Vinicius Costa Gomes

[permalink] [raw]
Subject: Re: [PATCH bluez] add get_le/get_be helpers

Hi Marcel,

On Mon, Oct 24, 2011 at 1:50 PM, Marcel Holtmann <[email protected]> wrote:
> Hi Vinicius,
>
>> Sorry if this comes too late, but here's an idea:
>>
>> How about changing the name of the functions to something a little
>> more high level, for example: in attrib/att.h we have
>> att_{get,put}_u16() that uses the byte order defined in the ATT spec
>> (little endian).
>>
>> So my suggestion is to have bt_{get,put}_u*() and sdp_{get,put}_u*
>> functions (perhaps also no_{get,put}_u* for cases when we use the host
>> byte order). The bt_ functions will be used for everything that uses
>> the Bluetooth byte order and the sdp_ ones for SDP, which is the only
>> case of Big Endian defined by Bluetooth, correct me if I am wrong.
>
> lets not try to be too smart. We want the code clearly identify what
> endian is expected. Same as the kernel handles this.
>

After reading Szymon comments I came to the same conclusion. This more
explicit approach makes more sense.


> Regards
>
> Marcel
>
>
>


Cheers,
--
Vinicius

2011-10-24 11:50:16

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH bluez] add get_le/get_be helpers

Hi Vinicius,

> Sorry if this comes too late, but here's an idea:
>
> How about changing the name of the functions to something a little
> more high level, for example: in attrib/att.h we have
> att_{get,put}_u16() that uses the byte order defined in the ATT spec
> (little endian).
>
> So my suggestion is to have bt_{get,put}_u*() and sdp_{get,put}_u*
> functions (perhaps also no_{get,put}_u* for cases when we use the host
> byte order). The bt_ functions will be used for everything that uses
> the Bluetooth byte order and the sdp_ ones for SDP, which is the only
> case of Big Endian defined by Bluetooth, correct me if I am wrong.

lets not try to be too smart. We want the code clearly identify what
endian is expected. Same as the kernel handles this.

Regards

Marcel



2011-10-24 10:27:41

by Szymon Janc

[permalink] [raw]
Subject: Re: [PATCH bluez] add get_le/get_be helpers

Hi,

> Sorry if this comes too late, but here's an idea:
>
> How about changing the name of the functions to something a little
> more high level, for example: in attrib/att.h we have
> att_{get,put}_u16() that uses the byte order defined in the ATT spec
> (little endian).
>
> So my suggestion is to have bt_{get,put}_u*() and sdp_{get,put}_u*
> functions (perhaps also no_{get,put}_u* for cases when we use the host
> byte order). The bt_ functions will be used for everything that uses
> the Bluetooth byte order and the sdp_ ones for SDP, which is the only
> case of Big Endian defined by Bluetooth, correct me if I am wrong.
>
> What do you think?

Some profiles e.g. AVRCP and SAP use big endian...

--
BR
Szymon

2011-10-24 10:26:06

by Andrei Emeltchenko

[permalink] [raw]
Subject: Re: [PATCH bluez] add get_le/get_be helpers

Hi Vinicius,

On Mon, Oct 24, 2011 at 12:13:43PM +0200, Vinicius Gomes wrote:
> Hi Andrei,
>
> On Mon, Oct 24, 2011 at 11:20 AM, Emeltchenko Andrei
> <[email protected]> wrote:
> > From: Andrei Emeltchenko <[email protected]>
> >
> > Helpers to access LE / BE values. In bluetooth there is a mixture
> > of LE / BE network byte order.
> > ---
>
> Sorry if this comes too late, but here's an idea:
>
> How about changing the name of the functions to something a little
> more high level, for example: in attrib/att.h we have
> att_{get,put}_u16() that uses the byte order defined in the ATT spec
> (little endian).
>
> So my suggestion is to have bt_{get,put}_u*() and sdp_{get,put}_u*
> functions (perhaps also no_{get,put}_u* for cases when we use the host
> byte order). The bt_ functions will be used for everything that uses
> the Bluetooth byte order and the sdp_ ones for SDP, which is the only

maybe we can define bt_{get,put}_u*() as bt_{get,put}_le*() and be with
SDP?

Best regards
Andrei Emeltchenko

> case of Big Endian defined by Bluetooth, correct me if I am wrong.
>
> What do you think?
>
> > ?lib/bluetooth.h | ? 64 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > ?1 files changed, 64 insertions(+), 0 deletions(-)
> >
> > diff --git a/lib/bluetooth.h b/lib/bluetooth.h
> > index b0680e2..5bd4f03 100644
> > --- a/lib/bluetooth.h
> > +++ b/lib/bluetooth.h
> > @@ -125,6 +125,70 @@ do { ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? \
> > ? ? ? ?__p->__v = (val); ? ? ? ? ? ? ? ? ? ? ? \
> > ?} while(0)
> >
> > +#if __BYTE_ORDER == __LITTLE_ENDIAN
> > +static inline uint64_t bt_get_le64(void *ptr)
> > +{
> > + ? ? ? return bt_get_unaligned((uint64_t *) ptr);
> > +}
> > +
> > +static inline uint64_t bt_get_be64(void *ptr)
> > +{
> > + ? ? ? return bswap_64(bt_get_unaligned((uint64_t *) ptr));
> > +}
> > +
> > +static inline uint32_t bt_get_le32(void *ptr)
> > +{
> > + ? ? ? return bt_get_unaligned((uint32_t *) ptr);
> > +}
> > +
> > +static inline uint32_t bt_get_be32(void *ptr)
> > +{
> > + ? ? ? return bswap_32(bt_get_unaligned((uint32_t *) ptr));
> > +}
> > +
> > +static inline uint16_t bt_get_le16(void *ptr)
> > +{
> > + ? ? ? return bt_get_unaligned((uint16_t *) ptr);
> > +}
> > +
> > +static inline uint16_t bt_get_be16(void *ptr)
> > +{
> > + ? ? ? return bswap_16(bt_get_unaligned((uint16_t *) ptr));
> > +}
> > +#elif __BYTE_ORDER == __BIG_ENDIAN
> > +static inline uint64_t bt_get_le64(void *ptr)
> > +{
> > + ? ? ? return bswap_64(bt_get_unaligned((uint64_t *) ptr));
> > +}
> > +
> > +static inline uint64_t bt_get_be64(void *ptr)
> > +{
> > + ? ? ? return bt_get_unaligned((uint64_t *) ptr);
> > +}
> > +
> > +static inline uint32_t bt_get_le32(void *ptr)
> > +{
> > + ? ? ? return bswap_32(bt_get_unaligned((uint32_t *) ptr));
> > +}
> > +
> > +static inline uint32_t bt_get_be32(void *ptr)
> > +{
> > + ? ? ? return bt_get_unaligned((uint32_t *) ptr);
> > +}
> > +
> > +static inline uint16_t bt_get_le16(void *ptr)
> > +{
> > + ? ? ? return bswap_16(bt_get_unaligned((uint16_t *) ptr));
> > +}
> > +
> > +static inline uint16_t bt_get_be16(void *ptr)
> > +{
> > + ? ? ? return bt_get_unaligned((uint16_t *) ptr);
> > +}
> > +#else
> > +#error "Unknown byte order"
> > +#endif
> > +
> > ?/* BD Address */
> > ?typedef struct {
> > ? ? ? ?uint8_t b[6];
> > --
> > 1.7.4.1
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> > the body of a message to [email protected]
> > More majordomo info at ?http://vger.kernel.org/majordomo-info.html
> >
>
>
> Cheers,
> --
> Vinicius

2011-10-24 10:13:43

by Vinicius Costa Gomes

[permalink] [raw]
Subject: Re: [PATCH bluez] add get_le/get_be helpers

Hi Andrei,

On Mon, Oct 24, 2011 at 11:20 AM, Emeltchenko Andrei
<[email protected]> wrote:
> From: Andrei Emeltchenko <[email protected]>
>
> Helpers to access LE / BE values. In bluetooth there is a mixture
> of LE / BE network byte order.
> ---

Sorry if this comes too late, but here's an idea:

How about changing the name of the functions to something a little
more high level, for example: in attrib/att.h we have
att_{get,put}_u16() that uses the byte order defined in the ATT spec
(little endian).

So my suggestion is to have bt_{get,put}_u*() and sdp_{get,put}_u*
functions (perhaps also no_{get,put}_u* for cases when we use the host
byte order). The bt_ functions will be used for everything that uses
the Bluetooth byte order and the sdp_ ones for SDP, which is the only
case of Big Endian defined by Bluetooth, correct me if I am wrong.

What do you think?

>  lib/bluetooth.h |   64 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 64 insertions(+), 0 deletions(-)
>
> diff --git a/lib/bluetooth.h b/lib/bluetooth.h
> index b0680e2..5bd4f03 100644
> --- a/lib/bluetooth.h
> +++ b/lib/bluetooth.h
> @@ -125,6 +125,70 @@ do {                                               \
>        __p->__v = (val);                       \
>  } while(0)
>
> +#if __BYTE_ORDER == __LITTLE_ENDIAN
> +static inline uint64_t bt_get_le64(void *ptr)
> +{
> +       return bt_get_unaligned((uint64_t *) ptr);
> +}
> +
> +static inline uint64_t bt_get_be64(void *ptr)
> +{
> +       return bswap_64(bt_get_unaligned((uint64_t *) ptr));
> +}
> +
> +static inline uint32_t bt_get_le32(void *ptr)
> +{
> +       return bt_get_unaligned((uint32_t *) ptr);
> +}
> +
> +static inline uint32_t bt_get_be32(void *ptr)
> +{
> +       return bswap_32(bt_get_unaligned((uint32_t *) ptr));
> +}
> +
> +static inline uint16_t bt_get_le16(void *ptr)
> +{
> +       return bt_get_unaligned((uint16_t *) ptr);
> +}
> +
> +static inline uint16_t bt_get_be16(void *ptr)
> +{
> +       return bswap_16(bt_get_unaligned((uint16_t *) ptr));
> +}
> +#elif __BYTE_ORDER == __BIG_ENDIAN
> +static inline uint64_t bt_get_le64(void *ptr)
> +{
> +       return bswap_64(bt_get_unaligned((uint64_t *) ptr));
> +}
> +
> +static inline uint64_t bt_get_be64(void *ptr)
> +{
> +       return bt_get_unaligned((uint64_t *) ptr);
> +}
> +
> +static inline uint32_t bt_get_le32(void *ptr)
> +{
> +       return bswap_32(bt_get_unaligned((uint32_t *) ptr));
> +}
> +
> +static inline uint32_t bt_get_be32(void *ptr)
> +{
> +       return bt_get_unaligned((uint32_t *) ptr);
> +}
> +
> +static inline uint16_t bt_get_le16(void *ptr)
> +{
> +       return bswap_16(bt_get_unaligned((uint16_t *) ptr));
> +}
> +
> +static inline uint16_t bt_get_be16(void *ptr)
> +{
> +       return bt_get_unaligned((uint16_t *) ptr);
> +}
> +#else
> +#error "Unknown byte order"
> +#endif
> +
>  /* BD Address */
>  typedef struct {
>        uint8_t b[6];
> --
> 1.7.4.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to [email protected]
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>


Cheers,
--
Vinicius