bluez 4.97: build failure when used in C++ apps

bluez 4.97: build failure when used in C++ apps

[Patrick Ohly]

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

Hello!

Bluez 4.97 has entered some distros recently (Fedora Core, MeeGo 1.2
update), which broke compilation of the C++ code in SyncEvolution
using /usr/lib/bluetooth.h.

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.

Attached is a proposed solution. Note that I have only verified that it
now compiles in C and C++, I have not actually tested the resulting
Bluez, because compilation fails for me on Debian Testing for another
reason (libcheck.a linked into shared object, doesn't work because not
relocatable).

-- 
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: bluez-4.97-cplusplus.patch --]
[-- Type: text/x-patch, Size: 819 bytes --]

*** bluez-4.97/lib/bluetooth.h.orig	2012-01-16 09:06:07.171931288 +0100
--- bluez-4.97/lib/bluetooth.h	2012-01-16 09:07:02.523930152 +0100
***************
*** 113,119 ****
  ({						\
  	struct __attribute__((packed)) {	\
  		typeof(*(ptr)) __v;		\
! 	} *__p = (void *) (ptr);		\
  	__p->__v;				\
  })
  
--- 113,119 ----
  ({						\
  	struct __attribute__((packed)) {	\
  		typeof(*(ptr)) __v;		\
! 	} *__p = (typeof(__p)) (ptr);		\
  	__p->__v;				\
  })
  
***************
*** 121,127 ****
  do {						\
  	struct __attribute__((packed)) {	\
  		typeof(*(ptr)) __v;		\
! 	} *__p = (void *) (ptr);		\
  	__p->__v = (val);			\
  } while(0)
  
--- 121,127 ----
  do {						\
  	struct __attribute__((packed)) {	\
  		typeof(*(ptr)) __v;		\
! 	} *__p = (typeof(__p)) (ptr);		\
  	__p->__v = (val);			\
  } while(0)
  

RE: bluez 4.97: build failure when used in C++ apps

[Deng, Ying An]

