Mirror of the lttng-dev mailing list
 help / color / mirror / Atom feed
* [ltt-dev] [UST PATCH] Fixes container_of macro redefinition warnings
@ 2010-09-22 20:52 David Goulet
  2010-09-22 21:25 ` Mathieu Desnoyers
  2010-09-23 14:27 ` Nils Carlson
  0 siblings, 2 replies; 4+ messages in thread
From: David Goulet @ 2010-09-22 20:52 UTC (permalink / raw)


On urcu commit 453629a9317adef5b96c3d55e4dcd98db680997a, the
container_of macro was moved to urcu/compiler.h. Since, multiple
warnings appear about redefinition

Signed-off-by: David Goulet <david.goulet at polymtl.ca>
---
 include/ust/core.h           |    2 ++
 include/ust/kcompat/simple.h |    3 ++-
 2 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/include/ust/core.h b/include/ust/core.h
index 0172614..d5a1f4d 100644
--- a/include/ust/core.h
+++ b/include/ust/core.h
@@ -141,8 +141,10 @@ static __inline__ int get_count_order(unsigned int count)
 	return order;
 }
 
+#ifndef container_of
 #define container_of(ptr, type, member) ({                      \
         const typeof( ((type *)0)->member ) *__mptr = (ptr);    \
         (type *)( (char *)__mptr - offsetof(type,member) );})
+#endif /* containter_of */
 
 #endif /* UST_CORE_H */
diff --git a/include/ust/kcompat/simple.h b/include/ust/kcompat/simple.h
index 762d802..586b2ed 100644
--- a/include/ust/kcompat/simple.h
+++ b/include/ust/kcompat/simple.h
@@ -28,10 +28,11 @@
  * @member:	the name of the member within the struct.
  *
  */
+#ifndef container_of
 #define container_of(ptr, type, member) ({			\
 	const typeof( ((type *)0)->member ) *__mptr = (ptr);	\
 	(type *)( (char *)__mptr - offsetof(type,member) );})
-
+#endif /* container_of */
 
 /* libkcompat: from rcupdate.h */
 
-- 
1.7.3




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

* [ltt-dev] [UST PATCH] Fixes container_of macro redefinition warnings
  2010-09-22 20:52 [ltt-dev] [UST PATCH] Fixes container_of macro redefinition warnings David Goulet
@ 2010-09-22 21:25 ` Mathieu Desnoyers
  2010-09-23 14:27 ` Nils Carlson
  1 sibling, 0 replies; 4+ messages in thread
From: Mathieu Desnoyers @ 2010-09-22 21:25 UTC (permalink / raw)


* David Goulet (david.goulet at polymtl.ca) wrote:
> On urcu commit 453629a9317adef5b96c3d55e4dcd98db680997a, the
> container_of macro was moved to urcu/compiler.h. Since, multiple
> warnings appear about redefinition
> 
> Signed-off-by: David Goulet <david.goulet at polymtl.ca>

Acked-by: Mathieu Desnoyers <mathieu.desnoyers at efficios.com>

> ---
>  include/ust/core.h           |    2 ++
>  include/ust/kcompat/simple.h |    3 ++-
>  2 files changed, 4 insertions(+), 1 deletions(-)
> 
> diff --git a/include/ust/core.h b/include/ust/core.h
> index 0172614..d5a1f4d 100644
> --- a/include/ust/core.h
> +++ b/include/ust/core.h
> @@ -141,8 +141,10 @@ static __inline__ int get_count_order(unsigned int count)
>  	return order;
>  }
>  
> +#ifndef container_of
>  #define container_of(ptr, type, member) ({                      \
>          const typeof( ((type *)0)->member ) *__mptr = (ptr);    \
>          (type *)( (char *)__mptr - offsetof(type,member) );})
> +#endif /* containter_of */
>  
>  #endif /* UST_CORE_H */
> diff --git a/include/ust/kcompat/simple.h b/include/ust/kcompat/simple.h
> index 762d802..586b2ed 100644
> --- a/include/ust/kcompat/simple.h
> +++ b/include/ust/kcompat/simple.h
> @@ -28,10 +28,11 @@
>   * @member:	the name of the member within the struct.
>   *
>   */
> +#ifndef container_of
>  #define container_of(ptr, type, member) ({			\
>  	const typeof( ((type *)0)->member ) *__mptr = (ptr);	\
>  	(type *)( (char *)__mptr - offsetof(type,member) );})
> -
> +#endif /* container_of */
>  
>  /* libkcompat: from rcupdate.h */
>  
> -- 
> 1.7.3
> 
> 
> _______________________________________________
> ltt-dev mailing list
> ltt-dev at lists.casi.polymtl.ca
> http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev
> 

