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

From: Arnaldo Carvalho de Melo
Date: Thu Apr 23 2015 - 10:55:35 EST


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@xxxxxxxxxxx>
> ---
> 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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/