DQo+LS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj5Gcm9tOiBQYXRyaWNrIE9obHkgW21haWx0
bzpwYXRyaWNrLm9obHlAaW50ZWwuY29tXQ0KPlNlbnQ6IE1vbmRheSwgSmFudWFyeSAxNiwgMjAx
MiA0OjI2IFBNDQo+VG86IGxpbnV4LWJsdWV0b290aEB2Z2VyLmtlcm5lbC5vcmcNCj5DYzogTWls
YW4gQ3JoYTsgRGVuZywgWWluZyBBbjsgSG9mZW1laWVyLCBVbGY7IFdhbmcsIE5pbmcgVw0KPlN1
YmplY3Q6IGJsdWV6IDQuOTc6IGJ1aWxkIGZhaWx1cmUgd2hlbiB1c2VkIGluIEMrKyBhcHBzDQo+
DQo+SGVsbG8hDQo+DQo+Qmx1ZXogNC45NyBoYXMgZW50ZXJlZCBzb21lIGRpc3Ryb3MgcmVjZW50
bHkgKEZlZG9yYSBDb3JlLCBNZWVHbyAxLjIgdXBkYXRlKSwNCj53aGljaCBicm9rZSBjb21waWxh
dGlvbiBvZiB0aGUgQysrIGNvZGUgaW4gU3luY0V2b2x1dGlvbiB1c2luZw0KPi91c3IvbGliL2Js
dWV0b290aC5oLg0KPg0KPlRoZSBjb21waWxlciBlcnJvciBpczoNCj4vdXNyL2luY2x1ZGUvYmx1
ZXRvb3RoL2JsdWV0b290aC5oOjoxMzE6OTrigIJlcnJvcjrigIJpbnZhbGlk4oCCY29udmVyc2lv
biBmcm9t4oCCDQo+J3ZvaWQqJ+KAgnRv4oCCJ2J0X2dldF9sZTY0KHZvaWQqKTo6PGFub255bW91
c+KAgnN0cnVjdD4qJw0KPi4uLg0KPg0KPlRoZSByZWFzb24gaXMgdGhhdCBDKyssIGluIGNvbnRy
YXN0IHRvIEMsIGRvZXMgbm90IGFsbG93IGNvbnZlcnNpb24gb2Ygdm9pZCAqIHRvDQo+YW55dGhp
bmcsIGFuZCB0aGlzIGNvZGUgZ2V0cyBjb21waWxlZCBhcyBDKysgd2hlbiB0aGUgYXBwIGlzIHdy
aXR0ZW4gaW4gQysrLiBUaGUNCj5tYWNybyB3aXRoIHRoZSBhc3NpZ25tZW50IGl0c2VsZiBpcyBv
bGRlciwgYnV0IG9ubHkgcmVjZW50IEJsdWV6IHN0YXJ0cyB0byB1c2UgaXQgaW4NCj5pbmxpbmUg
ZnVuY3Rpb25zLCB0aHVzIHRyaWdnZXJpbmcgdGhlIHByb2JsZW0uDQo+DQo+QXR0YWNoZWQgaXMg
YSBwcm9wb3NlZCBzb2x1dGlvbi4gTm90ZSB0aGF0IEkgaGF2ZSBvbmx5IHZlcmlmaWVkIHRoYXQg
aXQgbm93DQo+Y29tcGlsZXMgaW4gQyBhbmQgQysrLCBJIGhhdmUgbm90IGFjdHVhbGx5IHRlc3Rl
ZCB0aGUgcmVzdWx0aW5nIEJsdWV6LCBiZWNhdXNlDQo+Y29tcGlsYXRpb24gZmFpbHMgZm9yIG1l
IG9uIERlYmlhbiBUZXN0aW5nIGZvciBhbm90aGVyIHJlYXNvbiAobGliY2hlY2suYSBsaW5rZWQN
Cj5pbnRvIHNoYXJlZCBvYmplY3QsIGRvZXNuJ3Qgd29yayBiZWNhdXNlIG5vdCByZWxvY2F0YWJs
ZSkuDQo+DQoNClZlcmlmaWVkIGluIEJsdWV6LCBhbmQgaXQgY29tcGlsZXMgc3VjY2Vzc2Z1bGx5
Lg0KVGhhbmtzLg0KDQo+LS0NCj5CZXN0IFJlZ2FyZHMsIFBhdHJpY2sgT2hseQ0KPg0KPlRoZSBj
b250ZW50IG9mIHRoaXMgbWVzc2FnZSBpcyBteSBwZXJzb25hbCBvcGluaW9uIG9ubHkgYW5kIGFs
dGhvdWdoIEkgYW0gYW4NCj5lbXBsb3llZSBvZiBJbnRlbCwgdGhlIHN0YXRlbWVudHMgSSBtYWtl
IGhlcmUgaW4gbm8gd2F5IHJlcHJlc2VudCBJbnRlbCdzDQo+cG9zaXRpb24gb24gdGhlIGlzc3Vl
LCBub3IgYW0gSSBhdXRob3JpemVkIHRvIHNwZWFrIG9uIGJlaGFsZiBvZiBJbnRlbCBvbiB0aGlz
DQo+bWF0dGVyLg0KDQo=

Re: bluez 4.97: build failure when used in C++ apps

[Marcel Holtmann]

Hi Patrick,

> Bluez 4.97 has entered some distros recently (Fedora Core, MeeGo 1.2
> update), which broke compilation of the C++ code in SyncEvolution
> using /usr/lib/bluetooth.h.
> 
> 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.
> 
> Attached is a proposed solution. Note that I have only verified that it
> now compiles in C and C++, I have not actually tested the resulting
> Bluez, because compilation fails for me on Debian Testing for another
> reason (libcheck.a linked into shared object, doesn't work because not
> relocatable).

