[PATCH] bluetooth.h: fix compile issue when using in C++

[PATCH] bluetooth.h: fix compile issue when using in C++

[Patrick Ohly]

The compiler error is:
 /usr/include/bluetooth/bluetooth.h::131:9: error: invalid conversion from 'void*' to 'bt_get_le64(void*)::<anonymous struct>*'
 ...

The reason is that C++, in contrast to C, does not allow conversion of
void * to anything, and this code gets compiled as C++ when the app is
written in C++. The macro with the assignment itself is older, but only
recent Bluez starts to use it in inline functions, thus triggering the
problem.

This patch keeps the "struct __attribute__((packed))" magic and merely
changes the typecast so that it works in C and C++. Like the existing
macro this patch relies on support for typeof.
---
 lib/bluetooth.h |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/bluetooth.h b/lib/bluetooth.h
index 5bd4f03..2fee4ac 100644
--- a/lib/bluetooth.h
+++ b/lib/bluetooth.h
@@ -113,7 +113,7 @@ enum {
 ({						\
 	struct __attribute__((packed)) {	\
 		typeof(*(ptr)) __v;		\
-	} *__p = (void *) (ptr);		\
+	} *__p = (typeof(__p)) (ptr);		\
 	__p->__v;				\
 })
 
@@ -121,7 +121,7 @@ enum {
 do {						\
 	struct __attribute__((packed)) {	\
 		typeof(*(ptr)) __v;		\
-	} *__p = (void *) (ptr);		\
+	} *__p = (typeof(__p)) (ptr);		\
 	__p->__v = (val);			\
 } while(0)
 
-- 
1.7.7.3

Re: [PATCH] bluetooth.h: fix compile issue when using in C++

[Marcel Holtmann]

Hi Patrick,

> The compiler error is:
>  /usr/include/bluetooth/bluetooth.h::131:9: error: invalid conversion from 'void*' to 'bt_get_le64(void*)::<anonymous struct>*'
>  ...
> 
> The reason is that C++, in contrast to C, does not allow conversion of
> void * to anything, and this code gets compiled as C++ when the app is
> written in C++. The macro with the assignment itself is older, but only
> recent Bluez starts to use it in inline functions, thus triggering the
> problem.
> 
> This patch keeps the "struct __attribute__((packed))" magic and merely
> changes the typecast so that it works in C and C++. Like the existing
> macro this patch relies on support for typeof.

so I am applying this patch now, but I still have no idea what kind of
stupid C++ compiler you are using. The section of this include is
clearly marked as C code:

