Re: [PATCH 4/4] libtracefs: New API for applying filter on event.

[Steven Rostedt <rostedt@goodmis.org>]

Note, the subject should not have a period.

On Tue, 30 Nov 2021 07:00:57 +0200
"Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote:

> There is no API for applying a filter string on event. Existing APIs

"on an event."

> only constructs and verifies the filter string. Even though the actual

  "only construct and verify"

> applying is just writing into the event's filter file, it is good to
> have a dedicated API for that:

[ add empty line here ]

> 	tracefs_event_apply_filter()
> 
> Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
> ---
>  Documentation/libtracefs-filter.txt | 11 ++++++++++-
>  include/tracefs.h                   |  3 +++
>  src/tracefs-filter.c                | 19 +++++++++++++++++++
>  3 files changed, 32 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/libtracefs-filter.txt b/Documentation/libtracefs-filter.txt
> index 7e167bc..7532c96 100644
> --- a/Documentation/libtracefs-filter.txt
> +++ b/Documentation/libtracefs-filter.txt
> @@ -3,7 +3,8 @@ libtracefs(3)
>  
>  NAME
>  ----
> -tracefs_event_append_filter, tracefs_event_verify_filter - Add and verify event filters
> +tracefs_event_append_filter, tracefs_event_verify_filter tracefs_event_apply_filter -

Missing comma (and you also need to rebase with the updates).


> +Add, verify and apply event filters
>  
>  SYNOPSIS
>  --------
> @@ -15,6 +16,7 @@ int tracefs_event_append_filter(struct tep_event pass:[*]event, char pass:[**] f
>  				 struct tracefs_filter type, const char pass:[*]field,
>  				 enum tracefs_synth_compare compare, const char pass:[*]val);
>  int tracefs_event_verify_filter(struct tep_event pass:[*]event, const char pass:[*]filter, char pass:[**]err);

I renamed the above, because it doesn't really affect the event.

> +int tracefs_event_apply_filter(struct tracefs_instance pass:[*]instance, struct tep_event pass:[*]event, const char pass:[*]filter);

But you can keep this name, as it makes sense to call this
"tracefs_event_.."

>  
>  --
>  
> @@ -66,6 +68,8 @@ error in the syntax, and _err_ is not NULL, then it will be allocated with an
>  error message stating what was found wrong with the filter. _err_ must be freed
>  with *free*().
>  
> +*tracefs_event_apply_filter*() applies given _filter_ string on _event_ in given _instance_.
> +
>  RETURN VALUE
>  ------------
>  *tracefs_event_append_filter*() returns 0 on success and -1 on error.
> @@ -75,6 +79,8 @@ is an error, and _errno_ is not *ENOMEM*, then _err_ is allocated and will
>  contain a string describing what was found wrong with _filter_. _err_ must be
>  freed with *free*().
>  
> +*tracefs_event_apply_filter*() returns 0 on success and -1 on error.
> +
>  EXAMPLE
>  -------
>  [source,c]
> @@ -269,6 +275,9 @@ int main (int argc, char **argv)
>  		}
>  	}
>  
> +	if (tracefs_event_apply_filter(NULL, event, new_filter))
> +		fprintf(stderr, "Failed to apply filter on event");
> +
>  	tep_free(tep);
>  
>  	printf("Created new filter: '%s'\n", new_filter);
> diff --git a/include/tracefs.h b/include/tracefs.h
> index fbd7d31..8ac9694 100644
> --- a/include/tracefs.h
> +++ b/include/tracefs.h
> @@ -475,6 +475,9 @@ int tracefs_event_append_filter(struct tep_event *event, char **filter,
>  				const char *val);
>  int tracefs_event_verify_filter(struct tep_event *event, const char *filter,
>  				char **err);
> +int tracefs_event_apply_filter(struct tracefs_instance *instance,
> +			       struct tep_event *event, const char *filter);
> +
>  
>  #define TRACEFS_TIMESTAMP "common_timestamp"
>  #define TRACEFS_TIMESTAMP_USECS "common_timestamp.usecs"
> diff --git a/src/tracefs-filter.c b/src/tracefs-filter.c
> index def8f68..43683d0 100644
> --- a/src/tracefs-filter.c
> +++ b/src/tracefs-filter.c
> @@ -745,3 +745,22 @@ int tracefs_event_verify_filter(struct tep_event *event, const char *filter,
>  	free(str);
>  	return 0;
>  }
> +
> +/**
> + * tracefs_event_apply_filter - apply given filter on event in given instance
> + * @instance: The instance in which the filter will be applied (NULL for toplevel).
> + * @event: The event to apply the filter on.
> + * @filter: The filter to apply.
> + *
> + * Apply the @filter to given @event in givem @instance. The @filter string
> + * should be created with tracefs_event_append_filter().

The name of the function has been renamed.

> + *
> + * Returns 0 on succes and -1 on error.
> + */
> +int tracefs_event_apply_filter(struct tracefs_instance *instance,
> +			       struct tep_event *event, const char *filter)
> +{
> +	return tracefs_event_file_append(instance, event->system, event->name,
> +					 "filter", filter);

I think we want this to be tracefs_event_file_write(), as it should replace
the filter, not add on to it.

> +}
> +

Note, I moved the old deprecated names at the end of the file. Make sure
they stay at the end. In other words, this goes before them.

-- Steve