From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752656AbbGNGyx (ORCPT ); Tue, 14 Jul 2015 02:54:53 -0400 Received: from mx1.redhat.com ([209.132.183.28]:46222 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751848AbbGNGyv (ORCPT ); Tue, 14 Jul 2015 02:54:51 -0400 Date: Tue, 14 Jul 2015 08:54:48 +0200 From: Jiri Olsa To: kan.liang@intel.com Cc: acme@kernel.org, jolsa@kernel.org, namhyung@kernel.org, ak@linux.intel.com, linux-kernel@vger.kernel.org Subject: Re: [PATCH RFC V3 1/5] perf,tools: introduce OPT_CALLBACK_SET/OPT_CALLBACK_NOOPT_SET Message-ID: <20150714065448.GF22977@krava.local> References: <1436345097-11113-1-git-send-email-kan.liang@intel.com> <1436345097-11113-2-git-send-email-kan.liang@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1436345097-11113-2-git-send-email-kan.liang@intel.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 On Wed, Jul 08, 2015 at 04:44:53AM -0400, kan.liang@intel.com wrote: > From: Kan Liang > > This patch extends the OPT_CALLBACK/OPT_CALLBACK_NOOPT to support 'set' > item of struct option. So the perf knows whether an option was set by > user. > The new macros are used by call-graph options. > > Signed-off-by: Kan Liang > --- > tools/perf/builtin-record.c | 4 ++-- > tools/perf/perf.h | 1 + > tools/perf/util/parse-options.c | 2 ++ > tools/perf/util/parse-options.h | 4 ++++ > 4 files changed, 9 insertions(+), 2 deletions(-) > > diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c > index 283fe96..a6eb24e 100644 > --- a/tools/perf/builtin-record.c > +++ b/tools/perf/builtin-record.c > @@ -1018,10 +1018,10 @@ struct option __record_options[] = { > record__parse_mmap_pages), > OPT_BOOLEAN(0, "group", &record.opts.group, > "put the counters into a counter group"), > - OPT_CALLBACK_NOOPT('g', NULL, &record.opts, > + OPT_CALLBACK_NOOPT_SET('g', NULL, &record.opts, &record.opts.callgraph_set, > NULL, "enables call-graph recording" , > &record_callchain_opt), > - OPT_CALLBACK(0, "call-graph", &record.opts, > + OPT_CALLBACK_SET(0, "call-graph", &record.opts, &record.opts.callgraph_set, > "mode[,dump_size]", record_callchain_help, > &record_parse_callchain_opt), we already provide &record.opts and callback which can set record.opts.callgraph_set.. seems redundant do I miss some other benefit? jirka > OPT_INCR('v', "verbose", &verbose, > diff --git a/tools/perf/perf.h b/tools/perf/perf.h > index 937b16a..9ba02e0 100644 > --- a/tools/perf/perf.h > +++ b/tools/perf/perf.h > @@ -52,6 +52,7 @@ struct record_opts { > bool sample_weight; > bool sample_time; > bool sample_time_set; > + bool callgraph_set; > bool period; > bool sample_intr_regs; > bool running_time; > diff --git a/tools/perf/util/parse-options.c b/tools/perf/util/parse-options.c > index 01626be..064385f 100644 > --- a/tools/perf/util/parse-options.c > +++ b/tools/perf/util/parse-options.c > @@ -140,6 +140,8 @@ static int get_value(struct parse_opt_ctx_t *p, > return err; > > case OPTION_CALLBACK: > + if (opt->set) > + *(bool *)opt->set = true; > if (unset) > return (*opt->callback)(opt, NULL, 1) ? (-1) : 0; > if (opt->flags & PARSE_OPT_NOARG) > diff --git a/tools/perf/util/parse-options.h b/tools/perf/util/parse-options.h > index 367d8b8..2bec32e 100644 > --- a/tools/perf/util/parse-options.h > +++ b/tools/perf/util/parse-options.h > @@ -132,8 +132,12 @@ struct option { > { .type = OPTION_CALLBACK, .short_name = (s), .long_name = (l), .value = (v), .argh = "time", .help = (h), .callback = parse_opt_approxidate_cb } > #define OPT_CALLBACK(s, l, v, a, h, f) \ > { .type = OPTION_CALLBACK, .short_name = (s), .long_name = (l), .value = (v), (a), .help = (h), .callback = (f) } > +#define OPT_CALLBACK_SET(s, l, v, os, a, h, f) \ > + { .type = OPTION_CALLBACK, .short_name = (s), .long_name = (l), .value = (v), (a), .help = (h), .callback = (f), .set = check_vtype(os, bool *) } > #define OPT_CALLBACK_NOOPT(s, l, v, a, h, f) \ > { .type = OPTION_CALLBACK, .short_name = (s), .long_name = (l), .value = (v), (a), .help = (h), .callback = (f), .flags = PARSE_OPT_NOARG } > +#define OPT_CALLBACK_NOOPT_SET(s, l, v, os, a, h, f) \ > + { .type = OPTION_CALLBACK, .short_name = (s), .long_name = (l), .value = (v), (a), .help = (h), .callback = (f), .flags = PARSE_OPT_NOARG, .set = check_vtype(os, bool *) } > #define OPT_CALLBACK_DEFAULT(s, l, v, a, h, f, d) \ > { .type = OPTION_CALLBACK, .short_name = (s), .long_name = (l), .value = (v), (a), .help = (h), .callback = (f), .defval = (intptr_t)d, .flags = PARSE_OPT_LASTARG_DEFAULT } > #define OPT_CALLBACK_DEFAULT_NOOPT(s, l, v, a, h, f, d) \ > -- > 1.8.3.1 >