#ifdef __cplusplus
extern "C" {
#endif

Has anybody actually tested this on ARM where we do need the unaligned
access?

Regards

Marcel

Re: [PATCH] bluetooth.h: fix compile issue when using in C++

[Patrick Ohly]

On Sun, 2012-03-04 at 15:27 -0800, Marcel Holtmann wrote:
> Hi Patrick,
> 
> > The compiler error is:
> >  /usr/include/bluetooth/bluetooth.h::131:9: error: invalid conversion from 'void*' to 'bt_get_le64(void*)::<anonymous struct>*'
> >  ...
> > 
> > The reason is that C++, in contrast to C, does not allow conversion of
> > void * to anything, and this code gets compiled as C++ when the app is
> > written in C++. The macro with the assignment itself is older, but only
> > recent Bluez starts to use it in inline functions, thus triggering the
> > problem.
> > 
> > This patch keeps the "struct __attribute__((packed))" magic and merely
> > changes the typecast so that it works in C and C++. Like the existing
> > macro this patch relies on support for typeof.
> 
> so I am applying this patch now, but I still have no idea what kind of
> stupid C++ compiler you are using.

The default g++ as used by affected distros.

>  The section of this include is
> clearly marked as C code:
> 
> #ifdef __cplusplus
> extern "C" {
> #endif

That doesn't mean that the C++ compiler falls back to C mode. It merely
uses C names for the symbols in that section.

-- 
Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.

Re: [PATCH] bluetooth.h: fix compile issue when using in C++

[Patrick Ohly]

[-- Attachment #1: Type: text/plain, Size: 1224 bytes --]

On Mon, 2012-03-05 at 09:31 +0100, Markus Rathgeb wrote:
> Before you are applying the patch, could you also have a look at:
> http://permalink.gmane.org/gmane.linux.bluez.kernel/22294

That's a second C++ problem with the header file. With typeof and -std=c
++0x I get an error, as you said.

I'm fine with using __typeof__ instead of typeof. I verified that g++
4.6.2 and clang++ 3.0 grok __typeof__, with and without -std=c++0x. I
also checked that gcc accepts it without -std, with -std=gnu89, and with
-std=c99. It rejects the header file with -std=c89 because of the inline
keyword.

Updated patch attached.

Note that Debian seems to have picked the older
http://marc.info/?l=linux-bluetooth&m=132644289619172&w=2 patch, the one
which removes __attribute__((packed)) and replaces the (void *) cast
with (struct __s *).

I think that shows that it is important that upstream includes a
solution, because otherwise distros will pick one randomly.

-- 
Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.


[-- Attachment #2: 0001-bluetooth.h-fix-typecast-and-typeof-compile-issue-wh.patch --]
[-- Type: text/x-patch, Size: 2136 bytes --]

>From 51f8b4d8343454b04e7ed83b7770185d718d6203 Mon Sep 17 00:00:00 2001
From: Patrick Ohly <patrick.ohly@intel.com>
Date: Mon, 16 Jan 2012 10:58:44 +0100
Subject: [PATCH] bluetooth.h: fix typecast and typeof compile issue when used
 in C++
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The compiler error for the typecast is:
 /usr/include/bluetooth/bluetooth.h::131:9: error: invalid conversion from 'void*' to 'bt_get_le64(void*)::<anonymous struct>*'
 ...

The reason is that C++, in contrast to C, does not allow conversion of
void * to anything, and this code gets compiled as C++ when the app is
written in C++. The macro with the assignment itself is older, but only
recent Bluez starts to use it in inline functions, thus triggering the
problem.

Another issue in combination with -std=c++0x is the use of typeof,
which is not a standard C++ feature and thus gets rejected in strict
mode.

This patch keeps the "struct __attribute__((packed))" magic and merely
changes the typecast so that it works in C and C++. typeof gets replaced
with __typeof__.

g++ 4.6.2 and clang++ 3.0-5 accept the modified header file, with and
without -std=c++0x. gcc 4.6.2 accepts it without -std, with -std=gnu89,
and with -std=c99. It rejects the header file with -std=c89 because of
the inline keyword.

Signed-off-by: Patrick Ohly <patrick.ohly@intel.com>
---
 lib/bluetooth.h |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/lib/bluetooth.h b/lib/bluetooth.h
index 5bd4f03..297d52d 100644
--- a/lib/bluetooth.h
+++ b/lib/bluetooth.h
@@ -112,16 +112,16 @@ enum {
 #define bt_get_unaligned(ptr)			\
 ({						\
 	struct __attribute__((packed)) {	\
-		typeof(*(ptr)) __v;		\
-	} *__p = (void *) (ptr);		\
+		__typeof__(*(ptr)) __v;		\
+	} *__p = (__typeof__(__p)) (ptr);	\
 	__p->__v;				\
 })
 
 #define bt_put_unaligned(val, ptr)		\
 do {						\
 	struct __attribute__((packed)) {	\
-		typeof(*(ptr)) __v;		\
-	} *__p = (void *) (ptr);		\
+		__typeof__(*(ptr)) __v;		\
+	} *__p = (__typeof__(__p)) (ptr);	\
 	__p->__v = (val);			\
 } while(0)
 
-- 
1.7.9

Re: [PATCH] bluetooth.h: fix compile issue when using in C++

[Markus Rathgeb]

2012/3/5 Patrick Ohly <patrick.ohly@intel.com>:
> I'm fine with using __typeof__ instead of typeof. I verified that g++
> 4.6.2 and clang++ 3.0 grok __typeof__, with and without -std=c++0x. I
> also checked that gcc accepts it without -std, with -std=gnu89, and with
> -std=c99. It rejects the header file with -std=c89 because of the inline
> keyword.

The gcc documentation states also this for the typeof C extension:

"If you are writing a header file that must work when included in ISO
C programs, write __typeof__ instead of typeof."

Source: http://gcc.gnu.org/onlinedocs/gcc/Typeof.html

Re: [PATCH] bluetooth.h: fix compile issue when using in C++

[W. Trevor King]

[-- Attachment #1: Type: text/plain, Size: 1065 bytes --]

On Mon, Mar 05, 2012 at 01:12:47PM -0000, Patrick Ohly wrote:
> From: Patrick Ohly <patrick.ohly@intel.com>
> Date: Mon, 16 Jan 2012 10:58:44 +0100
> Subject: [PATCH] bluetooth.h: fix typecast and typeof compile issue when used
>  in C++
>
> …
>
> Another issue in combination with -std=c++0x is the use of typeof,
> which is not a standard C++ feature and thus gets rejected in strict
> mode.
> 
> This patch keeps the "struct __attribute__((packed))" magic and merely
> changes the typecast so that it works in C and C++. typeof gets replaced
> with __typeof__.

This patch seems to have never made it into bluez, and I just bumped
into the typeof/__typeof__ issue again today.  I could not find any
comments in the mailing list explaining if or why the patch was
rejected, so perhaps it just fell through the cracks?

Is there any chance of getting it accepted?

Cheers,
Trevor

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] bluetooth.h: fix compile issue when using in C++

[W. Trevor King]

[-- Attachment #1: Type: text/plain, Size: 1484 bytes --]

On Mon, Mar 05, 2012 at 01:12:47PM -0000, Patrick Ohly wrote:
> On Mon, 2012-03-05 at 09:31 +0100, Markus Rathgeb wrote:
> > Before you are applying the patch, could you also have a look at:
> > http://permalink.gmane.org/gmane.linux.bluez.kernel/22294
> 
[snip]
> 
> Updated patch attached.
> 
> Note that Debian seems to have picked the older
> http://marc.info/?l=linux-bluetooth&m=132644289619172&w=2 patch, the one
> which removes __attribute__((packed)) and replaces the (void *) cast
> with (struct __s *).
> 
> I think that shows that it is important that upstream includes a
> solution, because otherwise distros will pick one randomly.

On Fri, Jul 27, 2012 at 12:38:53PM -0400, W. Trevor King wrote:
> This patch seems to have never made it into bluez,
[snip]
> Is there any chance of getting it accepted?

On Wed, Aug 01, 2012 at 04:41:33PM +0200, Pacho Ramos wrote:
> El dom, 29-07-2012 a las 09:52 +0200, Pacho Ramos escribió:
> > Today I got a report downstream showing that old problem:
> > http://permalink.gmane.org/gmane.linux.bluez.kernel/22306
> > 
> > is still not fixed. 
> > 
> > Could you please commit debian patch to fix this?
> > http://patch-tracker.debian.org/patch/series/view/bluez/4.101-1/09_fix_ftbfs_with_c99.patch
> 
> Any news about this? Thanks

Bump :)

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] bluetooth.h: fix compile issue when using in C++

[Johan Hedberg]

Hi,

On Fri, Aug 17, 2012, W. Trevor King wrote:
> On Wed, Aug 01, 2012 at 04:41:33PM +0200, Pacho Ramos wrote:
> > El dom, 29-07-2012 a las 09:52 +0200, Pacho Ramos escribió:
> > > Today I got a report downstream showing that old problem:
> > > http://permalink.gmane.org/gmane.linux.bluez.kernel/22306
> > > 
> > > is still not fixed. 
> > > 
> > > Could you please commit debian patch to fix this?
> > > http://patch-tracker.debian.org/patch/series/view/bluez/4.101-1/09_fix_ftbfs_with_c99.patch
> > 
> > Any news about this? Thanks
> 
> Bump :)

The patch would need to be sent as a proper git patch to this mailing
list. Particularly, we need proper author information for the patch as
well as a commit message explaining the background to the patch and why
it is correct (including a reference to the C++ standard and/or gcc
documentation).

Johan

Re: [PATCH] bluetooth.h: fix compile issue when using in C++

[Pacho Ramos]

[-- Attachment #1: Type: text/plain, Size: 1112 bytes --]

El lun, 27-08-2012 a las 16:40 -0700, Johan Hedberg escribió:
> Hi,
> 
> On Fri, Aug 17, 2012, W. Trevor King wrote:
> > On Wed, Aug 01, 2012 at 04:41:33PM +0200, Pacho Ramos wrote:
> > > El dom, 29-07-2012 a las 09:52 +0200, Pacho Ramos escribió:
> > > > Today I got a report downstream showing that old problem:
> > > > http://permalink.gmane.org/gmane.linux.bluez.kernel/22306
> > > > 
> > > > is still not fixed. 
> > > > 
> > > > Could you please commit debian patch to fix this?
> > > > http://patch-tracker.debian.org/patch/series/view/bluez/4.101-1/09_fix_ftbfs_with_c99.patch
> > > 
> > > Any news about this? Thanks
> > 
> > Bump :)
> 
> The patch would need to be sent as a proper git patch to this mailing
> list. Particularly, we need proper author information for the patch as
> well as a commit message explaining the background to the patch and why
> it is correct (including a reference to the C++ standard and/or gcc
> documentation).
> 
> Johan
> 

Will forward this message to patch authors and debian maintainers to let
them reformat it with proper authoring.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH] bluetooth.h: fix compile issue when using in C++

