Re: [PATCH perf/core v2 3/8] perf probe: Accept multiple filter options

[Arnaldo Carvalho de Melo <acme@kernel.org>]

Em Thu, Apr 23, 2015 at 10:46:17PM +0900, Masami Hiramatsu escreveu:
> Accept multiple filter options. Each filters are combined
> by logical-or. E.g. --filter abc* --filter *def is same
> as --filter abc*|*def

Please break this patch in two, one introducing the new strfilter
functionality, the other making perf-probe use it.

This way if later I had to revert the perf-probe part but keep the
strfilter, if used by a later patch, that would be possible.

I.e. in general please try to add new functionality for a library
function in a patch and the actual use of it in another patch.

I applied the first two, and will continue after you reply to this,
thanks!

- Arnaldo
 
> Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> ---
>  tools/perf/builtin-probe.c  |   14 +++++++++-----
>  tools/perf/util/strfilter.c |   34 ++++++++++++++++++++++++++++++++++
>  tools/perf/util/strfilter.h |   13 +++++++++++++
>  3 files changed, 56 insertions(+), 5 deletions(-)
> 
> diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
> index 92dcce0..be17075 100644
> --- a/tools/perf/builtin-probe.c
> +++ b/tools/perf/builtin-probe.c
> @@ -262,21 +262,25 @@ static int opt_set_filter(const struct option *opt __maybe_unused,
>  			  const char *str, int unset __maybe_unused)
>  {
>  	const char *err;
> +	int ret = 0;
>  
>  	if (str) {
>  		pr_debug2("Set filter: %s\n", str);
> -		if (params.filter)
> -			strfilter__delete(params.filter);
> -		params.filter = strfilter__new(str, &err);
>  		if (!params.filter) {
> +			params.filter = strfilter__new(str, &err);
> +			if (!params.filter)
> +				ret = err ? -EINVAL : -ENOMEM;
> +		} else
> +			ret = strfilter__or(params.filter, str, &err);
> +
> +		if (ret == -EINVAL) {
>  			pr_err("Filter parse error at %td.\n", err - str + 1);
>  			pr_err("Source: \"%s\"\n", str);
>  			pr_err("         %*c\n", (int)(err - str + 1), '^');
> -			return -EINVAL;
>  		}
>  	}
>  
> -	return 0;
> +	return ret;
>  }
>  
>  static int init_params(void)
> diff --git a/tools/perf/util/strfilter.c b/tools/perf/util/strfilter.c
> index 79a757a..cd659e6 100644
> --- a/tools/perf/util/strfilter.c
> +++ b/tools/perf/util/strfilter.c
> @@ -170,6 +170,40 @@ struct strfilter *strfilter__new(const char *rules, const char **err)
>  	return filter;
>  }
>  
> +static int strfilter__add(struct strfilter *filter, bool _or, const char *rules,
> +			  const char **err)
> +{
> +	struct strfilter_node *right, *root;
> +	const char *ep = NULL;
> +
> +	if (!filter || !rules)
> +		return -EINVAL;
> +
> +	right = strfilter_node__new(rules, &ep);
> +	if (!right || *ep != '\0') {
> +		if (err)
> +			*err = ep;
> +		goto error;
> +	}
> +	root = strfilter_node__alloc(_or ? OP_or : OP_and, filter->root, right);
> +	if (!root) {
> +		ep = NULL;
> +		goto error;
> +	}
> +
> +	filter->root = root;
> +	return 0;
> +
> +error:
> +	strfilter_node__delete(right);
> +	return ep ? -EINVAL : -ENOMEM;
> +}
> +
> +int strfilter__or(struct strfilter *filter, const char *rules, const char **err)
> +{
> +	return strfilter__add(filter, true, rules, err);
> +}
> +
>  static bool strfilter_node__compare(struct strfilter_node *node,
>  				    const char *str)
>  {
> diff --git a/tools/perf/util/strfilter.h b/tools/perf/util/strfilter.h
> index fe611f3..c81ff97 100644
> --- a/tools/perf/util/strfilter.h
> +++ b/tools/perf/util/strfilter.h
> @@ -29,6 +29,19 @@ struct strfilter {
>  struct strfilter *strfilter__new(const char *rules, const char **err);
>  
>  /**
> + * strfilter__or - Add an additional rule by logical-or
> + * @filter: Original string filter
> + * @rules: Filter rule to add at left of the root of @filter
> + *         by using logical-and.
> + * @err: Pointer which points an error detected on @rules
> + *
> + * Parse @rules and join it to the @filter by using logical-or.
> + * Return 0 if success, or return the error code.
> + */
> +int strfilter__or(struct strfilter *filter,
> +		  const char *rules, const char **err);
> +
> +/**
>   * strfilter__compare - compare given string and a string filter
>   * @filter: String filter
>   * @str: target string