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

[Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>]

(2015/04/23 23:55), Arnaldo Carvalho de Melo wrote:
> 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.

OK, I'll split strfilter features(__or, __string) out from
these patches.

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

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


-- 
Masami HIRAMATSU
Linux Technology Research Center, System Productivity Research Dept.
Center for Technology Innovation - Systems Engineering
Hitachi, Ltd., Research & Development Group
E-mail: masami.hiramatsu.pt@hitachi.com