[Pacho Ramos]

[-- Attachment #1: Type: text/plain, Size: 1325 bytes --]

El mié, 05-09-2012 a las 19:27 +0200, Pacho Ramos escribió:
> El lun, 27-08-2012 a las 16:40 -0700, Johan Hedberg escribió:
> > Hi,
> > 
> > On Fri, Aug 17, 2012, W. Trevor King wrote:
> > > On Wed, Aug 01, 2012 at 04:41:33PM +0200, Pacho Ramos wrote:
> > > > El dom, 29-07-2012 a las 09:52 +0200, Pacho Ramos escribió:
> > > > > Today I got a report downstream showing that old problem:
> > > > > http://permalink.gmane.org/gmane.linux.bluez.kernel/22306
> > > > > 
> > > > > is still not fixed. 
> > > > > 
> > > > > Could you please commit debian patch to fix this?
> > > > > http://patch-tracker.debian.org/patch/series/view/bluez/4.101-1/09_fix_ftbfs_with_c99.patch
> > > > 
> > > > Any news about this? Thanks
> > > 
> > > Bump :)
> > 
> > The patch would need to be sent as a proper git patch to this mailing
> > list. Particularly, we need proper author information for the patch as
> > well as a commit message explaining the background to the patch and why
> > it is correct (including a reference to the C++ standard and/or gcc
> > documentation).
> > 
> > Johan
> > 
> 
> Will forward this message to patch authors and debian maintainers to let
> them reformat it with proper authoring.

