Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1506837 > unrolled thread

[RFC/PATCH 1/2] tools lib subcmd: Suppport cascading options

Back to article view | Back to linux.kernel

Contents

  [RFC/PATCH 1/2] tools lib subcmd: Suppport cascading options Namhyung Kim <namhyung@kernel.org> - 2016-10-24 05:10 +0200
    [RFC/PATCH 2/2] perf sched: Make common options cascading Namhyung Kim <namhyung@kernel.org> - 2016-10-24 05:10 +0200
      Re: [RFC/PATCH 2/2] perf sched: Make common options cascading Jiri Olsa <jolsa@redhat.com> - 2016-10-24 19:10 +0200
        Re: [RFC/PATCH 2/2] perf sched: Make common options cascading Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-10-25 19:30 +0200
          Re: [RFC/PATCH 2/2] perf sched: Make common options cascading Namhyung Kim <namhyung@kernel.org> - 2016-10-25 23:30 +0200

#1506837 — [RFC/PATCH 1/2] tools lib subcmd: Suppport cascading options

Sometimes subcommand have common options and it can only handled in the
upper level command unless it duplicates the options.

This patch adds a parent field and fallback to the parent if the given
argument was not found in the current options.

Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/lib/subcmd/parse-options.c | 14 ++++++++++++++
 tools/lib/subcmd/parse-options.h |  2 ++
 2 files changed, 16 insertions(+)

