All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] bluetooth.h: fix compile issue when using in C++
@ 2012-01-16 10:11 Patrick Ohly
  2012-03-04 23:27 ` Marcel Holtmann
  0 siblings, 1 reply; 17+ messages in thread
From: Patrick Ohly @ 2012-01-16 10:11 UTC (permalink / raw)
  To: linux-bluetooth

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



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH] bluetooth.h: fix compile issue when using in C++
  2012-01-16 10:11 [PATCH] " Patrick Ohly
@ 2012-03-04 23:27 ` Marcel Holtmann
  2012-03-05  7:33   ` Patrick Ohly
  2012-03-05 20:06   ` Lucas De Marchi
  0 siblings, 2 replies; 17+ messages in thread
From: Marcel Holtmann @ 2012-03-04 23:27 UTC (permalink / raw)
  To: Patrick Ohly; +Cc: linux-bluetooth

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



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] bluetooth.h: fix compile issue when using in C++
  2012-03-04 23:27 ` Marcel Holtmann
@ 2012-03-05  7:33   ` Patrick Ohly
       [not found]     ` <CAOcK=CNZofxZtUp4Zh5eiGMM_TYrbGOwseA3tcwAnAFDvLEFxg@mail.gmail.com>
  2012-03-05 20:06   ` Lucas De Marchi
  1 sibling, 1 reply; 17+ messages in thread
From: Patrick Ohly @ 2012-03-05  7:33 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: linux-bluetooth

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.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] bluetooth.h: fix compile issue when using in C++
       [not found]     ` <CAOcK=CNZofxZtUp4Zh5eiGMM_TYrbGOwseA3tcwAnAFDvLEFxg@mail.gmail.com>
@ 2012-03-05 13:12       ` Patrick Ohly
  2012-03-05 14:53         ` Markus Rathgeb
                           ` (2 more replies)
  0 siblings, 3 replies; 17+ messages in thread
From: Patrick Ohly @ 2012-03-05 13:12 UTC (permalink / raw)
  To: Markus Rathgeb; +Cc: Marcel Holtmann, linux-bluetooth

[-- 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


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH] bluetooth.h: fix compile issue when using in C++
  2012-03-05 13:12       ` Patrick Ohly
@ 2012-03-05 14:53         ` Markus Rathgeb
  2012-07-27 16:38         ` W. Trevor King
  2012-08-17  5:18         ` W. Trevor King
  2 siblings, 0 replies; 17+ messages in thread
From: Markus Rathgeb @ 2012-03-05 14:53 UTC (permalink / raw)
  To: Patrick Ohly; +Cc: Marcel Holtmann, linux-bluetooth

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] bluetooth.h: fix compile issue when using in C++
  2012-03-04 23:27 ` Marcel Holtmann
  2012-03-05  7:33   ` Patrick Ohly
@ 2012-03-05 20:06   ` Lucas De Marchi
  2012-03-08 13:22     ` Markus Rathgeb
  1 sibling, 1 reply; 17+ messages in thread
From: Lucas De Marchi @ 2012-03-05 20:06 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: Patrick Ohly, linux-bluetooth

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] bluetooth.h: fix compile issue when using in C++
  2012-03-05 20:06   ` Lucas De Marchi
@ 2012-03-08 13:22     ` Markus Rathgeb
  2012-03-08 17:51       ` Marcel Holtmann
  0 siblings, 1 reply; 17+ messages in thread
From: Markus Rathgeb @ 2012-03-08 13:22 UTC (permalink / raw)
  To: Lucas De Marchi; +Cc: Marcel Holtmann, Patrick Ohly, linux-bluetooth

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] bluetooth.h: fix compile issue when using in C++
  2012-03-08 13:22     ` Markus Rathgeb
@ 2012-03-08 17:51       ` Marcel Holtmann
  2012-03-08 20:59         ` Markus Rathgeb
  0 siblings, 1 reply; 17+ messages in thread
From: Marcel Holtmann @ 2012-03-08 17:51 UTC (permalink / raw)
  To: Markus Rathgeb; +Cc: Lucas De Marchi, Patrick Ohly, linux-bluetooth

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



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] bluetooth.h: fix compile issue when using in C++
  2012-03-08 17:51       ` Marcel Holtmann
@ 2012-03-08 20:59         ` Markus Rathgeb
  2012-03-09 10:52           ` Markus Rathgeb
  0 siblings, 1 reply; 17+ messages in thread
From: Markus Rathgeb @ 2012-03-08 20:59 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: Lucas De Marchi, Patrick Ohly, linux-bluetooth

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.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] bluetooth.h: fix compile issue when using in C++
  2012-03-08 20:59         ` Markus Rathgeb