Umm, it could be a gcc bug per:
https://bugzilla.redhat.com/show_bug.cgi?id=786966#c5


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH] bluetooth.h: fix compile issue when using in C++

[Lucas De Marchi]

On Sun, Mar 4, 2012 at 8:27 PM, Marcel Holtmann <marcel@holtmann.org> wrote=
:
> Hi Patrick,
>
>> The compiler error is:
>> =C2=A0/usr/include/bluetooth/bluetooth.h::131:9:=E2=80=82error:=E2=80=82=
invalid=E2=80=82conversion from=E2=80=82'void*'=E2=80=82to=E2=80=82'bt_get_=
le64(void*)::<anonymous=E2=80=82struct>*'
>> =C2=A0...
>>
>> The reason is that C++, in contrast to C, does not allow conversion of
>> void * to anything, and this code gets compiled as C++ when the app is
>> written in C++. The macro with the assignment itself is older, but only
>> recent Bluez starts to use it in inline functions, thus triggering the
>> problem.
>>
>> This patch keeps the "struct __attribute__((packed))" magic and merely
>> changes the typecast so that it works in C and C++. Like the existing
>> macro this patch relies on support for typeof.
>
> so I am applying this patch now, but I still have no idea what kind of
> stupid C++ compiler you are using. The section of this include is
> clearly marked as C code:
>
> #ifdef __cplusplus
> extern "C" {
> #endif

This only tells the compiler that this part has C linkage and not C++


Lucas De Marchi

Re: [PATCH] bluetooth.h: fix compile issue when using in C++

[Markus Rathgeb]

Why can't we get rid off the "typeof" keyword completely and using a
template similar approach:

=3D=3D=3D

#define bt_get_unaligned_tmpl(type, ptr)         \
({                                               \
       struct __p_s  {                           \
               type __v;                         \
       } __attribute__((packed));                \
       struct __p_s *__p =3D (struct __p_s*)(ptr); \
       __p->__v;                                 \
})

static inline uint64_t bt_get_le64(void *ptr)
{
	return bt_get_unaligned_tmpl(uint64_t, ptr);
}

static inline uint64_t bt_get_be64(void *ptr)
{
	return bswap_64(bt_get_unaligned_tmpl(uint64_t, ptr));
}

=3D=3D=3D

Okay, the macro parameter type is not encapsulated by parenthesis, but
it seems okay and working.
So it seems to be a very more portable solution (IMHO).

2012/3/5 Lucas De Marchi <lucas.demarchi@profusion.mobi>:
> On Sun, Mar 4, 2012 at 8:27 PM, Marcel Holtmann <marcel@holtmann.org> wro=
te:
>> Hi Patrick,
>>
>>> The compiler error is:
>>> =C2=A0/usr/include/bluetooth/bluetooth.h::131:9:=E2=80=82error:=E2=80=
=82invalid=E2=80=82conversion from=E2=80=82'void*'=E2=80=82to=E2=80=82'bt_g=
et_le64(void*)::<anonymous=E2=80=82struct>*'
>>> =C2=A0...
>>>
>>> The reason is that C++, in contrast to C, does not allow conversion of
>>> void * to anything, and this code gets compiled as C++ when the app is
>>> written in C++. The macro with the assignment itself is older, but only
>>> recent Bluez starts to use it in inline functions, thus triggering the
>>> problem.
>>>
>>> This patch keeps the "struct __attribute__((packed))" magic and merely
>>> changes the typecast so that it works in C and C++. Like the existing
>>> macro this patch relies on support for typeof.
>>
>> so I am applying this patch now, but I still have no idea what kind of
>> stupid C++ compiler you are using. The section of this include is
>> clearly marked as C code:
>>
>> #ifdef __cplusplus
>> extern "C" {
>> #endif
>
> This only tells the compiler that this part has C linkage and not C++
>
>
> Lucas De Marchi
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth=
" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at =C2=A0http://vger.kernel.org/majordomo-info.html

Re: [PATCH] bluetooth.h: fix compile issue when using in C++

[Marcel Holtmann]

Hi Markus,

please do not top post on this mailing list.

> Why can't we get rid off the "typeof" keyword completely and using a
> template similar approach:
> 
> ===
> 
> #define bt_get_unaligned_tmpl(type, ptr)         \
> ({                                               \
>        struct __p_s  {                           \
>                type __v;                         \
>        } __attribute__((packed));                \
>        struct __p_s *__p = (struct __p_s*)(ptr); \
>        __p->__v;                                 \
> })
> 
> static inline uint64_t bt_get_le64(void *ptr)
> {
> 	return bt_get_unaligned_tmpl(uint64_t, ptr);
> }
> 
> static inline uint64_t bt_get_be64(void *ptr)
> {
> 	return bswap_64(bt_get_unaligned_tmpl(uint64_t, ptr));
> }
> 
> ===
> 
> Okay, the macro parameter type is not encapsulated by parenthesis, but
> it seems okay and working.
> So it seems to be a very more portable solution (IMHO).

what does seems okay and working actually mean? Have you actually
verified on platform that require correct unaligned access to data?

Regards

Marcel

Re: [PATCH] bluetooth.h: fix compile issue when using in C++

[Markus Rathgeb]

2012/3/8 Marcel Holtmann <marcel@holtmann.org>:
> Hi Markus,
>
> please do not top post on this mailing list.
>
>> Why can't we get rid off the "typeof" keyword completely and using a
>> template similar approach:
>>
>> =3D=3D=3D
>>
>> #define bt_get_unaligned_tmpl(type, ptr) =C2=A0 =C2=A0 =C2=A0 =C2=A0 \
>> ({ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0=
 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 \
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0struct __p_s =C2=A0{ =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 \
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0type __v; =C2=A0 =
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 \
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0} __attribute__((packed)); =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0\
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0struct __p_s *__p =3D (struct __p_s*)(ptr); \
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0__p->__v; =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 \
>> })
>>
>> static inline uint64_t bt_get_le64(void *ptr)
>> {
>> =C2=A0 =C2=A0 =C2=A0 return bt_get_unaligned_tmpl(uint64_t, ptr);
>> }
>>
>> static inline uint64_t bt_get_be64(void *ptr)
>> {
>> =C2=A0 =C2=A0 =C2=A0 return bswap_64(bt_get_unaligned_tmpl(uint64_t, ptr=
));
>> }
>>
>> =3D=3D=3D
>>
>> Okay, the macro parameter type is not encapsulated by parenthesis, but
>> it seems okay and working.
>> So it seems to be a very more portable solution (IMHO).
>
> what does seems okay and working actually mean? Have you actually
> verified on platform that require correct unaligned access to data?
>
> Regards
>
> Marcel
>
>

Hi!

Sorry for the top posting. I have forgotten, to not do that.

I could check that on an ARM architecture, that needs correct
unaligned access handling.

What did I mean with "seems okay"?
You could write code and test it if it is correct, if it fails, it is
not correct, if it not fails, it could be correct.
Or you can write code, think about what the code is doing, and say,
this should be correct.
Sorry, this should not be arrogant, just explain, that I see no
different in the code (if I compare both).

I will write a test suite after the weekend.
But perhaps another one could have a look at.

Re: [PATCH] bluetooth.h: fix compile issue when using in C++

[Markus Rathgeb]

2012/3/8 Markus Rathgeb <maggu2810@googlemail.com>:
> 2012/3/8 Marcel Holtmann <marcel@holtmann.org>:
>> Hi Markus,
>>
>> please do not top post on this mailing list.
>>
>>> Why can't we get rid off the "typeof" keyword completely and using a
>>> template similar approach:
>>>
>>> =3D=3D=3D
>>>
>>> #define bt_get_unaligned_tmpl(type, ptr) =C2=A0 =C2=A0 =C2=A0 =C2=A0 \
>>> ({ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0 =C2=A0 \
>>> =C2=A0 =C2=A0 =C2=A0 =C2=A0struct __p_s =C2=A0{ =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 \
>>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0type __v; =C2=A0=
 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 \
>>> =C2=A0 =C2=A0 =C2=A0 =C2=A0} __attribute__((packed)); =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0\
>>> =C2=A0 =C2=A0 =C2=A0 =C2=A0struct __p_s *__p =3D (struct __p_s*)(ptr); =
\
>>> =C2=A0 =C2=A0 =C2=A0 =C2=A0__p->__v; =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0=
 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 \
>>> })
>>>
>>> static inline uint64_t bt_get_le64(void *ptr)
>>> {
>>> =C2=A0 =C2=A0 =C2=A0 return bt_get_unaligned_tmpl(uint64_t, ptr);
>>> }
>>>
>>> static inline uint64_t bt_get_be64(void *ptr)
>>> {
>>> =C2=A0 =C2=A0 =C2=A0 return bswap_64(bt_get_unaligned_tmpl(uint64_t, pt=
r));
>>> }
>>>
>>> =3D=3D=3D
>>>
>>> Okay, the macro parameter type is not encapsulated by parenthesis, but
>>> it seems okay and working.
>>> So it seems to be a very more portable solution (IMHO).
>>
>> what does seems okay and working actually mean? Have you actually
>> verified on platform that require correct unaligned access to data?
>>
>> Regards
>>
>> Marcel
>>
>>
>
> Hi!
>
> Sorry for the top posting. I have forgotten, to not do that.
>
> I could check that on an ARM architecture, that needs correct
> unaligned access handling.
>
> What did I mean with "seems okay"?
> You could write code and test it if it is correct, if it fails, it is
> not correct, if it not fails, it could be correct.
> Or you can write code, think about what the code is doing, and say,
> this should be correct.
> Sorry, this should not be arrogant, just explain, that I see no
> different in the code (if I compare both).
>
> I will write a test suite after the weekend.
> But perhaps another one could have a look at.

#include <stdint.h>
#include <stdio.h>

#define DO_UNALIGNED 1

uint64_t bt_get_le64_old(void *ptr)
{
 return (
            {
                struct __attribute__((packed))
                {
                    typeof(*((uint64_t *) ptr)) __v;
                } *__p =3D (void *) ((uint64_t *) ptr);
                __p->__v;
            }
        );
}

uint64_t bt_get_le64_new(void *ptr)
{
 return (
            {
                struct __p_s
                {
                    uint64_t __v;
                } __attribute__((packed));
                struct __p_s *__p =3D (struct __p_s*)(ptr);
                __p->__v;
            }
        );
}


int main(int argc, char* argv[])
{
    int i, j;
    uint8_t data[2 * sizeof(uint64_t)];
    void* ptr;
    uint64_t result_old, result_new;

    for (i =3D 0; i < sizeof(data); ++i)
        data[i] =3D i;

    for (i =3D 0; i < sizeof(data) - sizeof(uint64_t); ++i)
    {
        ptr =3D data + i;

        printf("as is: ");

#if __BYTE_ORDER =3D=3D __LITTLE_ENDIAN
        for (j =3D sizeof(uint64_t) - 1; j >=3D 0; --j)
#else
        for (j =3D 0; j < sizeof(uint64_t); ++j)
#endif
        {
            printf("%02X", *(data + i + j));
        }
        printf("\n");

#if DO_UNALIGNED
        printf("unali: %016llX\n", *((uint64_t*)ptr));
#endif

        result_old =3D bt_get_le64_old(ptr);
        result_new =3D bt_get_le64_new(ptr);
        printf("  old: %016llX\n  new: %016llX\n---\n", result_old, result_=
new);
    }

    return 0;
}

=3D=3D=3D

Running on
Processor       : ARM926EJ-S rev 5 (v5l)

Let's ignore alignment errors:
echo 0 > /proc/cpu/alignment

=3D=3D=3D

# ./unaligned_arm
as is: 0706050403020100
unali: 0706050403020100
  old: 0706050403020100
  new: 0706050403020100
---
as is: 0807060504030201
unali: 0706050403020100
  old: 0807060504030201
  new: 0807060504030201
---
as is: 0908070605040302
unali: 0706050403020100
  old: 0908070605040302
  new: 0908070605040302
---
as is: 0A09080706050403
unali: 0706050403020100
  old: 0A09080706050403
  new: 0A09080706050403
---
as is: 0B0A090807060504
unali: 0B0A090807060504
  old: 0B0A090807060504
  new: 0B0A090807060504
---
as is: 0C0B0A0908070605
unali: 0B0A090807060504
  old: 0C0B0A0908070605
  new: 0C0B0A0908070605
---
as is: 0D0C0B0A09080706
unali: 0B0A090807060504
  old: 0D0C0B0A09080706
  new: 0D0C0B0A09080706
---
as is: 0E0D0C0B0A090807
unali: 0B0A090807060504
  old: 0E0D0C0B0A090807
  new: 0E0D0C0B0A090807
---

=3D=3D=3D

I could also run a loop and break if result_new !=3D result_old...

Re: [PATCH] bluetooth.h: fix compile issue when using in C++

[Markus Rathgeb]

Hello!

The patch "http://www.spinics.net/lists/linux-bluetooth/msg20220.html"
is using the typeof keyword.
I also found the statement:
"I was uncomfortable to depend on "typeof" for that initially, but the
macro already uses that for the struct member."

IMHO typeof is not part of the C++ standard, and isn't available when
we use -std=c++0x.
g++ should support __typeof__ even with -std=c++0x.

So perhaps we could change the C++ "workaround" to use __typeof__
instead of typeof to support also programs that are using c++0x
standard.

bluetooth.h: fix compile issue when using in C++

[Pacho Ramos]

[-- Attachment #1: Type: text/plain, Size: 314 bytes --]

Hello

Today I got a report downstream showing that old problem:
http://permalink.gmane.org/gmane.linux.bluez.kernel/22306

is still not fixed. 

Could you please commit debian patch to fix this?
http://patch-tracker.debian.org/patch/series/view/bluez/4.101-1/09_fix_ftbfs_with_c99.patch

Thanks  a lot

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: bluetooth.h: fix compile issue when using in C++

[Pacho Ramos]

[-- Attachment #1: Type: text/plain, Size: 429 bytes --]

El dom, 29-07-2012 a las 09:52 +0200, Pacho Ramos escribió:
> Hello
> 
> Today I got a report downstream showing that old problem:
> http://permalink.gmane.org/gmane.linux.bluez.kernel/22306
> 
> is still not fixed. 
> 
> Could you please commit debian patch to fix this?
> http://patch-tracker.debian.org/patch/series/view/bluez/4.101-1/09_fix_ftbfs_with_c99.patch
> 
> Thanks  a lot

Any news about this? Thanks

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]