diff --git a/tools/lib/subcmd/parse-options.c b/tools/lib/subcmd/parse-options.c
index 981bb4481fd5..3284bb14ae78 100644
--- a/tools/lib/subcmd/parse-options.c
+++ b/tools/lib/subcmd/parse-options.c
@@ -314,12 +314,19 @@ static int get_value(struct parse_opt_ctx_t *p,
 
 static int parse_short_opt(struct parse_opt_ctx_t *p, const struct option *options)
 {
+retry:
 	for (; options->type != OPTION_END; options++) {
 		if (options->short_name == *p->opt) {
 			p->opt = p->opt[1] ? p->opt + 1 : NULL;
 			return get_value(p, options, OPT_SHORT);
 		}
 	}
+
+	if (options->parent) {
+		options = options->parent;
+		goto retry;
+	}
+
 	return -2;
 }
 
@@ -333,6 +340,7 @@ static int parse_long_opt(struct parse_opt_ctx_t *p, const char *arg,
 	if (!arg_end)
 		arg_end = arg + strlen(arg);
 
+retry:
 	for (; options->type != OPTION_END; options++) {
 		const char *rest;
 		int flags = 0;
@@ -426,6 +434,12 @@ static int parse_long_opt(struct parse_opt_ctx_t *p, const char *arg,
 	}
 	if (abbrev_option)
 		return get_value(p, abbrev_option, abbrev_flags);
+
+	if (options->parent) {
+		options = options->parent;
+		goto retry;
+	}
+
 	return -2;
 }
 
diff --git a/tools/lib/subcmd/parse-options.h b/tools/lib/subcmd/parse-options.h
index d60cab2726da..8866ac438b34 100644
--- a/tools/lib/subcmd/parse-options.h
+++ b/tools/lib/subcmd/parse-options.h
@@ -109,11 +109,13 @@ struct option {
 	intptr_t defval;
 	bool *set;
 	void *data;
+	const struct option *parent;
 };
 
 #define check_vtype(v, type) ( BUILD_BUG_ON_ZERO(!__builtin_types_compatible_p(typeof(v), type)) + v )
 
 #define OPT_END()                   { .type = OPTION_END }
+#define OPT_PARENT(p)               { .type = OPTION_END, .parent = (p) }
 #define OPT_ARGUMENT(l, h)          { .type = OPTION_ARGUMENT, .long_name = (l), .help = (h) }
 #define OPT_GROUP(h)                { .type = OPTION_GROUP, .help = (h) }
 #define OPT_BIT(s, l, v, h, b)      { .type = OPTION_BIT, .short_name = (s), .long_name = (l), .value = check_vtype(v, int *), .help = (h), .defval = (b) }
-- 
2.10.0

[toc] | [next] | [standalone]

#1506841 — [RFC/PATCH 2/2] perf sched: Make common options cascading

The -i and -v options can be used in subcommands so enable cascading the
sched_options.  This fixes the following inconvenience in 'perf sched':

  $ perf sched -i perf.data.sched  map
  ... (it works well) ...

  $ perf sched map  -i perf.data.sched
    Error: unknown switch `i'

   Usage: perf sched map [<options>]

          --color-cpus <cpus>
                            highlight given CPUs in map
          --color-pids <pids>
                            highlight given pids in map
          --compact         map output in compact mode
          --cpus <cpus>     display given CPUs in map

With this patch, the second command line works with the perf.data.sched
data file.

Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/builtin-sched.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
index f5503ca22e1c..8ca1b5409289 100644
--- a/tools/perf/builtin-sched.c
+++ b/tools/perf/builtin-sched.c
@@ -1954,6 +1954,15 @@ int cmd_sched(int argc, const char **argv, const char *prefix __maybe_unused)
 		.next_shortname2      = '0',
 		.skip_merge           = 0,
 	};
+	const struct option sched_options[] = {
+	OPT_STRING('i', "input", &input_name, "file",
+		    "input file name"),
+	OPT_INCR('v', "verbose", &verbose,
+		    "be more verbose (show symbol address, etc)"),
+	OPT_BOOLEAN('D', "dump-raw-trace", &dump_trace,
+		    "dump raw trace in ASCII"),
+	OPT_END()
+	};
 	const struct option latency_options[] = {
 	OPT_STRING('s', "sort", &sched.sort_order, "key[,key2...]",
 		   "sort by key(s): runtime, switch, avg, max"),
@@ -1965,7 +1974,7 @@ int cmd_sched(int argc, const char **argv, const char *prefix __maybe_unused)
 		    "dump raw trace in ASCII"),
 	OPT_BOOLEAN('p', "pids", &sched.skip_merge,
 		    "latency stats per pid instead of per comm"),
-	OPT_END()
+	OPT_PARENT(sched_options)
 	};
 	const struct option replay_options[] = {
 	OPT_UINTEGER('r', "repeat", &sched.replay_repeat,
@@ -1975,16 +1984,7 @@ int cmd_sched(int argc, const char **argv, const char *prefix __maybe_unused)
 	OPT_BOOLEAN('D', "dump-raw-trace", &dump_trace,
 		    "dump raw trace in ASCII"),
 	OPT_BOOLEAN('f', "force", &sched.force, "don't complain, do it"),
-	OPT_END()
-	};
-	const struct option sched_options[] = {
-	OPT_STRING('i', "input", &input_name, "file",
-		    "input file name"),
-	OPT_INCR('v', "verbose", &verbose,
-		    "be more verbose (show symbol address, etc)"),
-	OPT_BOOLEAN('D', "dump-raw-trace", &dump_trace,
-		    "dump raw trace in ASCII"),
-	OPT_END()
+	OPT_PARENT(sched_options)
 	};
 	const struct option map_options[] = {
 	OPT_BOOLEAN(0, "compact", &sched.map.comp,
@@ -1995,7 +1995,7 @@ int cmd_sched(int argc, const char **argv, const char *prefix __maybe_unused)
                     "highlight given CPUs in map"),
 	OPT_STRING(0, "cpus", &sched.map.cpus_str, "cpus",
                     "display given CPUs in map"),
-	OPT_END()
+	OPT_PARENT(sched_options)
 	};
 	const char * const latency_usage[] = {
 		"perf sched latency [<options>]",
-- 
2.10.0

[toc] | [prev] | [next] | [standalone]

#1507465 — Re: [RFC/PATCH 2/2] perf sched: Make common options cascading

On Mon, Oct 24, 2016 at 12:00:03PM +0900, Namhyung Kim wrote:
> The -i and -v options can be used in subcommands so enable cascading the
> sched_options.  This fixes the following inconvenience in 'perf sched':
> 
>   $ perf sched -i perf.data.sched  map
>   ... (it works well) ...
> 
>   $ perf sched map  -i perf.data.sched
>     Error: unknown switch `i'
> 
>    Usage: perf sched map [<options>]
> 
>           --color-cpus <cpus>
>                             highlight given CPUs in map
>           --color-pids <pids>
>                             highlight given pids in map
>           --compact         map output in compact mode
>           --cpus <cpus>     display given CPUs in map
> 
> With this patch, the second command line works with the perf.data.sched
> data file.
> 
> Cc: Josh Poimboeuf <jpoimboe@redhat.com>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>

good idea, I'll use it in c2c ;-)

for both:

Acked-by: Jiri Olsa <jolsa@kernel.org>

thanks,
jirka

> ---
>  tools/perf/builtin-sched.c | 24 ++++++++++++------------
>  1 file changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
> index f5503ca22e1c..8ca1b5409289 100644
> --- a/tools/perf/builtin-sched.c
> +++ b/tools/perf/builtin-sched.c
> @@ -1954,6 +1954,15 @@ int cmd_sched(int argc, const char **argv, const char *prefix __maybe_unused)
>  		.next_shortname2      = '0',
>  		.skip_merge           = 0,
>  	};
> +	const struct option sched_options[] = {
> +	OPT_STRING('i', "input", &input_name, "file",
> +		    "input file name"),
> +	OPT_INCR('v', "verbose", &verbose,
> +		    "be more verbose (show symbol address, etc)"),
> +	OPT_BOOLEAN('D', "dump-raw-trace", &dump_trace,
> +		    "dump raw trace in ASCII"),
> +	OPT_END()
> +	};
>  	const struct option latency_options[] = {
>  	OPT_STRING('s', "sort", &sched.sort_order, "key[,key2...]",
>  		   "sort by key(s): runtime, switch, avg, max"),
> @@ -1965,7 +1974,7 @@ int cmd_sched(int argc, const char **argv, const char *prefix __maybe_unused)
>  		    "dump raw trace in ASCII"),
>  	OPT_BOOLEAN('p', "pids", &sched.skip_merge,
>  		    "latency stats per pid instead of per comm"),
> -	OPT_END()
> +	OPT_PARENT(sched_options)
>  	};
>  	const struct option replay_options[] = {
>  	OPT_UINTEGER('r', "repeat", &sched.replay_repeat,
> @@ -1975,16 +1984,7 @@ int cmd_sched(int argc, const char **argv, const char *prefix __maybe_unused)
>  	OPT_BOOLEAN('D', "dump-raw-trace", &dump_trace,
>  		    "dump raw trace in ASCII"),
>  	OPT_BOOLEAN('f', "force", &sched.force, "don't complain, do it"),
> -	OPT_END()
> -	};
> -	const struct option sched_options[] = {
> -	OPT_STRING('i', "input", &input_name, "file",
> -		    "input file name"),
> -	OPT_INCR('v', "verbose", &verbose,
> -		    "be more verbose (show symbol address, etc)"),
> -	OPT_BOOLEAN('D', "dump-raw-trace", &dump_trace,
> -		    "dump raw trace in ASCII"),
> -	OPT_END()
> +	OPT_PARENT(sched_options)
>  	};
>  	const struct option map_options[] = {
>  	OPT_BOOLEAN(0, "compact", &sched.map.comp,
> @@ -1995,7 +1995,7 @@ int cmd_sched(int argc, const char **argv, const char *prefix __maybe_unused)
>                      "highlight given CPUs in map"),
>  	OPT_STRING(0, "cpus", &sched.map.cpus_str, "cpus",
>                      "display given CPUs in map"),
> -	OPT_END()
> +	OPT_PARENT(sched_options)
>  	};
>  	const char * const latency_usage[] = {
>  		"perf sched latency [<options>]",
> -- 
> 2.10.0
> 

[toc] | [prev] | [next] | [standalone]

#1508499 — Re: [RFC/PATCH 2/2] perf sched: Make common options cascading

Em Mon, Oct 24, 2016 at 07:03:32PM +0200, Jiri Olsa escreveu:
> On Mon, Oct 24, 2016 at 12:00:03PM +0900, Namhyung Kim wrote:
> > The -i and -v options can be used in subcommands so enable cascading the
> > sched_options.  This fixes the following inconvenience in 'perf sched':
> > 
> >   $ perf sched -i perf.data.sched  map
> >   ... (it works well) ...
> > 
> >   $ perf sched map  -i perf.data.sched
> >     Error: unknown switch `i'
> > 
> >    Usage: perf sched map [<options>]
> > 
> >           --color-cpus <cpus>
> >                             highlight given CPUs in map
> >           --color-pids <pids>
> >                             highlight given pids in map
> >           --compact         map output in compact mode
> >           --cpus <cpus>     display given CPUs in map
> > 
> > With this patch, the second command line works with the perf.data.sched
> > data file.
> > 
> > Cc: Josh Poimboeuf <jpoimboe@redhat.com>
> > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> 
> good idea, I'll use it in c2c ;-)
> 
> for both:
> 
> Acked-by: Jiri Olsa <jolsa@kernel.org>

Applied, I guess 'perf kvm' would be another user, to make it sane wrt
file_name, -i and -o being available for 'perf kvm record', etc.

- Arnaldo

[toc] | [prev] | [next] | [standalone]

#1508654 — Re: [RFC/PATCH 2/2] perf sched: Make common options cascading

On Tue, Oct 25, 2016 at 02:21:32PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Mon, Oct 24, 2016 at 07:03:32PM +0200, Jiri Olsa escreveu:
> > On Mon, Oct 24, 2016 at 12:00:03PM +0900, Namhyung Kim wrote:
> > > The -i and -v options can be used in subcommands so enable cascading the
> > > sched_options.  This fixes the following inconvenience in 'perf sched':
> > > 
> > >   $ perf sched -i perf.data.sched  map
> > >   ... (it works well) ...
> > > 
> > >   $ perf sched map  -i perf.data.sched
> > >     Error: unknown switch `i'
> > > 
> > >    Usage: perf sched map [<options>]
> > > 
> > >           --color-cpus <cpus>
> > >                             highlight given CPUs in map
> > >           --color-pids <pids>
> > >                             highlight given pids in map
> > >           --compact         map output in compact mode
> > >           --cpus <cpus>     display given CPUs in map
> > > 
> > > With this patch, the second command line works with the perf.data.sched
> > > data file.
> > > 
> > > Cc: Josh Poimboeuf <jpoimboe@redhat.com>
> > > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > 
> > good idea, I'll use it in c2c ;-)
> > 
> > for both:
> > 
> > Acked-by: Jiri Olsa <jolsa@kernel.org>
> 
> Applied, I guess 'perf kvm' would be another user, to make it sane wrt
> file_name, -i and -o being available for 'perf kvm record', etc.

Yep, I think we have few more.  I'll send the patch when I can find a
time (FYI I'm travelling now).

Thanks,
Namhyung

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web