@ 2012-03-09 10:52           ` Markus Rathgeb
  0 siblings, 0 replies; 17+ messages in thread
From: Markus Rathgeb @ 2012-03-09 10:52 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: Lucas De Marchi, Patrick Ohly, linux-bluetooth

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...

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] bluetooth.h: fix compile issue when using in C++
  2012-03-05 13:12       ` Patrick Ohly
  2012-03-05 14:53         ` Markus Rathgeb
@ 2012-07-27 16:38         ` W. Trevor King
  2012-08-17  5:18         ` W. Trevor King
  2 siblings, 0 replies; 17+ messages in thread
From: W. Trevor King @ 2012-07-27 16:38 UTC (permalink / raw)
  To: linux-bluetooth

[-- 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 --]

^ permalink raw reply	[flat|nested] 17+ messages in thread

* bluetooth.h: fix compile issue when using in C++
@ 2012-07-29  7:52 Pacho Ramos
  2012-08-01 14:41 ` Pacho Ramos
  0 siblings, 1 reply; 17+ messages in thread
From: Pacho Ramos @ 2012-07-29  7:52 UTC (permalink / raw)
  To: BlueZ development

[-- 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 --]

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: bluetooth.h: fix compile issue when using in C++
  2012-07-29  7:52 bluetooth.h: fix compile issue when using in C++ Pacho Ramos
@ 2012-08-01 14:41 ` Pacho Ramos
  0 siblings, 0 replies; 17+ messages in thread
From: Pacho Ramos @ 2012-08-01 14:41 UTC (permalink / raw)
  To: BlueZ development

[-- 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 --]

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] bluetooth.h: fix compile issue when using in C++
  2012-03-05 13:12       ` Patrick Ohly
  2012-03-05 14:53         ` Markus Rathgeb
  2012-07-27 16:38         ` W. Trevor King
@ 2012-08-17  5:18         ` W. Trevor King
  2012-08-27 23:40           ` Johan Hedberg
  2 siblings, 1 reply; 17+ messages in thread
From: W. Trevor King @ 2012-08-17  5:18 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Patrick Ohly, Pacho Ramos

[-- 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 --]

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] bluetooth.h: fix compile issue when using in C++
  2012-08-17  5:18         ` W. Trevor King
@ 2012-08-27 23:40           ` Johan Hedberg
  2012-09-05 17:27             ` Pacho Ramos
  0 siblings, 1 reply; 17+ messages in thread
From: Johan Hedberg @ 2012-08-27 23:40 UTC (permalink / raw)
  To: W. Trevor King; +Cc: linux-bluetooth, Patrick Ohly, Pacho Ramos

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] bluetooth.h: fix compile issue when using in C++
  2012-08-27 23:40           ` Johan Hedberg
@ 2012-09-05 17:27             ` Pacho Ramos
  2012-09-05 17:56               ` Pacho Ramos
  0 siblings, 1 reply; 17+ messages in thread
From: Pacho Ramos @ 2012-09-05 17:27 UTC (permalink / raw)
  To: Johan Hedberg, iwamatsu, pkg-bluetooth-maintainers
  Cc: W. Trevor King, linux-bluetooth, Patrick Ohly

[-- 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 --]

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] bluetooth.h: fix compile issue when using in C++
  2012-09-05 17:27             ` Pacho Ramos
@ 2012-09-05 17:56               ` Pacho Ramos
  0 siblings, 0 replies; 17+ messages in thread
From: Pacho Ramos @ 2012-09-05 17:56 UTC (permalink / raw)
  To: Johan Hedberg
  Cc: iwamatsu, pkg-bluetooth-maintainers, W. Trevor King,
	linux-bluetooth, Patrick Ohly

[-- 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 --]

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2012-09-05 17:56 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-07-29  7:52 bluetooth.h: fix compile issue when using in C++ Pacho Ramos
2012-08-01 14:41 ` Pacho Ramos
  -- strict thread matches above, loose matches on Subject: below --
2012-01-16 10:11 [PATCH] " Patrick Ohly
2012-03-04 23:27 ` Marcel Holtmann
2012-03-05  7:33   ` Patrick Ohly
     [not found]     ` <CAOcK=CNZofxZtUp4Zh5eiGMM_TYrbGOwseA3tcwAnAFDvLEFxg@mail.gmail.com>
2012-03-05 13:12       ` Patrick Ohly
2012-03-05 14:53         ` Markus Rathgeb
2012-07-27 16:38         ` W. Trevor King
2012-08-17  5:18         ` W. Trevor King
2012-08-27 23:40           ` Johan Hedberg
2012-09-05 17:27             ` Pacho Ramos
2012-09-05 17:56               ` Pacho Ramos
2012-03-05 20:06   ` Lucas De Marchi
2012-03-08 13:22     ` Markus Rathgeb
2012-03-08 17:51       ` Marcel Holtmann
2012-03-08 20:59         ` Markus Rathgeb
2012-03-09 10:52           ` Markus Rathgeb

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.