please can we get inline patches from git format-patch. Also this topic
has been raised before and I need confirmation for a GCC guru to confirm
that this does exactly the same all all platforms.

Tested this on x86 hardware is useless since it does unaligned access
properly anyway. ARM, MIPS and PowerPC would be interesting.

Regards

Marcel

Re: bluez 4.97: build failure when used in C++ apps

[Patrick Ohly]

On Mo, 2012-01-16 at 10:24 +0100, Marcel Holtmann wrote:
> Hi Patrick,
> 
> > Bluez 4.97 has entered some distros recently (Fedora Core, MeeGo 1.2
> > update), which broke compilation of the C++ code in SyncEvolution
> > using /usr/lib/bluetooth.h.
> > 
> > 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.
> > 
> > Attached is a proposed solution. Note that I have only verified that it
> > now compiles in C and C++, I have not actually tested the resulting
> > Bluez, because compilation fails for me on Debian Testing for another
> > reason (libcheck.a linked into shared object, doesn't work because not
> > relocatable).
> 
> please can we get inline patches from git format-patch.

Will do. 

> Also this topic
> has been raised before and I need confirmation for a GCC guru to confirm
> that this does exactly the same all all platforms.

Now that you mentioned it I found the previous patch:
http://article.gmane.org/gmane.linux.bluez.kernel/20276/match=invalid
+conversion+void+bt_get_le64+anonymous+struct

Note that my patch is different. It keeps the "struct
__attribute__((packed))" magic and merely changes the (void *) typecast
to something that works in C++ and C. I was uncomfortable to depend on
"typeof" for that initially, but the macro already uses that for the
struct member.

In the worst case it'll fail to compile with some (hypothetical) version
of gcc which supports typeof for the old case but not the new one. IMHO
that's less problematic than the known failure that we have now - but of
course I am biased. I'd like to keep SyncEvolution working with Bluez
without rewriting it in C++ ;-)

-- 
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: bluez 4.97: build failure when used in C++ apps

[Patrick Ohly]

On Mon, 2012-01-16 at 11:09 +0100, Patrick Ohly wrote:
> On Mo, 2012-01-16 at 10:24 +0100, Marcel Holtmann wrote:
> > Also this topic
> > has been raised before and I need confirmation for a GCC guru to confirm
> > that this does exactly the same all all platforms.
> 
> Now that you mentioned it I found the previous patch:
> http://article.gmane.org/gmane.linux.bluez.kernel/20276/match=invalid
> +conversion+void+bt_get_le64+anonymous+struct
> 
> Note that my patch is different. It keeps the "struct
> __attribute__((packed))" magic and merely changes the (void *) typecast
> to something that works in C++ and C.

How do we move forward with this? More and more distros are picking up
the broken header file.

In MeeGo and Tizen we applied the patch that I had suggested.

Milan, do you know what Fedora is doing? Tino, what about Debian? Roran,
you had it in Ubuntu Precise Pangolin. Care to file a bug there, if
there isn't one already?

Would it be more acceptable for upstream to put the modified macros into
an ifdef so that they are really only used in C++, keeping the code for
C as it is now?

I'll send such a patch immediately.

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

[PATCH] bluetooth.h: fix compile issue in C++ with ifdef

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

The new variant of the code is in an ifdef and only used for C++
to avoid unexpected regressions in C applications.

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

