Re: [PATCH v6 5/5] libtraceevent: Add tep_print_selected_fields()

[Steven Rostedt <rostedt@goodmis.org>]

On Thu, 12 Aug 2021 11:59:29 +0300
"Yordan Karadzhov (VMware)" <y.karadz@gmail.com> wrote:

> The new method can print only a subset of the unique data fields of
> the trace event. The print format is derived from the parsing tokens
> (tep_print_parse objects) of the event.
> 
> Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com>
> ---
>  src/event-parse.c | 26 ++++++++++++++++++++++----
>  src/event-parse.h |  3 +++
>  2 files changed, 25 insertions(+), 4 deletions(-)
> 
> diff --git a/src/event-parse.c b/src/event-parse.c
> index 0795135..645506e 100644
> --- a/src/event-parse.c
> +++ b/src/event-parse.c
> @@ -5455,20 +5455,38 @@ void tep_print_field(struct trace_seq *s, void *data,
>  	_tep_print_field(s, data, field, NULL);
>  }
>  
> -void tep_print_fields(struct trace_seq *s, void *data,
> -		      int size __maybe_unused, struct tep_event *event)
> +static inline void
> +print_selected_fields(struct trace_seq *s, void *data,
> +		      struct tep_event *event,
> +		      unsigned long long ignore_mask)
>  {
>  	struct tep_print_parse *parse = event->print_fmt.print_cache;
>  	struct tep_format_field *field;
> +	unsigned long long field_mask = 1;
>  
>  	field = event->format.fields;
> -	while (field) {
> +	for (;field; field = field->next, field_mask *= 2) {

The above should be:

	for(; field; field = field->next, field_mask <<= 1) {

As a shift should be done with a shift operator and not a
multiplication.

Other than that, the rest looks good. I'll go ahead and pull in patches
1 and 2.

-- Steve


> +		if (field_mask & ignore_mask)
> +			continue;
> +
>  		trace_seq_printf(s, " %s=", field->name);
>  		_tep_print_field(s, data, field, &parse);
> -		field = field->next;
>  	}
>  }
>  
> +void tep_print_selected_fields(struct trace_seq *s, void *data,

As the above is an API, it needs a kernel doc type comment, and also an
addition to the man pages. The man page may be a separate patch.

-- Steve


> +			       struct tep_event *event,
> +			       unsigned long long ignore_mask)
> +{
> +	print_selected_fields(s, data, event, ignore_mask);
> +}
> +
> +void tep_print_fields(struct trace_seq *s, void *data,
> +		      int size __maybe_unused, struct tep_event *event)
> +{
> +	print_selected_fields(s, data, event, 0);
> +}
> +
>  static int print_function(struct trace_seq *s, const char *format,
>  			  void *data, int size, struct tep_event *event,
>  			  struct tep_print_arg *arg)
> diff --git a/src/event-parse.h b/src/event-parse.h
> index d4a876f..e3638cf 100644
> --- a/src/event-parse.h
> +++ b/src/event-parse.h
> @@ -545,6 +545,9 @@ int tep_cmdline_pid(struct tep_handle *tep, struct tep_cmdline *cmdline);
>  
>  void tep_print_field(struct trace_seq *s, void *data,
>  		     struct tep_format_field *field);
> +void tep_print_selected_fields(struct trace_seq *s, void *data,
> +			       struct tep_event *event,
> +			       unsigned long long ignore_mask);
>  void tep_print_fields(struct trace_seq *s, void *data,
>  		      int size __maybe_unused, struct tep_event *event);
>  int tep_strerror(struct tep_handle *tep, enum tep_errno errnum,