Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758000AbbDWOza (ORCPT ); Thu, 23 Apr 2015 10:55:30 -0400 Received: from mail.kernel.org ([198.145.29.136]:41094 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756917AbbDWOz3 (ORCPT ); Thu, 23 Apr 2015 10:55:29 -0400 Date: Thu, 23 Apr 2015 11:55:24 -0300 From: Arnaldo Carvalho de Melo To: Masami Hiramatsu Cc: Peter Zijlstra , Linux Kernel Mailing List , David Ahern , namhyung@kernel.org, Jiri Olsa , Ingo Molnar Subject: Re: [PATCH perf/core v2 3/8] perf probe: Accept multiple filter options Message-ID: <20150423145524.GG7881@kernel.org> References: <20150423134610.26128.82557.stgit@localhost.localdomain> <20150423134616.26128.40942.stgit@localhost.localdomain> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150423134616.26128.40942.stgit@localhost.localdomain> X-Url: http://acmel.wordpress.com User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4375 Lines: 135 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 > --- > 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/