-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com




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

* [ltt-dev] [UST PATCH] Fixes container_of macro redefinition warnings
  2010-09-22 20:52 [ltt-dev] [UST PATCH] Fixes container_of macro redefinition warnings David Goulet
  2010-09-22 21:25 ` Mathieu Desnoyers
@ 2010-09-23 14:27 ` Nils Carlson
  2010-09-23 14:40   ` David Goulet
  1 sibling, 1 reply; 4+ messages in thread
From: Nils Carlson @ 2010-09-23 14:27 UTC (permalink / raw)


Commente below...

On Wed, 22 Sep 2010, David Goulet wrote:

> On urcu commit 453629a9317adef5b96c3d55e4dcd98db680997a, the
> container_of macro was moved to urcu/compiler.h. Since, multiple
> warnings appear about redefinition
>

I think we should consider things in a more general sense. This is a 
little bit of what I think the Qemu/KVM people were complaining about.

We are terrible namespace polluters. Ideally, when you make a library,
you want to provide a limited set of external api's that are well defined
and well maintained and consistent over versions. You then want to hide
EVERYTHING else. This means that we should put some effort into hiding
all the macros and functions that we don't want to explicity export.

So I suggest we prefix all out internal-only macros which are visible in
global header files with __ust_. I would suggest Mathieu do the same with
urcu. If we don't want to support the macros, then they should have a 
private namespace. I think container_of is an excellent example of this, 
we use it, but it's not ours to export.

So a NACK here. Instead, call it __ust_container_of and then fix all the 
call sites.

/Nils




> Signed-off-by: David Goulet <david.goulet at polymtl.ca>
> ---
> include/ust/core.h           |    2 ++
> include/ust/kcompat/simple.h |    3 ++-
> 2 files changed, 4 insertions(+), 1 deletions(-)
>
> diff --git a/include/ust/core.h b/include/ust/core.h
> index 0172614..d5a1f4d 100644
> --- a/include/ust/core.h
> +++ b/include/ust/core.h
> @@ -141,8 +141,10 @@ static __inline__ int get_count_order(unsigned int count)
> 	return order;
> }
>
> +#ifndef container_of
> #define container_of(ptr, type, member) ({                      \
>         const typeof( ((type *)0)->member ) *__mptr = (ptr);    \
>         (type *)( (char *)__mptr - offsetof(type,member) );})
> +#endif /* containter_of */
>
> #endif /* UST_CORE_H */
> diff --git a/include/ust/kcompat/simple.h b/include/ust/kcompat/simple.h
> index 762d802..586b2ed 100644
> --- a/include/ust/kcompat/simple.h
> +++ b/include/ust/kcompat/simple.h
> @@ -28,10 +28,11 @@
>  * @member:	the name of the member within the struct.
>  *
>  */
> +#ifndef container_of
> #define container_of(ptr, type, member) ({			\
> 	const typeof( ((type *)0)->member ) *__mptr = (ptr);	\
> 	(type *)( (char *)__mptr - offsetof(type,member) );})
> -
> +#endif /* container_of */
>
> /* libkcompat: from rcupdate.h */
>
> -- 
> 1.7.3
>
>
> _______________________________________________
> ltt-dev mailing list
> ltt-dev at lists.casi.polymtl.ca
> http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev
>




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

* [ltt-dev] [UST PATCH] Fixes container_of macro redefinition warnings
  2010-09-23 14:27 ` Nils Carlson
