Re: [PATCH 1/2 v2] tracing/events: provide string with undefinedsize support

[Frederic Weisbecker]

On Fri, Apr 17, 2009 at 06:39:56AM -0400, Steven Rostedt wrote:
> On Fri, 17 Apr 2009, Steven Rostedt wrote:
> > 
> > But that said, one could still do what you are suggesting. The "__ending" 
> > wart still needs to be there, since it must happen at the end of the 
> > struct.
> > 
> > 
> >         TP_STRUCT__entry(
> >                 __field(int, str2loc)
> >                 __field(int, str3loc)
> > 		__string(str1)
> > 		__string(str2)
> >                 __ending_string(data, str3)
> >         ),
> >         TP_fast_assign(
> > 		__entry->str2loc = strlen(str1);
> > 		__entry->str3loc = strlen(str2) + __entry->str2loc;
> >                 strcpy(__entry->data, str1);
> > 		strcpy(__entry->data + __entry->str2loc, str2)
> > 		strcpy(__entry->data + __entry->str3loc, str3)
> >         ),
> >         TP_printk("str1:%s str2:%s str3:%s\n",
> > 		__entry->data,
> > 		__entry->data + __entry->str2loc,
> > 		__entry->date + __entry->str3loc)
> > 
> > 
> > For this to work, we need to just add a define for the __string macro for 
> > the reservation:
> > 
> > #define __string(x)  __str_size_ += strlen(x) + 1;
> 
> Hmm, thinking about this more, we could do...
> 
> 
> Make all structs end with:

> 
> 	char __str_data__[0];
> 
> 
> then we could just have __string(dec, param), ie.
> 
> 
> 	__string(str1, param_str1)
> 	__string(str2, param_str2)
> 	__string(srt3, param_str3)
> 
> But instead of defining string as a character, it would just be an index 
> into the __str_data_.
> 
> #define __string(dec, param)	int dec;
> 
> 
> We can break up the TRACE_EVENT macro again to create the declaration:
> 
> #undef everything and make empty macros of all we don't use
> 
> #define __string(dec, str)	int __str_loc_##dec;
> 
> #define TRACE_EVENT(...)	\
> static void ftrace_raw_event_##call(proto)	\
> {	\
> 	struct ring_buffer_event *event;	\
> 	struct ftrace_raw_##call *entry;	\
> 	unsigned long flags;			\
> 	int pc;					\
> 	int _str_size_ = 0;			\
> 						\
> 	tstruct
> 
> #include the file
> 
> #undef __string
> #define __string(str, param)	__str_loc_##str = _str_size_; \
> 				_str_size_ += strlen(param) + 1;
> 
> #define TRACE_EVENT(...)
> 	tstruct
> 
> #include the file again


While I'm trying to implement this, I'm discovering that it's
impossible, unless we have only one TRACE_EVENT in the file we are
including, otherwise we would end up with interleaving functions:

 static void ftrace_raw_event_event1(proto)
 {
       struct ring_buffer_event *event;
       struct ftrace_raw_event1 *entry;
       unsigned long flags;
       int pc;
       int _str_size_ = 0;

       tstruct

 static void ftrace_raw_event_event2(proto)     
 {      
       struct ring_buffer_event *event;         
       struct ftrace_raw_event2 *entry;   
       unsigned long flags;                     
       int pc;                                  
       int _str_size_ = 0;                      

       tstruct


So we still need something inside the fast_assign to do the
assignment job.

Something like the following macro will suffice:

#define	__assign_string(field, src)	\
	strcpy(entry->__ending_str + __str_loc_##field, src);

That's not a big deal, we just actually need this in TP__fast_assign()
and then we are done.

And also:


> The tstruct would do the assign for the user.
> 
> Then this is what a TRACE_EVENT would look like:
> 
> TRACE_EVENT(my_event,
> 
> 	TP_PROTO(char *str1, char *str2, char *str3),
> 
> 	TP_ARGS(str1, str2, str3),
> 
> 	TP_STRUCT__entry(
> 		__string(str1, str1)
> 		__string(str2, str2)
> 		__string(str3, str3)
> 	),
> 
> 	TP_fast_assign(
> 		/* empty, strings are automated */
> 	),
> 
> 	TP_printk("str1:%s str2:%s str3:%s\n",
> 		__entry->__str_data__ + __entry->str1,
> 		__entry->__str_data__ + __entry->str2,
> 		__entry->__str_data__ + __entry->str3)



A simple macro here to simplify that for the users:

#define __get_str(item)		\
	__entry->__ending_str + __entry->__str_loc_#item

would be much more convenient.

Anyway, I'll submit it and will wait for your comments.

Frederic.


> );
> 
> 
> /me feels more evil than ever ;-)
> 
> -- Steve
> 
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/