Re: [RFC PATCH] libtraceevent: Add tep_print_selected_fields()

[Steven Rostedt <rostedt@goodmis.org>]

On Tue, 3 Aug 2021 15:42:02 +0300
Yordan Karadzhov <y.karadz@gmail.com> wrote:

> On 2.08.21 г. 19:29, Steven Rostedt wrote:
> > What I was talking about is to change tep_print_field() to do something like:
> > 
> > Take the current tep_print_field() and turn it into static _tep_print_field().
> > 
> >   [ Not even compiled tested ]
> >   
> 
> Hi Steven,
> 
> I am not able to make sense from the code below.
> 
> > void tep_print_field(struct trace_seq *s, void *data,
> > 		     struct tep_format_field *field)
> > {
> > 	struct tep_event = field->event;
> > 	struct tep_print_parse = event->print_fmt.print_cache;
> > 	struct tep_handle *tep = event->tep;
> > 	unsigned int offset, len;
> > 
> > 	if (event->flags & TEP_EVENT_FL_FAILED)
> > 		goto out;
> > 
> > 	if (field->flags & TEP_FIELD_IS_DYNAMIC)
> > 		dynamic_offset(tep, field, data, &offset, &len);
> > 
> > 	for (;parse; parse = parse->next) {  
> 
> You need
>          if (!parse->arg)
> 			continue;

Sure. Like I said, I never even compiled this ;-)

> 
> > 		if (parse->type == PRINT_FMT_STRING)
> > 			continue;
> > 		if (parse->arg->type != TEP_PRINT_FIELD)
> > 			continue;  
> 
> I can't understand the idea of those two checks. I printed the values and they don't seem to have any selective power.

Well, the print_cache stores portions of the print fmt format string. Where if you have something like:


  print fmt: "fd: 0x%08lx, offset: 0x%08lx, count: 0x%08lx", ((unsigned long)(REC->fd)), ((unsigned long)(REC->offset)), ((unsigned long)(REC->count))

The "fd: " part is a PRINT_FMT_STRING. We don't want anything to do with that.

But the cache part that has fields, should be an arg of type
TEP_PRINT_FIELD, which should be mapped to some field to print. That
is, one of the REC->fd, REC->offset or REC->count.

That's what you are looking to see how to print it. As it also contains
how that field should be printed normally.


> 
> > 		if (parse->arg->field.field->field != field)
> > 			continue;
> >   
> 
> This does not compile. I guess you mean
>   		if (parse->arg->field.field != field)
>   			continue;
> 
> however "parse->arg->field.field" looks like unused memory (NULL or 0xffffffff) and this check always fails.
> 
> Maybe we must call some of the process_XXX()static functions first in order to make your new version of 
> tep_print_field() works?

How are you calling it? After loading the tep with tracefs_local_events()?

Because it should be loaded via tep_parse_event(), which calll
parse_args() and load the args to the event.

Basically we should be doing what print_event_cache() does, but only
for the field we are interested in.

-- Steve