@ 2010-09-23 14:40   ` David Goulet
  0 siblings, 0 replies; 4+ messages in thread
From: David Goulet @ 2010-09-23 14:40 UTC (permalink / raw)


-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

I totally agree with you!

Stefan Hajnoczi (of QEMU) raised this concern and I wanted to do something like
you just proposed! So, very glad you came up with that comment now! Let's do this ;)

I'll resubmit soon

Cheers
David

On 10-09-23 10:27 AM, Nils Carlson wrote:
> Commente below...
> 
> On Wed, 22 Sep 2010, David Goulet wrote:
> 
>> On urcu commit 453629a9317adef5b96c3d55e4dcd98db680997a, the
>> container_of macro was moved to urcu/compiler.h. Since, multiple
>> warnings appear about redefinition
>>
> 
> I think we should consider things in a more general sense. This is a
> little bit of what I think the Qemu/KVM people were complaining about.
> 
> We are terrible namespace polluters. Ideally, when you make a library,
> you want to provide a limited set of external api's that are well defined
> and well maintained and consistent over versions. You then want to hide
> EVERYTHING else. This means that we should put some effort into hiding
> all the macros and functions that we don't want to explicity export.
> 
> So I suggest we prefix all out internal-only macros which are visible in
> global header files with __ust_. I would suggest Mathieu do the same with
> urcu. If we don't want to support the macros, then they should have a
> private namespace. I think container_of is an excellent example of this,
> we use it, but it's not ours to export.
> 
> So a NACK here. Instead, call it __ust_container_of and then fix all the
> call sites.
> 
> /Nils
> 
> 
> 
> 
>> Signed-off-by: David Goulet <david.goulet at polymtl.ca>
>> ---
>> include/ust/core.h           |    2 ++
>> include/ust/kcompat/simple.h |    3 ++-
>> 2 files changed, 4 insertions(+), 1 deletions(-)
>>
>> diff --git a/include/ust/core.h b/include/ust/core.h
>> index 0172614..d5a1f4d 100644
>> --- a/include/ust/core.h
>> +++ b/include/ust/core.h
>> @@ -141,8 +141,10 @@ static __inline__ int get_count_order(unsigned
>> int count)
>>     return order;
>> }
>>
>> +#ifndef container_of
>> #define container_of(ptr, type, member) ({                      \
>>         const typeof( ((type *)0)->member ) *__mptr = (ptr);    \
>>         (type *)( (char *)__mptr - offsetof(type,member) );})
>> +#endif /* containter_of */
>>
>> #endif /* UST_CORE_H */
>> diff --git a/include/ust/kcompat/simple.h b/include/ust/kcompat/simple.h
>> index 762d802..586b2ed 100644
>> --- a/include/ust/kcompat/simple.h
>> +++ b/include/ust/kcompat/simple.h
>> @@ -28,10 +28,11 @@
>>  * @member:    the name of the member within the struct.
>>  *
>>  */
>> +#ifndef container_of
>> #define container_of(ptr, type, member) ({            \
>>     const typeof( ((type *)0)->member ) *__mptr = (ptr);    \
>>     (type *)( (char *)__mptr - offsetof(type,member) );})
>> -
>> +#endif /* container_of */
>>
>> /* libkcompat: from rcupdate.h */
>>
>> -- 
>> 1.7.3
>>
>>
>> _______________________________________________
>> ltt-dev mailing list
>> ltt-dev at lists.casi.polymtl.ca
>> http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev
>>

- -- 
David Goulet
LTTng project, DORSAL Lab.

1024D/16BD8563
BE3C 672B 9331 9796 291A  14C6 4AF7 C14B 16BD 8563
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)

iEYEARECAAYFAkybZvMACgkQSvfBSxa9hWPy1gCfdA9KZag2rLL3IkPtprE31DXT
hEEAn3OglxXOTbs0kLIGRPHFAsNY4JFo
=afmW
-----END PGP SIGNATURE-----




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

end of thread, other threads:[~2010-09-23 14:40 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-09-22 20:52 [ltt-dev] [UST PATCH] Fixes container_of macro redefinition warnings David Goulet
2010-09-22 21:25 ` Mathieu Desnoyers
2010-09-23 14:27 ` Nils Carlson
2010-09-23 14:40   ` David Goulet

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox