Mirror of the lttng-dev mailing list
 help / color / mirror / Atom feed
* [ltt-dev] [UST PATCH] New naming convention for macro container_of
@ 2010-09-23 16:26 David Goulet
  2010-09-23 16:46 ` Mathieu Desnoyers
  2010-09-23 16:48 ` David Goulet
  0 siblings, 2 replies; 8+ messages in thread
From: David Goulet @ 2010-09-23 16:26 UTC (permalink / raw)


This is the first patch introducing the new naming convention
for the UST tracer. To prevent namespace pollution, __ust_ prefix
is added to visible symbol in global header files.

For this patch, only the container_of macro is renamed and the
redundant definition in kcompat/simple.h is removed.

Signed-off-by: David Goulet <david.goulet at polymtl.ca>
---
 include/ust/core.h           |    2 +-
 include/ust/kcompat/simple.h |   12 ------------
 libust/buffers.c             |    6 +++---
 libust/channels.c            |    2 +-
 libust/marker.c              |    2 +-
 libust/tracepoint.c          |    4 ++--
 libust/tracer.c              |    2 +-
 7 files changed, 9 insertions(+), 21 deletions(-)

diff --git a/include/ust/core.h b/include/ust/core.h
index 0172614..e23224a 100644
--- a/include/ust/core.h
+++ b/include/ust/core.h
@@ -141,7 +141,7 @@ static __inline__ int get_count_order(unsigned int count)
 	return order;
 }
 
-#define container_of(ptr, type, member) ({                      \
+#define __ust_container_of(ptr, type, member) ({                      \
         const typeof( ((type *)0)->member ) *__mptr = (ptr);    \
         (type *)( (char *)__mptr - offsetof(type,member) );})
 
diff --git a/include/ust/kcompat/simple.h b/include/ust/kcompat/simple.h
index 762d802..38a9be5 100644
--- a/include/ust/kcompat/simple.h
+++ b/include/ust/kcompat/simple.h
@@ -21,18 +21,6 @@
 #ifndef KCOMPAT_SIMPLE_H
 #define KCOMPAT_SIMPLE_H
 
-/**
- * container_of - cast a member of a structure out to the containing structure
- * @ptr:	the pointer to the member.
- * @type:	the type of the container struct this is embedded in.
- * @member:	the name of the member within the struct.
- *
- */
-#define container_of(ptr, type, member) ({			\
-	const typeof( ((type *)0)->member ) *__mptr = (ptr);	\
-	(type *)( (char *)__mptr - offsetof(type,member) );})
-
-
 /* libkcompat: from rcupdate.h */
 
 struct rcu_head {
diff --git a/libust/buffers.c b/libust/buffers.c
index 374ec61..8a2b90a 100644
--- a/libust/buffers.c
+++ b/libust/buffers.c
@@ -203,7 +203,7 @@ int ust_buffers_create_buf(struct ust_channel *channel, int cpu)
 
 static void ust_buffers_destroy_channel(struct kref *kref)
 {
-	struct ust_channel *chan = container_of(kref, struct ust_channel, kref);
+	struct ust_channel *chan = __ust_container_of(kref, struct ust_channel, kref);
 	free(chan);
 }
 
@@ -225,7 +225,7 @@ static void ust_buffers_destroy_buf(struct ust_buffer *buf)
 /* called from kref_put */
 static void ust_buffers_remove_buf(struct kref *kref)
 {
-	struct ust_buffer *buf = container_of(kref, struct ust_buffer, kref);
+	struct ust_buffer *buf = __ust_container_of(kref, struct ust_buffer, kref);
 	ust_buffers_destroy_buf(buf);
 }
 
@@ -592,7 +592,7 @@ static void ltt_relay_print_buffer_errors(struct ust_channel *channel, int cpu)
 
 static void ltt_relay_release_channel(struct kref *kref)
 {
-	struct ust_channel *ltt_chan = container_of(kref,
+	struct ust_channel *ltt_chan = __ust_container_of(kref,
 			struct ust_channel, kref);
 	free(ltt_chan->buf);
 }
diff --git a/libust/channels.c b/libust/channels.c
index df1a972..4de54a9 100644
--- a/libust/channels.c
+++ b/libust/channels.c
@@ -66,7 +66,7 @@ static struct ltt_channel_setting *lookup_channel(const char *name)
  */
 static void release_channel_setting(struct kref *kref)
 {
-	struct ltt_channel_setting *setting = container_of(kref,
+	struct ltt_channel_setting *setting = __ust_container_of(kref,
 		struct ltt_channel_setting, kref);
 	struct ltt_channel_setting *iter;
 
diff --git a/libust/marker.c b/libust/marker.c
index dbabf3e..761785a 100644
--- a/libust/marker.c
+++ b/libust/marker.c
@@ -235,7 +235,7 @@ static notrace void marker_probe_cb_noarg(const struct marker *mdata,
 
 static void free_old_closure(struct rcu_head *head)
 {
-	struct marker_entry *entry = container_of(head,
+	struct marker_entry *entry = __ust_container_of(head,
 		struct marker_entry, rcu);
 	free(entry->oldptr);
 	/* Make sure we free the data before setting the pending flag to 0 */
diff --git a/libust/tracepoint.c b/libust/tracepoint.c
index 16b4819..63ae17d 100644
--- a/libust/tracepoint.c
+++ b/libust/tracepoint.c
@@ -87,7 +87,7 @@ static inline void *allocate_probes(int count)
 static inline void release_probes(void *old)
 {
 	if (old) {
-		struct tp_probes *tp_probes = container_of(old,
+		struct tp_probes *tp_probes = __ust_container_of(old,
 			struct tp_probes, probes[0]);
 //ust//		call_rcu_sched(&tp_probes->u.rcu, rcu_free_old_probes);
 		synchronize_rcu();
@@ -427,7 +427,7 @@ static void tracepoint_add_old_probes(void *old)
 {
 	need_update = 1;
 	if (old) {
-		struct tp_probes *tp_probes = container_of(old,
+		struct tp_probes *tp_probes = __ust_container_of(old,
 			struct tp_probes, probes[0]);
 		list_add(&tp_probes->u.list, &old_probes);
 	}
diff --git a/libust/tracer.c b/libust/tracer.c
index 8c3f774..ed5e329 100644
--- a/libust/tracer.c
+++ b/libust/tracer.c
@@ -337,7 +337,7 @@ void ltt_release_transport(struct kref *kref)
  */
 void ltt_release_trace(struct kref *kref)
 {
-	struct ust_trace *trace = container_of(kref,
+	struct ust_trace *trace = __ust_container_of(kref,
 			struct ust_trace, kref);
 	ltt_channels_trace_free(trace->channels);
 	free(trace);
-- 
1.7.3





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

* [ltt-dev] [UST PATCH] New naming convention for macro container_of
  2010-09-23 16:26 [ltt-dev] [UST PATCH] New naming convention for macro container_of David Goulet
@ 2010-09-23 16:46 ` Mathieu Desnoyers
  2010-09-23 17:29   ` Nils Carlson
  2010-09-23 16:48 ` David Goulet
  1 sibling, 1 reply; 8+ messages in thread
From: Mathieu Desnoyers @ 2010-09-23 16:46 UTC (permalink / raw)


* David Goulet (david.goulet at polymtl.ca) wrote:
> This is the first patch introducing the new naming convention
> for the UST tracer. To prevent namespace pollution, __ust_ prefix
> is added to visible symbol in global header files.

I'd use the "ust_" prefix rather than "__ust_", because __ is usually
reserved for the compiler (and makes function names needlessly long).

Mathieu

> 
> For this patch, only the container_of macro is renamed and the
> redundant definition in kcompat/simple.h is removed.
> 
> Signed-off-by: David Goulet <david.goulet at polymtl.ca>
> ---
>  include/ust/core.h           |    2 +-
>  include/ust/kcompat/simple.h |   12 ------------
>  libust/buffers.c             |    6 +++---
>  libust/channels.c            |    2 +-
>  libust/marker.c              |    2 +-
>  libust/tracepoint.c          |    4 ++--
>  libust/tracer.c              |    2 +-
>  7 files changed, 9 insertions(+), 21 deletions(-)
> 
> diff --git a/include/ust/core.h b/include/ust/core.h
> index 0172614..e23224a 100644
> --- a/include/ust/core.h
> +++ b/include/ust/core.h
> @@ -141,7 +141,7 @@ static __inline__ int get_count_order(unsigned int count)
>  	return order;
>  }
>  
> -#define container_of(ptr, type, member) ({                      \
> +#define __ust_container_of(ptr, type, member) ({                      \
>          const typeof( ((type *)0)->member ) *__mptr = (ptr);    \
>          (type *)( (char *)__mptr - offsetof(type,member) );})
>  
> diff --git a/include/ust/kcompat/simple.h b/include/ust/kcompat/simple.h
> index 762d802..38a9be5 100644
> --- a/include/ust/kcompat/simple.h
> +++ b/include/ust/kcompat/simple.h
> @@ -21,18 +21,6 @@
>  #ifndef KCOMPAT_SIMPLE_H
>  #define KCOMPAT_SIMPLE_H
>  
> -/**
> - * container_of - cast a member of a structure out to the containing structure
> - * @ptr:	the pointer to the member.
> - * @type:	the type of the container struct this is embedded in.
> - * @member:	the name of the member within the struct.
> - *
> - */
> -#define container_of(ptr, type, member) ({			\
> -	const typeof( ((type *)0)->member ) *__mptr = (ptr);	\
> -	(type *)( (char *)__mptr - offsetof(type,member) );})
> -
> -
>  /* libkcompat: from rcupdate.h */
>  
>  struct rcu_head {
> diff --git a/libust/buffers.c b/libust/buffers.c
> index 374ec61..8a2b90a 100644
> --- a/libust/buffers.c
> +++ b/libust/buffers.c
> @@ -203,7 +203,7 @@ int ust_buffers_create_buf(struct ust_channel *channel, int cpu)
>  
>  static void ust_buffers_destroy_channel(struct kref *kref)
>  {
> -	struct ust_channel *chan = container_of(kref, struct ust_channel, kref);
> +	struct ust_channel *chan = __ust_container_of(kref, struct ust_channel, kref);
>  	free(chan);
>  }
>  
> @@ -225,7 +225,7 @@ static void ust_buffers_destroy_buf(struct ust_buffer *buf)
>  /* called from kref_put */
>  static void ust_buffers_remove_buf(struct kref *kref)
>  {
> -	struct ust_buffer *buf = container_of(kref, struct ust_buffer, kref);
> +	struct ust_buffer *buf = __ust_container_of(kref, struct ust_buffer, kref);
>  	ust_buffers_destroy_buf(buf);
>  }
>  
> @@ -592,7 +592,7 @@ static void ltt_relay_print_buffer_errors(struct ust_channel *channel, int cpu)
>  
>  static void ltt_relay_release_channel(struct kref *kref)
>  {
> -	struct ust_channel *ltt_chan = container_of(kref,
> +	struct ust_channel *ltt_chan = __ust_container_of(kref,
>  			struct ust_channel, kref);
>  	free(ltt_chan->buf);
>  }
> diff --git a/libust/channels.c b/libust/channels.c
> index df1a972..4de54a9 100644
> --- a/libust/channels.c
> +++ b/libust/channels.c
> @@ -66,7 +66,7 @@ static struct ltt_channel_setting *lookup_channel(const char *name)
>   */
>  static void release_channel_setting(struct kref *kref)
>  {
> -	struct ltt_channel_setting *setting = container_of(kref,
> +	struct ltt_channel_setting *setting = __ust_container_of(kref,
>  		struct ltt_channel_setting, kref);
>  	struct ltt_channel_setting *iter;
>  
> diff --git a/libust/marker.c b/libust/marker.c
> index dbabf3e..761785a 100644
> --- a/libust/marker.c
> +++ b/libust/marker.c
> @@ -235,7 +235,7 @@ static notrace void marker_probe_cb_noarg(const struct marker *mdata,
>  
>  static void free_old_closure(struct rcu_head *head)
>  {
> -	struct marker_entry *entry = container_of(head,
> +	struct marker_entry *entry = __ust_container_of(head,
>  		struct marker_entry, rcu);
>  	free(entry->oldptr);
>  	/* Make sure we free the data before setting the pending flag to 0 */
> diff --git a/libust/tracepoint.c b/libust/tracepoint.c
> index 16b4819..63ae17d 100644
> --- a/libust/tracepoint.c
> +++ b/libust/tracepoint.c
> @@ -87,7 +87,7 @@ static inline void *allocate_probes(int count)
>  static inline void release_probes(void *old)
>  {
>  	if (old) {
> -		struct tp_probes *tp_probes = container_of(old,
> +		struct tp_probes *tp_probes = __ust_container_of(old,
>  			struct tp_probes, probes[0]);
>  //ust//		call_rcu_sched(&tp_probes->u.rcu, rcu_free_old_probes);
>  		synchronize_rcu();
> @@ -427,7 +427,7 @@ static void tracepoint_add_old_probes(void *old)
>  {
>  	need_update = 1;
>  	if (old) {
> -		struct tp_probes *tp_probes = container_of(old,
> +		struct tp_probes *tp_probes = __ust_container_of(old,
>  			struct tp_probes, probes[0]);
>  		list_add(&tp_probes->u.list, &old_probes);
>  	}
> diff --git a/libust/tracer.c b/libust/tracer.c
> index 8c3f774..ed5e329 100644
> --- a/libust/tracer.c
> +++ b/libust/tracer.c
> @@ -337,7 +337,7 @@ void ltt_release_transport(struct kref *kref)
>   */
>  void ltt_release_trace(struct kref *kref)
>  {
> -	struct ust_trace *trace = container_of(kref,
> +	struct ust_trace *trace = __ust_container_of(kref,
>  			struct ust_trace, kref);
>  	ltt_channels_trace_free(trace->channels);
>  	free(trace);
> -- 
> 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] 8+ messages in thread

* [ltt-dev] [UST PATCH] New naming convention for macro container_of
  2010-09-23 16:26 [ltt-dev] [UST PATCH] New naming convention for macro container_of David Goulet
  2010-09-23 16:46 ` Mathieu Desnoyers
@ 2010-09-23 16:48 ` David Goulet
  2010-09-23 17:07   ` Mathieu Desnoyers
  2010-09-26  4:46   ` Pierre-Marc Fournier
  1 sibling, 2 replies; 8+ messages in thread
From: David Goulet @ 2010-09-23 16:48 UTC (permalink / raw)


Following this.

We are talking here about a major overhaul of naming convention. Just 
for trace_mark, basically, every symbols in marker.h and following 
headers have to be refactor with the __ust_ (or ust_) prefix :

struct marker --> struct __ust_marker, and so on for types, macros and 
functions. This is quite a huge work in term of "syntax changes" and, as 
Mathieu pointed out, *long* name. I'm perfectly convince that this is 
necessary but if so now, should we than consider removing old LTTng 
comments, rename all ltt_* function to ust_*, etc... and so doing a 
complete cleanup once and for all ?

Cheers
David

On 10-09-23 12:26 PM, David Goulet wrote:
> This is the first patch introducing the new naming convention
> for the UST tracer. To prevent namespace pollution, __ust_ prefix
> is added to visible symbol in global header files.
>
> For this patch, only the container_of macro is renamed and the
> redundant definition in kcompat/simple.h is removed.
>
> Signed-off-by: David Goulet<david.goulet at polymtl.ca>
> ---
>   include/ust/core.h           |    2 +-
>   include/ust/kcompat/simple.h |   12 ------------
>   libust/buffers.c             |    6 +++---
>   libust/channels.c            |    2 +-
>   libust/marker.c              |    2 +-
>   libust/tracepoint.c          |    4 ++--
>   libust/tracer.c              |    2 +-
>   7 files changed, 9 insertions(+), 21 deletions(-)
>
> diff --git a/include/ust/core.h b/include/ust/core.h
> index 0172614..e23224a 100644
> --- a/include/ust/core.h
> +++ b/include/ust/core.h
> @@ -141,7 +141,7 @@ static __inline__ int get_count_order(unsigned int count)
>   	return order;
>   }
>
> -#define container_of(ptr, type, member) ({                      \
> +#define __ust_container_of(ptr, type, member) ({                      \
>           const typeof( ((type *)0)->member ) *__mptr = (ptr);    \
>           (type *)( (char *)__mptr - offsetof(type,member) );})
>
> diff --git a/include/ust/kcompat/simple.h b/include/ust/kcompat/simple.h
> index 762d802..38a9be5 100644
> --- a/include/ust/kcompat/simple.h
> +++ b/include/ust/kcompat/simple.h
> @@ -21,18 +21,6 @@
>   #ifndef KCOMPAT_SIMPLE_H
>   #define KCOMPAT_SIMPLE_H
>
> -/**
> - * container_of - cast a member of a structure out to the containing structure
> - * @ptr:	the pointer to the member.
> - * @type:	the type of the container struct this is embedded in.
> - * @member:	the name of the member within the struct.
> - *
> - */
> -#define container_of(ptr, type, member) ({			\
> -	const typeof( ((type *)0)->member ) *__mptr = (ptr);	\
> -	(type *)( (char *)__mptr - offsetof(type,member) );})
> -
> -
>   /* libkcompat: from rcupdate.h */
>
>   struct rcu_head {
> diff --git a/libust/buffers.c b/libust/buffers.c
> index 374ec61..8a2b90a 100644
> --- a/libust/buffers.c
> +++ b/libust/buffers.c
> @@ -203,7 +203,7 @@ int ust_buffers_create_buf(struct ust_channel *channel, int cpu)
>
>   static void ust_buffers_destroy_channel(struct kref *kref)
>   {
> -	struct ust_channel *chan = container_of(kref, struct ust_channel, kref);
> +	struct ust_channel *chan = __ust_container_of(kref, struct ust_channel, kref);
>   	free(chan);
>   }
>
> @@ -225,7 +225,7 @@ static void ust_buffers_destroy_buf(struct ust_buffer *buf)
>   /* called from kref_put */
>   static void ust_buffers_remove_buf(struct kref *kref)
>   {
> -	struct ust_buffer *buf = container_of(kref, struct ust_buffer, kref);
> +	struct ust_buffer *buf = __ust_container_of(kref, struct ust_buffer, kref);
>   	ust_buffers_destroy_buf(buf);
>   }
>
> @@ -592,7 +592,7 @@ static void ltt_relay_print_buffer_errors(struct ust_channel *channel, int cpu)
>
>   static void ltt_relay_release_channel(struct kref *kref)
>   {
> -	struct ust_channel *ltt_chan = container_of(kref,
> +	struct ust_channel *ltt_chan = __ust_container_of(kref,
>   			struct ust_channel, kref);
>   	free(ltt_chan->buf);
>   }
> diff --git a/libust/channels.c b/libust/channels.c
> index df1a972..4de54a9 100644
> --- a/libust/channels.c
> +++ b/libust/channels.c
> @@ -66,7 +66,7 @@ static struct ltt_channel_setting *lookup_channel(const char *name)
>    */
>   static void release_channel_setting(struct kref *kref)
>   {
> -	struct ltt_channel_setting *setting = container_of(kref,
> +	struct ltt_channel_setting *setting = __ust_container_of(kref,
>   		struct ltt_channel_setting, kref);
>   	struct ltt_channel_setting *iter;
>
> diff --git a/libust/marker.c b/libust/marker.c
> index dbabf3e..761785a 100644
> --- a/libust/marker.c
> +++ b/libust/marker.c
> @@ -235,7 +235,7 @@ static notrace void marker_probe_cb_noarg(const struct marker *mdata,
>
>   static void free_old_closure(struct rcu_head *head)
>   {
> -	struct marker_entry *entry = container_of(head,
> +	struct marker_entry *entry = __ust_container_of(head,
>   		struct marker_entry, rcu);
>   	free(entry->oldptr);
>   	/* Make sure we free the data before setting the pending flag to 0 */
> diff --git a/libust/tracepoint.c b/libust/tracepoint.c
> index 16b4819..63ae17d 100644
> --- a/libust/tracepoint.c
> +++ b/libust/tracepoint.c
> @@ -87,7 +87,7 @@ static inline void *allocate_probes(int count)
>   static inline void release_probes(void *old)
>   {
>   	if (old) {
> -		struct tp_probes *tp_probes = container_of(old,
> +		struct tp_probes *tp_probes = __ust_container_of(old,
>   			struct tp_probes, probes[0]);
>   //ust//		call_rcu_sched(&tp_probes->u.rcu, rcu_free_old_probes);
>   		synchronize_rcu();
> @@ -427,7 +427,7 @@ static void tracepoint_add_old_probes(void *old)
>   {
>   	need_update = 1;
>   	if (old) {
> -		struct tp_probes *tp_probes = container_of(old,
> +		struct tp_probes *tp_probes = __ust_container_of(old,
>   			struct tp_probes, probes[0]);
>   		list_add(&tp_probes->u.list,&old_probes);
>   	}
> diff --git a/libust/tracer.c b/libust/tracer.c
> index 8c3f774..ed5e329 100644
> --- a/libust/tracer.c
> +++ b/libust/tracer.c
> @@ -337,7 +337,7 @@ void ltt_release_transport(struct kref *kref)
>    */
>   void ltt_release_trace(struct kref *kref)
>   {
> -	struct ust_trace *trace = container_of(kref,
> +	struct ust_trace *trace = __ust_container_of(kref,
>   			struct ust_trace, kref);
>   	ltt_channels_trace_free(trace->channels);
>   	free(trace);

-- 
David Goulet
LTTng project, DORSAL Lab.

PGP/GPG : 1024D/16BD8563
BE3C 672B 9331 9796 291A  14C6 4AF7 C14B 16BD 8563



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

* [ltt-dev] [UST PATCH] New naming convention for macro container_of
  2010-09-23 16:48 ` David Goulet
@ 2010-09-23 17:07   ` Mathieu Desnoyers
  2010-09-23 17:16     ` David Goulet
  2010-09-26  4:46   ` Pierre-Marc Fournier
  1 sibling, 1 reply; 8+ messages in thread
From: Mathieu Desnoyers @ 2010-09-23 17:07 UTC (permalink / raw)


* David Goulet (david.goulet at polymtl.ca) wrote:
> Following this.
>
> We are talking here about a major overhaul of naming convention. Just  
> for trace_mark, basically, every symbols in marker.h and following  
> headers have to be refactor with the __ust_ (or ust_) prefix :
>
> struct marker --> struct __ust_marker, and so on for types, macros and  
> functions. This is quite a huge work in term of "syntax changes" and, as  
> Mathieu pointed out, *long* name. I'm perfectly convince that this is  
> necessary but if so now, should we than consider removing old LTTng  
> comments, rename all ltt_* function to ust_*, etc... and so doing a  
> complete cleanup once and for all ?

Yes.

Although for the code that is shared between the Linux kernel and UST,
(e.g. struct marker), I would be tempted to keep the name as is.

Mathieu

>
> Cheers
> David
>
> On 10-09-23 12:26 PM, David Goulet wrote:
>> This is the first patch introducing the new naming convention
>> for the UST tracer. To prevent namespace pollution, __ust_ prefix
>> is added to visible symbol in global header files.
>>
>> For this patch, only the container_of macro is renamed and the
>> redundant definition in kcompat/simple.h is removed.
>>
>> Signed-off-by: David Goulet<david.goulet at polymtl.ca>
>> ---
>>   include/ust/core.h           |    2 +-
>>   include/ust/kcompat/simple.h |   12 ------------
>>   libust/buffers.c             |    6 +++---
>>   libust/channels.c            |    2 +-
>>   libust/marker.c              |    2 +-
>>   libust/tracepoint.c          |    4 ++--
>>   libust/tracer.c              |    2 +-
>>   7 files changed, 9 insertions(+), 21 deletions(-)
>>
>> diff --git a/include/ust/core.h b/include/ust/core.h
>> index 0172614..e23224a 100644
>> --- a/include/ust/core.h
>> +++ b/include/ust/core.h
>> @@ -141,7 +141,7 @@ static __inline__ int get_count_order(unsigned int count)
>>   	return order;
>>   }
>>
>> -#define container_of(ptr, type, member) ({                      \
>> +#define __ust_container_of(ptr, type, member) ({                      \
>>           const typeof( ((type *)0)->member ) *__mptr = (ptr);    \
>>           (type *)( (char *)__mptr - offsetof(type,member) );})
>>
>> diff --git a/include/ust/kcompat/simple.h b/include/ust/kcompat/simple.h
>> index 762d802..38a9be5 100644
>> --- a/include/ust/kcompat/simple.h
>> +++ b/include/ust/kcompat/simple.h
>> @@ -21,18 +21,6 @@
>>   #ifndef KCOMPAT_SIMPLE_H
>>   #define KCOMPAT_SIMPLE_H
>>
>> -/**
>> - * container_of - cast a member of a structure out to the containing structure
>> - * @ptr:	the pointer to the member.
>> - * @type:	the type of the container struct this is embedded in.
>> - * @member:	the name of the member within the struct.
>> - *
>> - */
>> -#define container_of(ptr, type, member) ({			\
>> -	const typeof( ((type *)0)->member ) *__mptr = (ptr);	\
>> -	(type *)( (char *)__mptr - offsetof(type,member) );})
>> -
>> -
>>   /* libkcompat: from rcupdate.h */
>>
>>   struct rcu_head {
>> diff --git a/libust/buffers.c b/libust/buffers.c
>> index 374ec61..8a2b90a 100644
>> --- a/libust/buffers.c
>> +++ b/libust/buffers.c
>> @@ -203,7 +203,7 @@ int ust_buffers_create_buf(struct ust_channel *channel, int cpu)
>>
>>   static void ust_buffers_destroy_channel(struct kref *kref)
>>   {
>> -	struct ust_channel *chan = container_of(kref, struct ust_channel, kref);
>> +	struct ust_channel *chan = __ust_container_of(kref, struct ust_channel, kref);
>>   	free(chan);
>>   }
>>
>> @@ -225,7 +225,7 @@ static void ust_buffers_destroy_buf(struct ust_buffer *buf)
>>   /* called from kref_put */
>>   static void ust_buffers_remove_buf(struct kref *kref)
>>   {
>> -	struct ust_buffer *buf = container_of(kref, struct ust_buffer, kref);
>> +	struct ust_buffer *buf = __ust_container_of(kref, struct ust_buffer, kref);
>>   	ust_buffers_destroy_buf(buf);
>>   }
>>
>> @@ -592,7 +592,7 @@ static void ltt_relay_print_buffer_errors(struct ust_channel *channel, int cpu)
>>
>>   static void ltt_relay_release_channel(struct kref *kref)
>>   {
>> -	struct ust_channel *ltt_chan = container_of(kref,
>> +	struct ust_channel *ltt_chan = __ust_container_of(kref,
>>   			struct ust_channel, kref);
>>   	free(ltt_chan->buf);
>>   }
>> diff --git a/libust/channels.c b/libust/channels.c
>> index df1a972..4de54a9 100644
>> --- a/libust/channels.c
>> +++ b/libust/channels.c
>> @@ -66,7 +66,7 @@ static struct ltt_channel_setting *lookup_channel(const char *name)
>>    */
>>   static void release_channel_setting(struct kref *kref)
>>   {
>> -	struct ltt_channel_setting *setting = container_of(kref,
>> +	struct ltt_channel_setting *setting = __ust_container_of(kref,
>>   		struct ltt_channel_setting, kref);
>>   	struct ltt_channel_setting *iter;
>>
>> diff --git a/libust/marker.c b/libust/marker.c
>> index dbabf3e..761785a 100644
>> --- a/libust/marker.c
>> +++ b/libust/marker.c
>> @@ -235,7 +235,7 @@ static notrace void marker_probe_cb_noarg(const struct marker *mdata,
>>
>>   static void free_old_closure(struct rcu_head *head)
>>   {
>> -	struct marker_entry *entry = container_of(head,
>> +	struct marker_entry *entry = __ust_container_of(head,
>>   		struct marker_entry, rcu);
>>   	free(entry->oldptr);
>>   	/* Make sure we free the data before setting the pending flag to 0 */
>> diff --git a/libust/tracepoint.c b/libust/tracepoint.c
>> index 16b4819..63ae17d 100644
>> --- a/libust/tracepoint.c
>> +++ b/libust/tracepoint.c
>> @@ -87,7 +87,7 @@ static inline void *allocate_probes(int count)
>>   static inline void release_probes(void *old)
>>   {
>>   	if (old) {
>> -		struct tp_probes *tp_probes = container_of(old,
>> +		struct tp_probes *tp_probes = __ust_container_of(old,
>>   			struct tp_probes, probes[0]);
>>   //ust//		call_rcu_sched(&tp_probes->u.rcu, rcu_free_old_probes);
>>   		synchronize_rcu();
>> @@ -427,7 +427,7 @@ static void tracepoint_add_old_probes(void *old)
>>   {
>>   	need_update = 1;
>>   	if (old) {
>> -		struct tp_probes *tp_probes = container_of(old,
>> +		struct tp_probes *tp_probes = __ust_container_of(old,
>>   			struct tp_probes, probes[0]);
>>   		list_add(&tp_probes->u.list,&old_probes);
>>   	}
>> diff --git a/libust/tracer.c b/libust/tracer.c
>> index 8c3f774..ed5e329 100644
>> --- a/libust/tracer.c
>> +++ b/libust/tracer.c
>> @@ -337,7 +337,7 @@ void ltt_release_transport(struct kref *kref)
>>    */
>>   void ltt_release_trace(struct kref *kref)
>>   {
>> -	struct ust_trace *trace = container_of(kref,
>> +	struct ust_trace *trace = __ust_container_of(kref,
>>   			struct ust_trace, kref);
>>   	ltt_channels_trace_free(trace->channels);
>>   	free(trace);
>
> -- 
> David Goulet
> LTTng project, DORSAL Lab.
>
> PGP/GPG : 1024D/16BD8563
> BE3C 672B 9331 9796 291A  14C6 4AF7 C14B 16BD 8563
>
> _______________________________________________
> 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] 8+ messages in thread

* [ltt-dev] [UST PATCH] New naming convention for macro container_of
  2010-09-23 17:07   ` Mathieu Desnoyers
@ 2010-09-23 17:16     ` David Goulet
  2010-09-23 17:31       ` Mathieu Desnoyers
  0 siblings, 1 reply; 8+ messages in thread
From: David Goulet @ 2010-09-23 17:16 UTC (permalink / raw)




On 10-09-23 01:07 PM, Mathieu Desnoyers wrote:
> * David Goulet (david.goulet at polymtl.ca) wrote:
>> Following this.
>>
>> We are talking here about a major overhaul of naming convention. Just
>> for trace_mark, basically, every symbols in marker.h and following
>> headers have to be refactor with the __ust_ (or ust_) prefix :
>>
>> struct marker -->  struct __ust_marker, and so on for types, macros and
>> functions. This is quite a huge work in term of "syntax changes" and, as
>> Mathieu pointed out, *long* name. I'm perfectly convince that this is
>> necessary but if so now, should we than consider removing old LTTng
>> comments, rename all ltt_* function to ust_*, etc... and so doing a
>> complete cleanup once and for all ?
>
> Yes.
>

I'll try to put some time on that each file at a time. Good

> Although for the code that is shared between the Linux kernel and UST,
> (e.g. struct marker), I would be tempted to keep the name as is.
>

As a funny fact, few months ago, without knowing that much about UST, I 
had a redefinition error on exactly struct marker in one of my program 
that I wanted to trace with UST... It's kind of annoying when you have 
to refactor all your code because of the tracer...

What's the main reason you are tempted to keep the name? (since UST come 
from LTTng, it's easier for porting new feature from ltt to ust...?)

Thanks
David

> Mathieu
>
>>
>> Cheers
>> David
>>
>> On 10-09-23 12:26 PM, David Goulet wrote:
>>> This is the first patch introducing the new naming convention
>>> for the UST tracer. To prevent namespace pollution, __ust_ prefix
>>> is added to visible symbol in global header files.
>>>
>>> For this patch, only the container_of macro is renamed and the
>>> redundant definition in kcompat/simple.h is removed.
>>>
>>> Signed-off-by: David Goulet<david.goulet at polymtl.ca>
>>> ---
>>>    include/ust/core.h           |    2 +-
>>>    include/ust/kcompat/simple.h |   12 ------------
>>>    libust/buffers.c             |    6 +++---
>>>    libust/channels.c            |    2 +-
>>>    libust/marker.c              |    2 +-
>>>    libust/tracepoint.c          |    4 ++--
>>>    libust/tracer.c              |    2 +-
>>>    7 files changed, 9 insertions(+), 21 deletions(-)
>>>
>>> diff --git a/include/ust/core.h b/include/ust/core.h
>>> index 0172614..e23224a 100644
>>> --- a/include/ust/core.h
>>> +++ b/include/ust/core.h
>>> @@ -141,7 +141,7 @@ static __inline__ int get_count_order(unsigned int count)
>>>    	return order;
>>>    }
>>>
>>> -#define container_of(ptr, type, member) ({                      \
>>> +#define __ust_container_of(ptr, type, member) ({                      \
>>>            const typeof( ((type *)0)->member ) *__mptr = (ptr);    \
>>>            (type *)( (char *)__mptr - offsetof(type,member) );})
>>>
>>> diff --git a/include/ust/kcompat/simple.h b/include/ust/kcompat/simple.h
>>> index 762d802..38a9be5 100644
>>> --- a/include/ust/kcompat/simple.h
>>> +++ b/include/ust/kcompat/simple.h
>>> @@ -21,18 +21,6 @@
>>>    #ifndef KCOMPAT_SIMPLE_H
>>>    #define KCOMPAT_SIMPLE_H
>>>
>>> -/**
>>> - * container_of - cast a member of a structure out to the containing structure
>>> - * @ptr:	the pointer to the member.
>>> - * @type:	the type of the container struct this is embedded in.
>>> - * @member:	the name of the member within the struct.
>>> - *
>>> - */
>>> -#define container_of(ptr, type, member) ({			\
>>> -	const typeof( ((type *)0)->member ) *__mptr = (ptr);	\
>>> -	(type *)( (char *)__mptr - offsetof(type,member) );})
>>> -
>>> -
>>>    /* libkcompat: from rcupdate.h */
>>>
>>>    struct rcu_head {
>>> diff --git a/libust/buffers.c b/libust/buffers.c
>>> index 374ec61..8a2b90a 100644
>>> --- a/libust/buffers.c
>>> +++ b/libust/buffers.c
>>> @@ -203,7 +203,7 @@ int ust_buffers_create_buf(struct ust_channel *channel, int cpu)
>>>
>>>    static void ust_buffers_destroy_channel(struct kref *kref)
>>>    {
>>> -	struct ust_channel *chan = container_of(kref, struct ust_channel, kref);
>>> +	struct ust_channel *chan = __ust_container_of(kref, struct ust_channel, kref);
>>>    	free(chan);
>>>    }
>>>
>>> @@ -225,7 +225,7 @@ static void ust_buffers_destroy_buf(struct ust_buffer *buf)
>>>    /* called from kref_put */
>>>    static void ust_buffers_remove_buf(struct kref *kref)
>>>    {
>>> -	struct ust_buffer *buf = container_of(kref, struct ust_buffer, kref);
>>> +	struct ust_buffer *buf = __ust_container_of(kref, struct ust_buffer, kref);
>>>    	ust_buffers_destroy_buf(buf);
>>>    }
>>>
>>> @@ -592,7 +592,7 @@ static void ltt_relay_print_buffer_errors(struct ust_channel *channel, int cpu)
>>>
>>>    static void ltt_relay_release_channel(struct kref *kref)
>>>    {
>>> -	struct ust_channel *ltt_chan = container_of(kref,
>>> +	struct ust_channel *ltt_chan = __ust_container_of(kref,
>>>    			struct ust_channel, kref);
>>>    	free(ltt_chan->buf);
>>>    }
>>> diff --git a/libust/channels.c b/libust/channels.c
>>> index df1a972..4de54a9 100644
>>> --- a/libust/channels.c
>>> +++ b/libust/channels.c
>>> @@ -66,7 +66,7 @@ static struct ltt_channel_setting *lookup_channel(const char *name)
>>>     */
>>>    static void release_channel_setting(struct kref *kref)
>>>    {
>>> -	struct ltt_channel_setting *setting = container_of(kref,
>>> +	struct ltt_channel_setting *setting = __ust_container_of(kref,
>>>    		struct ltt_channel_setting, kref);
>>>    	struct ltt_channel_setting *iter;
>>>
>>> diff --git a/libust/marker.c b/libust/marker.c
>>> index dbabf3e..761785a 100644
>>> --- a/libust/marker.c
>>> +++ b/libust/marker.c
>>> @@ -235,7 +235,7 @@ static notrace void marker_probe_cb_noarg(const struct marker *mdata,
>>>
>>>    static void free_old_closure(struct rcu_head *head)
>>>    {
>>> -	struct marker_entry *entry = container_of(head,
>>> +	struct marker_entry *entry = __ust_container_of(head,
>>>    		struct marker_entry, rcu);
>>>    	free(entry->oldptr);
>>>    	/* Make sure we free the data before setting the pending flag to 0 */
>>> diff --git a/libust/tracepoint.c b/libust/tracepoint.c
>>> index 16b4819..63ae17d 100644
>>> --- a/libust/tracepoint.c
>>> +++ b/libust/tracepoint.c
>>> @@ -87,7 +87,7 @@ static inline void *allocate_probes(int count)
>>>    static inline void release_probes(void *old)
>>>    {
>>>    	if (old) {
>>> -		struct tp_probes *tp_probes = container_of(old,
>>> +		struct tp_probes *tp_probes = __ust_container_of(old,
>>>    			struct tp_probes, probes[0]);
>>>    //ust//		call_rcu_sched(&tp_probes->u.rcu, rcu_free_old_probes);
>>>    		synchronize_rcu();
>>> @@ -427,7 +427,7 @@ static void tracepoint_add_old_probes(void *old)
>>>    {
>>>    	need_update = 1;
>>>    	if (old) {
>>> -		struct tp_probes *tp_probes = container_of(old,
>>> +		struct tp_probes *tp_probes = __ust_container_of(old,
>>>    			struct tp_probes, probes[0]);
>>>    		list_add(&tp_probes->u.list,&old_probes);
>>>    	}
>>> diff --git a/libust/tracer.c b/libust/tracer.c
>>> index 8c3f774..ed5e329 100644
>>> --- a/libust/tracer.c
>>> +++ b/libust/tracer.c
>>> @@ -337,7 +337,7 @@ void ltt_release_transport(struct kref *kref)
>>>     */
>>>    void ltt_release_trace(struct kref *kref)
>>>    {
>>> -	struct ust_trace *trace = container_of(kref,
>>> +	struct ust_trace *trace = __ust_container_of(kref,
>>>    			struct ust_trace, kref);
>>>    	ltt_channels_trace_free(trace->channels);
>>>    	free(trace);
>>
>> --
>> David Goulet
>> LTTng project, DORSAL Lab.
>>
>> PGP/GPG : 1024D/16BD8563
>> BE3C 672B 9331 9796 291A  14C6 4AF7 C14B 16BD 8563
>>
>> _______________________________________________
>> 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.

PGP/GPG : 1024D/16BD8563
BE3C 672B 9331 9796 291A  14C6 4AF7 C14B 16BD 8563



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

* [ltt-dev] [UST PATCH] New naming convention for macro container_of
  2010-09-23 16:46 ` Mathieu Desnoyers
@ 2010-09-23 17:29   ` Nils Carlson
  0 siblings, 0 replies; 8+ messages in thread
From: Nils Carlson @ 2010-09-23 17:29 UTC (permalink / raw)


On Sep 23, 2010, at 6:46 PM, Mathieu Desnoyers wrote:

> * David Goulet (david.goulet at polymtl.ca) wrote:
>> This is the first patch introducing the new naming convention
>> for the UST tracer. To prevent namespace pollution, __ust_ prefix
>> is added to visible symbol in global header files.
>
> I'd use the "ust_" prefix rather than "__ust_", because __ is usually
> reserved for the compiler (and makes function names needlessly long).
>
What about just one underscore then? Here is what the GNU coding  
conventions have to say:

External symbols that are not documented entry points for the user  
should have names beginning with ?_?. The ?_? should be followed by  
the chosen name prefix for the library, to prevent collisions with  
other libraries. These can go in the same files with user entry points  
if you like.

http://www.gnu.org/prep/standards/standards.html#Libraries

The idea is to mark these out as non-external while everything that is  
external is prefixed with ust_ (we might want to consider this for  
trace_mark etc as well? Though that should be carefully considered I  
think.)

/Nils

> Mathieu
>
>>
>> For this patch, only the container_of macro is renamed and the
>> redundant definition in kcompat/simple.h is removed.
>>
>> Signed-off-by: David Goulet <david.goulet at polymtl.ca>
>> ---
>> include/ust/core.h           |    2 +-
>> include/ust/kcompat/simple.h |   12 ------------
>> libust/buffers.c             |    6 +++---
>> libust/channels.c            |    2 +-
>> libust/marker.c              |    2 +-
>> libust/tracepoint.c          |    4 ++--
>> libust/tracer.c              |    2 +-
>> 7 files changed, 9 insertions(+), 21 deletions(-)
>>
>> diff --git a/include/ust/core.h b/include/ust/core.h
>> index 0172614..e23224a 100644
>> --- a/include/ust/core.h
>> +++ b/include/ust/core.h
>> @@ -141,7 +141,7 @@ static __inline__ int get_count_order(unsigned  
>> int count)
>> 	return order;
>> }
>>
>> -#define container_of(ptr, type, member) ({                      \
>> +#define __ust_container_of(ptr, type, member)  
>> ({                      \
>>         const typeof( ((type *)0)->member ) *__mptr = (ptr);    \
>>         (type *)( (char *)__mptr - offsetof(type,member) );})
>>
>> diff --git a/include/ust/kcompat/simple.h b/include/ust/kcompat/ 
>> simple.h
>> index 762d802..38a9be5 100644
>> --- a/include/ust/kcompat/simple.h
>> +++ b/include/ust/kcompat/simple.h
>> @@ -21,18 +21,6 @@
>> #ifndef KCOMPAT_SIMPLE_H
>> #define KCOMPAT_SIMPLE_H
>>
>> -/**
>> - * container_of - cast a member of a structure out to the  
>> containing structure
>> - * @ptr:	the pointer to the member.
>> - * @type:	the type of the container struct this is embedded in.
>> - * @member:	the name of the member within the struct.
>> - *
>> - */
>> -#define container_of(ptr, type, member) ({			\
>> -	const typeof( ((type *)0)->member ) *__mptr = (ptr);	\
>> -	(type *)( (char *)__mptr - offsetof(type,member) );})
>> -
>> -
>> /* libkcompat: from rcupdate.h */
>>
>> struct rcu_head {
>> diff --git a/libust/buffers.c b/libust/buffers.c
>> index 374ec61..8a2b90a 100644
>> --- a/libust/buffers.c
>> +++ b/libust/buffers.c
>> @@ -203,7 +203,7 @@ int ust_buffers_create_buf(struct ust_channel  
>> *channel, int cpu)
>>
>> static void ust_buffers_destroy_channel(struct kref *kref)
>> {
>> -	struct ust_channel *chan = container_of(kref, struct ust_channel,  
>> kref);
>> +	struct ust_channel *chan = __ust_container_of(kref, struct  
>> ust_channel, kref);
>> 	free(chan);
>> }
>>
>> @@ -225,7 +225,7 @@ static void ust_buffers_destroy_buf(struct  
>> ust_buffer *buf)
>> /* called from kref_put */
>> static void ust_buffers_remove_buf(struct kref *kref)
>> {
>> -	struct ust_buffer *buf = container_of(kref, struct ust_buffer,  
>> kref);
>> +	struct ust_buffer *buf = __ust_container_of(kref, struct  
>> ust_buffer, kref);
>> 	ust_buffers_destroy_buf(buf);
>> }
>>
>> @@ -592,7 +592,7 @@ static void  
>> ltt_relay_print_buffer_errors(struct ust_channel *channel, int cpu)
>>
>> static void ltt_relay_release_channel(struct kref *kref)
>> {
>> -	struct ust_channel *ltt_chan = container_of(kref,
>> +	struct ust_channel *ltt_chan = __ust_container_of(kref,
>> 			struct ust_channel, kref);
>> 	free(ltt_chan->buf);
>> }
>> diff --git a/libust/channels.c b/libust/channels.c
>> index df1a972..4de54a9 100644
>> --- a/libust/channels.c
>> +++ b/libust/channels.c
>> @@ -66,7 +66,7 @@ static struct ltt_channel_setting  
>> *lookup_channel(const char *name)
>>  */
>> static void release_channel_setting(struct kref *kref)
>> {
>> -	struct ltt_channel_setting *setting = container_of(kref,
>> +	struct ltt_channel_setting *setting = __ust_container_of(kref,
>> 		struct ltt_channel_setting, kref);
>> 	struct ltt_channel_setting *iter;
>>
>> diff --git a/libust/marker.c b/libust/marker.c
>> index dbabf3e..761785a 100644
>> --- a/libust/marker.c
>> +++ b/libust/marker.c
>> @@ -235,7 +235,7 @@ static notrace void marker_probe_cb_noarg(const  
>> struct marker *mdata,
>>
>> static void free_old_closure(struct rcu_head *head)
>> {
>> -	struct marker_entry *entry = container_of(head,
>> +	struct marker_entry *entry = __ust_container_of(head,
>> 		struct marker_entry, rcu);
>> 	free(entry->oldptr);
>> 	/* Make sure we free the data before setting the pending flag to 0  
>> */
>> diff --git a/libust/tracepoint.c b/libust/tracepoint.c
>> index 16b4819..63ae17d 100644
>> --- a/libust/tracepoint.c
>> +++ b/libust/tracepoint.c
>> @@ -87,7 +87,7 @@ static inline void *allocate_probes(int count)
>> static inline void release_probes(void *old)
>> {
>> 	if (old) {
>> -		struct tp_probes *tp_probes = container_of(old,
>> +		struct tp_probes *tp_probes = __ust_container_of(old,
>> 			struct tp_probes, probes[0]);
>> //ust//		call_rcu_sched(&tp_probes->u.rcu, rcu_free_old_probes);
>> 		synchronize_rcu();
>> @@ -427,7 +427,7 @@ static void tracepoint_add_old_probes(void *old)
>> {
>> 	need_update = 1;
>> 	if (old) {
>> -		struct tp_probes *tp_probes = container_of(old,
>> +		struct tp_probes *tp_probes = __ust_container_of(old,
>> 			struct tp_probes, probes[0]);
>> 		list_add(&tp_probes->u.list, &old_probes);
>> 	}
>> diff --git a/libust/tracer.c b/libust/tracer.c
>> index 8c3f774..ed5e329 100644
>> --- a/libust/tracer.c
>> +++ b/libust/tracer.c
>> @@ -337,7 +337,7 @@ void ltt_release_transport(struct kref *kref)
>>  */
>> void ltt_release_trace(struct kref *kref)
>> {
>> -	struct ust_trace *trace = container_of(kref,
>> +	struct ust_trace *trace = __ust_container_of(kref,
>> 			struct ust_trace, kref);
>> 	ltt_channels_trace_free(trace->channels);
>> 	free(trace);
>> -- 
>> 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
>
> _______________________________________________
> 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] 8+ messages in thread

* [ltt-dev] [UST PATCH] New naming convention for macro container_of
  2010-09-23 17:16     ` David Goulet
@ 2010-09-23 17:31       ` Mathieu Desnoyers
  0 siblings, 0 replies; 8+ messages in thread
From: Mathieu Desnoyers @ 2010-09-23 17:31 UTC (permalink / raw)


* David Goulet (david.goulet at polymtl.ca) wrote:
>
>
> On 10-09-23 01:07 PM, Mathieu Desnoyers wrote:
>> * David Goulet (david.goulet at polymtl.ca) wrote:
>>> Following this.
>>>
>>> We are talking here about a major overhaul of naming convention. Just
>>> for trace_mark, basically, every symbols in marker.h and following
>>> headers have to be refactor with the __ust_ (or ust_) prefix :
>>>
>>> struct marker -->  struct __ust_marker, and so on for types, macros and
>>> functions. This is quite a huge work in term of "syntax changes" and, as
>>> Mathieu pointed out, *long* name. I'm perfectly convince that this is
>>> necessary but if so now, should we than consider removing old LTTng
>>> comments, rename all ltt_* function to ust_*, etc... and so doing a
>>> complete cleanup once and for all ?
>>
>> Yes.
>>
>
> I'll try to put some time on that each file at a time. Good
>
>> Although for the code that is shared between the Linux kernel and UST,
>> (e.g. struct marker), I would be tempted to keep the name as is.
>>
>
> As a funny fact, few months ago, without knowing that much about UST, I  
> had a redefinition error on exactly struct marker in one of my program  
> that I wanted to trace with UST... It's kind of annoying when you have  
> to refactor all your code because of the tracer...
>
> What's the main reason you are tempted to keep the name? (since UST come  
> from LTTng, it's easier for porting new feature from ltt to ust...?)

Yep, exactly.

Mathieu

>
> Thanks
> David
>
>> Mathieu
>>
>>>
>>> Cheers
>>> David
>>>
>>> On 10-09-23 12:26 PM, David Goulet wrote:
>>>> This is the first patch introducing the new naming convention
>>>> for the UST tracer. To prevent namespace pollution, __ust_ prefix
>>>> is added to visible symbol in global header files.
>>>>
>>>> For this patch, only the container_of macro is renamed and the
>>>> redundant definition in kcompat/simple.h is removed.
>>>>
>>>> Signed-off-by: David Goulet<david.goulet at polymtl.ca>
>>>> ---
>>>>    include/ust/core.h           |    2 +-
>>>>    include/ust/kcompat/simple.h |   12 ------------
>>>>    libust/buffers.c             |    6 +++---
>>>>    libust/channels.c            |    2 +-
>>>>    libust/marker.c              |    2 +-
>>>>    libust/tracepoint.c          |    4 ++--
>>>>    libust/tracer.c              |    2 +-
>>>>    7 files changed, 9 insertions(+), 21 deletions(-)
>>>>
>>>> diff --git a/include/ust/core.h b/include/ust/core.h
>>>> index 0172614..e23224a 100644
>>>> --- a/include/ust/core.h
>>>> +++ b/include/ust/core.h
>>>> @@ -141,7 +141,7 @@ static __inline__ int get_count_order(unsigned int count)
>>>>    	return order;
>>>>    }
>>>>
>>>> -#define container_of(ptr, type, member) ({                      \
>>>> +#define __ust_container_of(ptr, type, member) ({                      \
>>>>            const typeof( ((type *)0)->member ) *__mptr = (ptr);    \
>>>>            (type *)( (char *)__mptr - offsetof(type,member) );})
>>>>
>>>> diff --git a/include/ust/kcompat/simple.h b/include/ust/kcompat/simple.h
>>>> index 762d802..38a9be5 100644
>>>> --- a/include/ust/kcompat/simple.h
>>>> +++ b/include/ust/kcompat/simple.h
>>>> @@ -21,18 +21,6 @@
>>>>    #ifndef KCOMPAT_SIMPLE_H
>>>>    #define KCOMPAT_SIMPLE_H
>>>>
>>>> -/**
>>>> - * container_of - cast a member of a structure out to the containing structure
>>>> - * @ptr:	the pointer to the member.
>>>> - * @type:	the type of the container struct this is embedded in.
>>>> - * @member:	the name of the member within the struct.
>>>> - *
>>>> - */
>>>> -#define container_of(ptr, type, member) ({			\
>>>> -	const typeof( ((type *)0)->member ) *__mptr = (ptr);	\
>>>> -	(type *)( (char *)__mptr - offsetof(type,member) );})
>>>> -
>>>> -
>>>>    /* libkcompat: from rcupdate.h */
>>>>
>>>>    struct rcu_head {
>>>> diff --git a/libust/buffers.c b/libust/buffers.c
>>>> index 374ec61..8a2b90a 100644
>>>> --- a/libust/buffers.c
>>>> +++ b/libust/buffers.c
>>>> @@ -203,7 +203,7 @@ int ust_buffers_create_buf(struct ust_channel *channel, int cpu)
>>>>
>>>>    static void ust_buffers_destroy_channel(struct kref *kref)
>>>>    {
>>>> -	struct ust_channel *chan = container_of(kref, struct ust_channel, kref);
>>>> +	struct ust_channel *chan = __ust_container_of(kref, struct ust_channel, kref);
>>>>    	free(chan);
>>>>    }
>>>>
>>>> @@ -225,7 +225,7 @@ static void ust_buffers_destroy_buf(struct ust_buffer *buf)
>>>>    /* called from kref_put */
>>>>    static void ust_buffers_remove_buf(struct kref *kref)
>>>>    {
>>>> -	struct ust_buffer *buf = container_of(kref, struct ust_buffer, kref);
>>>> +	struct ust_buffer *buf = __ust_container_of(kref, struct ust_buffer, kref);
>>>>    	ust_buffers_destroy_buf(buf);
>>>>    }
>>>>
>>>> @@ -592,7 +592,7 @@ static void ltt_relay_print_buffer_errors(struct ust_channel *channel, int cpu)
>>>>
>>>>    static void ltt_relay_release_channel(struct kref *kref)
>>>>    {
>>>> -	struct ust_channel *ltt_chan = container_of(kref,
>>>> +	struct ust_channel *ltt_chan = __ust_container_of(kref,
>>>>    			struct ust_channel, kref);
>>>>    	free(ltt_chan->buf);
>>>>    }
>>>> diff --git a/libust/channels.c b/libust/channels.c
>>>> index df1a972..4de54a9 100644
>>>> --- a/libust/channels.c
>>>> +++ b/libust/channels.c
>>>> @@ -66,7 +66,7 @@ static struct ltt_channel_setting *lookup_channel(const char *name)
>>>>     */
>>>>    static void release_channel_setting(struct kref *kref)
>>>>    {
>>>> -	struct ltt_channel_setting *setting = container_of(kref,
>>>> +	struct ltt_channel_setting *setting = __ust_container_of(kref,
>>>>    		struct ltt_channel_setting, kref);
>>>>    	struct ltt_channel_setting *iter;
>>>>
>>>> diff --git a/libust/marker.c b/libust/marker.c
>>>> index dbabf3e..761785a 100644
>>>> --- a/libust/marker.c
>>>> +++ b/libust/marker.c
>>>> @@ -235,7 +235,7 @@ static notrace void marker_probe_cb_noarg(const struct marker *mdata,
>>>>
>>>>    static void free_old_closure(struct rcu_head *head)
>>>>    {
>>>> -	struct marker_entry *entry = container_of(head,
>>>> +	struct marker_entry *entry = __ust_container_of(head,
>>>>    		struct marker_entry, rcu);
>>>>    	free(entry->oldptr);
>>>>    	/* Make sure we free the data before setting the pending flag to 0 */
>>>> diff --git a/libust/tracepoint.c b/libust/tracepoint.c
>>>> index 16b4819..63ae17d 100644
>>>> --- a/libust/tracepoint.c
>>>> +++ b/libust/tracepoint.c
>>>> @@ -87,7 +87,7 @@ static inline void *allocate_probes(int count)
>>>>    static inline void release_probes(void *old)
>>>>    {
>>>>    	if (old) {
>>>> -		struct tp_probes *tp_probes = container_of(old,
>>>> +		struct tp_probes *tp_probes = __ust_container_of(old,
>>>>    			struct tp_probes, probes[0]);
>>>>    //ust//		call_rcu_sched(&tp_probes->u.rcu, rcu_free_old_probes);
>>>>    		synchronize_rcu();
>>>> @@ -427,7 +427,7 @@ static void tracepoint_add_old_probes(void *old)
>>>>    {
>>>>    	need_update = 1;
>>>>    	if (old) {
>>>> -		struct tp_probes *tp_probes = container_of(old,
>>>> +		struct tp_probes *tp_probes = __ust_container_of(old,
>>>>    			struct tp_probes, probes[0]);
>>>>    		list_add(&tp_probes->u.list,&old_probes);
>>>>    	}
>>>> diff --git a/libust/tracer.c b/libust/tracer.c
>>>> index 8c3f774..ed5e329 100644
>>>> --- a/libust/tracer.c
>>>> +++ b/libust/tracer.c
>>>> @@ -337,7 +337,7 @@ void ltt_release_transport(struct kref *kref)
>>>>     */
>>>>    void ltt_release_trace(struct kref *kref)
>>>>    {
>>>> -	struct ust_trace *trace = container_of(kref,
>>>> +	struct ust_trace *trace = __ust_container_of(kref,
>>>>    			struct ust_trace, kref);
>>>>    	ltt_channels_trace_free(trace->channels);
>>>>    	free(trace);
>>>
>>> --
>>> David Goulet
>>> LTTng project, DORSAL Lab.
>>>
>>> PGP/GPG : 1024D/16BD8563
>>> BE3C 672B 9331 9796 291A  14C6 4AF7 C14B 16BD 8563
>>>
>>> _______________________________________________
>>> 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.
>
> PGP/GPG : 1024D/16BD8563
> BE3C 672B 9331 9796 291A  14C6 4AF7 C14B 16BD 8563
>

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




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

* [ltt-dev] [UST PATCH] New naming convention for macro container_of
  2010-09-23 16:48 ` David Goulet
  2010-09-23 17:07   ` Mathieu Desnoyers
@ 2010-09-26  4:46   ` Pierre-Marc Fournier
  1 sibling, 0 replies; 8+ messages in thread
From: Pierre-Marc Fournier @ 2010-09-26  4:46 UTC (permalink / raw)


For me, there is no question, the ltt_* functions should be renamed to 
ust_*.

Concerning the comments: where commented ltt code is left, it means that 
minimal work was done on that code following the initial port. I would 
recommend auditing that code to see if it makes sense to reactivate some 
of it for ust and if not, deleting it.

pmf

On 09/23/2010 12:48 PM, David Goulet wrote:
> Following this.
>
> We are talking here about a major overhaul of naming convention. Just
> for trace_mark, basically, every symbols in marker.h and following
> headers have to be refactor with the __ust_ (or ust_) prefix :
>
> struct marker --> struct __ust_marker, and so on for types, macros and
> functions. This is quite a huge work in term of "syntax changes" and, as
> Mathieu pointed out, *long* name. I'm perfectly convince that this is
> necessary but if so now, should we than consider removing old LTTng
> comments, rename all ltt_* function to ust_*, etc... and so doing a
> complete cleanup once and for all ?
>
> Cheers
> David
>
> On 10-09-23 12:26 PM, David Goulet wrote:
>> This is the first patch introducing the new naming convention
>> for the UST tracer. To prevent namespace pollution, __ust_ prefix
>> is added to visible symbol in global header files.
>>
>> For this patch, only the container_of macro is renamed and the
>> redundant definition in kcompat/simple.h is removed.
>>
>> Signed-off-by: David Goulet<david.goulet at polymtl.ca>
>> ---
>> include/ust/core.h | 2 +-
>> include/ust/kcompat/simple.h | 12 ------------
>> libust/buffers.c | 6 +++---
>> libust/channels.c | 2 +-
>> libust/marker.c | 2 +-
>> libust/tracepoint.c | 4 ++--
>> libust/tracer.c | 2 +-
>> 7 files changed, 9 insertions(+), 21 deletions(-)
>>
>> diff --git a/include/ust/core.h b/include/ust/core.h
>> index 0172614..e23224a 100644
>> --- a/include/ust/core.h
>> +++ b/include/ust/core.h
>> @@ -141,7 +141,7 @@ static __inline__ int get_count_order(unsigned int
>> count)
>> return order;
>> }
>>
>> -#define container_of(ptr, type, member) ({ \
>> +#define __ust_container_of(ptr, type, member) ({ \
>> const typeof( ((type *)0)->member ) *__mptr = (ptr); \
>> (type *)( (char *)__mptr - offsetof(type,member) );})
>>
>> diff --git a/include/ust/kcompat/simple.h b/include/ust/kcompat/simple.h
>> index 762d802..38a9be5 100644
>> --- a/include/ust/kcompat/simple.h
>> +++ b/include/ust/kcompat/simple.h
>> @@ -21,18 +21,6 @@
>> #ifndef KCOMPAT_SIMPLE_H
>> #define KCOMPAT_SIMPLE_H
>>
>> -/**
>> - * container_of - cast a member of a structure out to the containing
>> structure
>> - * @ptr: the pointer to the member.
>> - * @type: the type of the container struct this is embedded in.
>> - * @member: the name of the member within the struct.
>> - *
>> - */
>> -#define container_of(ptr, type, member) ({ \
>> - const typeof( ((type *)0)->member ) *__mptr = (ptr); \
>> - (type *)( (char *)__mptr - offsetof(type,member) );})
>> -
>> -
>> /* libkcompat: from rcupdate.h */
>>
>> struct rcu_head {
>> diff --git a/libust/buffers.c b/libust/buffers.c
>> index 374ec61..8a2b90a 100644
>> --- a/libust/buffers.c
>> +++ b/libust/buffers.c
>> @@ -203,7 +203,7 @@ int ust_buffers_create_buf(struct ust_channel
>> *channel, int cpu)
>>
>> static void ust_buffers_destroy_channel(struct kref *kref)
>> {
>> - struct ust_channel *chan = container_of(kref, struct ust_channel,
>> kref);
>> + struct ust_channel *chan = __ust_container_of(kref, struct
>> ust_channel, kref);
>> free(chan);
>> }
>>
>> @@ -225,7 +225,7 @@ static void ust_buffers_destroy_buf(struct
>> ust_buffer *buf)
>> /* called from kref_put */
>> static void ust_buffers_remove_buf(struct kref *kref)
>> {
>> - struct ust_buffer *buf = container_of(kref, struct ust_buffer, kref);
>> + struct ust_buffer *buf = __ust_container_of(kref, struct ust_buffer,
>> kref);
>> ust_buffers_destroy_buf(buf);
>> }
>>
>> @@ -592,7 +592,7 @@ static void ltt_relay_print_buffer_errors(struct
>> ust_channel *channel, int cpu)
>>
>> static void ltt_relay_release_channel(struct kref *kref)
>> {
>> - struct ust_channel *ltt_chan = container_of(kref,
>> + struct ust_channel *ltt_chan = __ust_container_of(kref,
>> struct ust_channel, kref);
>> free(ltt_chan->buf);
>> }
>> diff --git a/libust/channels.c b/libust/channels.c
>> index df1a972..4de54a9 100644
>> --- a/libust/channels.c
>> +++ b/libust/channels.c
>> @@ -66,7 +66,7 @@ static struct ltt_channel_setting
>> *lookup_channel(const char *name)
>> */
>> static void release_channel_setting(struct kref *kref)
>> {
>> - struct ltt_channel_setting *setting = container_of(kref,
>> + struct ltt_channel_setting *setting = __ust_container_of(kref,
>> struct ltt_channel_setting, kref);
>> struct ltt_channel_setting *iter;
>>
>> diff --git a/libust/marker.c b/libust/marker.c
>> index dbabf3e..761785a 100644
>> --- a/libust/marker.c
>> +++ b/libust/marker.c
>> @@ -235,7 +235,7 @@ static notrace void marker_probe_cb_noarg(const
>> struct marker *mdata,
>>
>> static void free_old_closure(struct rcu_head *head)
>> {
>> - struct marker_entry *entry = container_of(head,
>> + struct marker_entry *entry = __ust_container_of(head,
>> struct marker_entry, rcu);
>> free(entry->oldptr);
>> /* Make sure we free the data before setting the pending flag to 0 */
>> diff --git a/libust/tracepoint.c b/libust/tracepoint.c
>> index 16b4819..63ae17d 100644
>> --- a/libust/tracepoint.c
>> +++ b/libust/tracepoint.c
>> @@ -87,7 +87,7 @@ static inline void *allocate_probes(int count)
>> static inline void release_probes(void *old)
>> {
>> if (old) {
>> - struct tp_probes *tp_probes = container_of(old,
>> + struct tp_probes *tp_probes = __ust_container_of(old,
>> struct tp_probes, probes[0]);
>> //ust// call_rcu_sched(&tp_probes->u.rcu, rcu_free_old_probes);
>> synchronize_rcu();
>> @@ -427,7 +427,7 @@ static void tracepoint_add_old_probes(void *old)
>> {
>> need_update = 1;
>> if (old) {
>> - struct tp_probes *tp_probes = container_of(old,
>> + struct tp_probes *tp_probes = __ust_container_of(old,
>> struct tp_probes, probes[0]);
>> list_add(&tp_probes->u.list,&old_probes);
>> }
>> diff --git a/libust/tracer.c b/libust/tracer.c
>> index 8c3f774..ed5e329 100644
>> --- a/libust/tracer.c
>> +++ b/libust/tracer.c
>> @@ -337,7 +337,7 @@ void ltt_release_transport(struct kref *kref)
>> */
>> void ltt_release_trace(struct kref *kref)
>> {
>> - struct ust_trace *trace = container_of(kref,
>> + struct ust_trace *trace = __ust_container_of(kref,
>> struct ust_trace, kref);
>> ltt_channels_trace_free(trace->channels);
>> free(trace);
>




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

end of thread, other threads:[~2010-09-26  4:46 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-09-23 16:26 [ltt-dev] [UST PATCH] New naming convention for macro container_of David Goulet
2010-09-23 16:46 ` Mathieu Desnoyers
2010-09-23 17:29   ` Nils Carlson
2010-09-23 16:48 ` David Goulet
2010-09-23 17:07   ` Mathieu Desnoyers
2010-09-23 17:16     ` David Goulet
2010-09-23 17:31       ` Mathieu Desnoyers
2010-09-26  4:46   ` Pierre-Marc Fournier

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