diff --git a/lib/bluetooth.h b/lib/bluetooth.h
index 5bd4f03..3293915 100644
--- a/lib/bluetooth.h
+++ b/lib/bluetooth.h
@@ -109,6 +109,11 @@ enum {
 #endif
 
 /* Bluetooth unaligned access */
+#ifndef __cplusplus
+/*
+ * traditional code, doesn't work in C++ because
+ * of the void * to struct pointer assignment
+ */
 #define bt_get_unaligned(ptr)			\
 ({						\
 	struct __attribute__((packed)) {	\
@@ -125,6 +130,31 @@ do {						\
 	__p->__v = (val);			\
 } while(0)
 
+#else /* __cplusplus */
+
+/*
+ * modified code with typeof typecast, for C++;
+ * the traditional code continues to be used for
+ * C to avoid unexpected regressions with this
+ * code here (it should work in C and C++, though)
+ */
+#define bt_get_unaligned(ptr)			\
+({						\
+	struct __attribute__((packed)) {	\
+		typeof(*(ptr)) __v;		\
+	} *__p = (typeof(__p)) (ptr);		\
+	__p->__v;				\
+})
+
+#define bt_put_unaligned(val, ptr)		\
+do {						\
+	struct __attribute__((packed)) {	\
+		typeof(*(ptr)) __v;		\
+	} *__p = (typeof(__p)) (ptr);		\
+	__p->__v = (val);			\
+} while(0)
+#endif /* __cplusplus */
+
 #if __BYTE_ORDER == __LITTLE_ENDIAN
 static inline uint64_t bt_get_le64(void *ptr)
 {
-- 
1.7.8.3

Re: bluez 4.97: build failure when used in C++ apps

[Rohan Garg]

I can file a bug for tracking purposes and get the patched package in
the archives, as long as upstream is willing to accept the patch ( or
a modified version of the patch ) in the near future. I don't speak
for the rest of the Ubuntu developers, but I consider the best
approach to getting issues fixed in distros.

Best
Rohan Garg

On Sun, Feb 5, 2012 at 4:26 PM, Patrick Ohly <patrick.ohly@intel.com> wrote:
> On Mon, 2012-01-16 at 11:09 +0100, Patrick Ohly wrote:
>> On Mo, 2012-01-16 at 10:24 +0100, Marcel Holtmann wrote:
>> > Also this topic
>> > has been raised before and I need confirmation for a GCC guru to confirm
>> > that this does exactly the same all all platforms.
>>
>> Now that you mentioned it I found the previous patch:
>> http://article.gmane.org/gmane.linux.bluez.kernel/20276/match=invalid
>> +conversion+void+bt_get_le64+anonymous+struct
>>
>> Note that my patch is different. It keeps the "struct
>> __attribute__((packed))" magic and merely changes the (void *) typecast
>> to something that works in C++ and C.
>
> How do we move forward with this? More and more distros are picking up
> the broken header file.
>
> In MeeGo and Tizen we applied the patch that I had suggested.
>
> Milan, do you know what Fedora is doing? Tino, what about Debian? Roran,
> you had it in Ubuntu Precise Pangolin. Care to file a bug there, if
> there isn't one already?
>
> Would it be more acceptable for upstream to put the modified macros into
> an ifdef so that they are really only used in C++, keeping the code for
> C as it is now?
>
> I'll send such a patch immediately.
>
> --
> 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 in C++ with ifdef

[Rohan Garg]

I've not gone through the entire patch, but if the problem is merely
the fact that one cannot convert a void* to anything in C++, why not
use a static_cast in syncevolution wherever the struct is used?

Best
Rohan Garg



On Sun, Feb 5, 2012 at 4:27 PM, Patrick Ohly <patrick.ohly@intel.com> wrote=
:
> The compiler error is:
> =C2=A0/usr/include/bluetooth/bluetooth.h::131:9:=E2=80=82error:=E2=80=82i=
nvalid=E2=80=82conversion from=E2=80=82'void*'=E2=80=82to=E2=80=82'bt_get_l=
e64(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.
>
> The new variant of the code is in an ifdef and only used for C++
> to avoid unexpected regressions in C applications.
>
> Signed-off-by: Patrick Ohly <patrick.ohly@intel.com>
> ---
> =C2=A0lib/bluetooth.h | =C2=A0 30 ++++++++++++++++++++++++++++++
> =C2=A01 files changed, 30 insertions(+), 0 deletions(-)
>
> diff --git a/lib/bluetooth.h b/lib/bluetooth.h
> index 5bd4f03..3293915 100644
> --- a/lib/bluetooth.h
> +++ b/lib/bluetooth.h
> @@ -109,6 +109,11 @@ enum {
> =C2=A0#endif
>
> =C2=A0/* Bluetooth unaligned access */
> +#ifndef __cplusplus
> +/*
> + * traditional code, doesn't work in C++ because
> + * of the void * to struct pointer assignment
> + */
> =C2=A0#define bt_get_unaligned(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=A0 =C2=A0 \
> =C2=A0 =C2=A0 =C2=A0 =C2=A0struct __attribute__((packed)) { =C2=A0 =C2=A0=
 =C2=A0 =C2=A0\
> @@ -125,6 +130,31 @@ do { =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__p->__v =3D (val); =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 \
> =C2=A0} while(0)
>
> +#else /* __cplusplus */
> +
> +/*
> + * modified code with typeof typecast, for C++;
> + * the traditional code continues to be used for
> + * C to avoid unexpected regressions with this
> + * code here (it should work in C and C++, though)
> + */
> +#define bt_get_unaligned(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=A0 \
> + =C2=A0 =C2=A0 =C2=A0 struct __attribute__((packed)) { =C2=A0 =C2=A0 =C2=
=A0 =C2=A0\
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 typeof(*(ptr)) __v; =
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 \
> + =C2=A0 =C2=A0 =C2=A0 } *__p =3D (typeof(__p)) (ptr); =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =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 \
> +})
> +
> +#define bt_put_unaligned(val, ptr) =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 \
> +do { =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 struct __attribute__((packed)) { =C2=A0 =C2=A0 =C2=
=A0 =C2=A0\
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 typeof(*(ptr)) __v; =
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 \
> + =C2=A0 =C2=A0 =C2=A0 } *__p =3D (typeof(__p)) (ptr); =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 \
> + =C2=A0 =C2=A0 =C2=A0 __p->__v =3D (val); =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 \
> +} while(0)
> +#endif /* __cplusplus */
> +
> =C2=A0#if __BYTE_ORDER =3D=3D __LITTLE_ENDIAN
> =C2=A0static inline uint64_t bt_get_le64(void *ptr)
> =C2=A0{
> --
> 1.7.8.3
>
>
>

Re: [PATCH] bluetooth.h: fix compile issue in C++ with ifdef

[Patrick Ohly]

On Sun, 2012-02-05 at 16:37 +0530, Rohan Garg wrote:
> I've not gone through the entire patch, but if the problem is merely
> the fact that one cannot convert a void* to anything in C++, why not
> use a static_cast in syncevolution wherever the struct is used?

The problem is not in the way how SyncEvolution uses the structs. The
problem is in the header files; it also occurs in any other C++ app
including the bluetooth.h file.

-- 
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 in C++ with ifdef

[Rohan Garg]

Ah, understood :)

Best
Rohan Garg



On Sun, Feb 5, 2012 at 5:03 PM, Patrick Ohly <patrick.ohly@intel.com> wrote:
> On Sun, 2012-02-05 at 16:37 +0530, Rohan Garg wrote:
>> I've not gone through the entire patch, but if the problem is merely
>> the fact that one cannot convert a void* to anything in C++, why not
>> use a static_cast in syncevolution wherever the struct is used?
>
> The problem is not in the way how SyncEvolution uses the structs. The
> problem is in the header files; it also occurs in any other C++ app
> including the bluetooth.h file.
>
> --
> 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: bluez 4.97: build failure when used in C++ apps

[Rohan Garg]

Bug filed in Launchpad :
https://bugs.launchpad.net/ubuntu/+source/bluez/+bug/927097

Regards
Rohan Garg



On Sun, Feb 5, 2012 at 4:29 PM, Rohan Garg <rohangarg@ubuntu.com> wrote:
> I can file a bug for tracking purposes and get the patched package in
> the archives, as long as upstream is willing to accept the patch ( or
> a modified version of the patch ) in the near future. I don't speak
> for the rest of the Ubuntu developers, but I consider the best
> approach to getting issues fixed in distros.
>
> Best
> Rohan Garg
>
> On Sun, Feb 5, 2012 at 4:26 PM, Patrick Ohly <patrick.ohly@intel.com> wrote:
>> On Mon, 2012-01-16 at 11:09 +0100, Patrick Ohly wrote:
>>> On Mo, 2012-01-16 at 10:24 +0100, Marcel Holtmann wrote:
>>> > Also this topic
>>> > has been raised before and I need confirmation for a GCC guru to confirm
>>> > that this does exactly the same all all platforms.
>>>
>>> Now that you mentioned it I found the previous patch:
>>> http://article.gmane.org/gmane.linux.bluez.kernel/20276/match=invalid
>>> +conversion+void+bt_get_le64+anonymous+struct
>>>
>>> Note that my patch is different. It keeps the "struct
>>> __attribute__((packed))" magic and merely changes the (void *) typecast
>>> to something that works in C++ and C.
>>
>> How do we move forward with this? More and more distros are picking up
>> the broken header file.
>>
>> In MeeGo and Tizen we applied the patch that I had suggested.
>>
>> Milan, do you know what Fedora is doing? Tino, what about Debian? Roran,
>> you had it in Ubuntu Precise Pangolin. Care to file a bug there, if
>> there isn't one already?
>>
>> Would it be more acceptable for upstream to put the modified macros into
>> an ifdef so that they are really only used in C++, keeping the code for
>> C as it is now?
>>
>> I'll send such a patch immediately.
>>
>> --
>> 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 in C++ with ifdef

[Rohan Garg]

Seems like your patch causes a build failiure : http://paste.ubuntu.com/829971/

Best
Rohan Garg



On Sun, Feb 5, 2012 at 5:03 PM, Rohan Garg <rohangarg@ubuntu.com> wrote:
> Ah, understood :)
>
> Best
> Rohan Garg
>
>
>
> On Sun, Feb 5, 2012 at 5:03 PM, Patrick Ohly <patrick.ohly@intel.com> wrote:
>> On Sun, 2012-02-05 at 16:37 +0530, Rohan Garg wrote:
>>> I've not gone through the entire patch, but if the problem is merely
>>> the fact that one cannot convert a void* to anything in C++, why not
>>> use a static_cast in syncevolution wherever the struct is used?
>>
>> The problem is not in the way how SyncEvolution uses the structs. The
>> problem is in the header files; it also occurs in any other C++ app
>> including the bluetooth.h file.
>>
>> --
>> 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 in C++ with ifdef

[Rohan Garg]

Hmm, quilt went full on retard and skipped a portion of your patch,
fixed it manually.

Regards
Rohan Garg



On Sun, Feb 5, 2012 at 5:53 PM, Rohan Garg <rohangarg@ubuntu.com> wrote:
> Seems like your patch causes a build failiure : http://paste.ubuntu.com/829971/
>
> Best
> Rohan Garg
>
>
>
> On Sun, Feb 5, 2012 at 5:03 PM, Rohan Garg <rohangarg@ubuntu.com> wrote:
>> Ah, understood :)
>>
>> Best
>> Rohan Garg
>>
>>
>>
>> On Sun, Feb 5, 2012 at 5:03 PM, Patrick Ohly <patrick.ohly@intel.com> wrote:
>>> On Sun, 2012-02-05 at 16:37 +0530, Rohan Garg wrote:
>>>> I've not gone through the entire patch, but if the problem is merely
>>>> the fact that one cannot convert a void* to anything in C++, why not
>>>> use a static_cast in syncevolution wherever the struct is used?
>>>
>>> The problem is not in the way how SyncEvolution uses the structs. The
>>> problem is in the header files; it also occurs in any other C++ app
>>> including the bluetooth.h file.
>>>
>>> --
>>> 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: bluez 4.97: build failure when used in C++ apps

[Tino Keitel]

On Sun, Feb 05, 2012 at 11:56:03 +0100, Patrick Ohly wrote:

[...]

> How do we move forward with this? More and more distros are picking up
> the broken header file.
> 
> In MeeGo and Tizen we applied the patch that I had suggested.

FYI, Debian also applied a patch and syncevolution builds fine.

Regards,
Tino

Re: bluez 4.97: build failure when used in C++ apps

[Rohan Garg]

Ah, seems you are correct. Thanks for pointing that out, I'll contact
the person who last uploaded the package in ubuntu to merge it from
debian.

Regards
Rohan Garg



On Sun, Feb 5, 2012 at 7:59 PM, Tino Keitel <tino.keitel@tikei.de> wrote:
> On Sun, Feb 05, 2012 at 11:56:03 +0100, Patrick Ohly wrote:
>
> [...]
>
>> How do we move forward with this? More and more distros are picking up
>> the broken header file.
>>
>> In MeeGo and Tizen we applied the patch that I had suggested.
>
> FYI, Debian also applied a patch and syncevolution builds fine.
>
> Regards,
> Tino

Re: bluez 4.97: build failure when used in C++ apps

[Milan Crha]

On Sun, 2012-02-05 at 16:29 +0530, Rohan Garg wrote:
> I can file a bug for tracking purposes and get the patched package in
> the archives, as long as upstream is willing to accept the patch ( or
> a modified version of the patch ) in the near future. I don't speak
> for the rest of the Ubuntu developers, but I consider the best
> approach to getting issues fixed in distros.

	Hi,
I do not package bluez for Fedora, thus I've no idea about it, but we
found this issue while building on Fedora.

Rohan, is it a typo or you really meant to say "fixed in distro"? As far
as I can tell, all the major distros will suffer of the issue, thus why
should each of them apply patch on their own, when it belongs to
upstream, from my point of view?
	Bye,
	Milan

Re: bluez 4.97: build failure when used in C++ apps

[Patrick Ohly]

On Mon, 2012-02-06 at 09:46 +0100, Milan Crha wrote:
> On Sun, 2012-02-05 at 16:29 +0530, Rohan Garg wrote:
> > I can file a bug for tracking purposes and get the patched package in
> > the archives, as long as upstream is willing to accept the patch ( or
> > a modified version of the patch ) in the near future. I don't speak
> > for the rest of the Ubuntu developers, but I consider the best
> > approach to getting issues fixed in distros.
> 
> 	Hi,
> I do not package bluez for Fedora, thus I've no idea about it, but we
> found this issue while building on Fedora.

And is it still broken or was bluez patched? I don't remember.

> Rohan, is it a typo or you really meant to say "fixed in distro"? As far
> as I can tell, all the major distros will suffer of the issue, thus why
> should each of them apply patch on their own, when it belongs to
> upstream, from my point of view?

Fixing it upstream certainly would be preferred, but as long as it is
not fixed there, distros need to find a workaround if they want to
continue supporting C++ apps like SyncEvolution.

-- 
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: bluez 4.97: build failure when used in C++ apps

[Milan Crha]

On Mon, 2012-02-06 at 10:29 +0100, Patrick Ohly wrote:
> On Mon, 2012-02-06 at 09:46 +0100, Milan Crha wrote:
> > On Sun, 2012-02-05 at 16:29 +0530, Rohan Garg wrote:
> > > I can file a bug for tracking purposes and get the patched package in
> > > the archives, as long as upstream is willing to accept the patch ( or
> > > a modified version of the patch ) in the near future. I don't speak
> > > for the rest of the Ubuntu developers, but I consider the best
> > > approach to getting issues fixed in distros.
> > 
> > I do not package bluez for Fedora, thus I've no idea about it, but we
> > found this issue while building on Fedora.
> 
> And is it still broken or was bluez patched? I don't remember.

Not patched (yet).
http://koji.fedoraproject.org/koji/packageinfo?packageID=6777

	Milan