Skip to contentSkip to conversation

perf tools: option parsing improvement

PATCH Applied Namhyung Kim 5 participants 17 messages Oct 22, 2014 – Oct 30, 2014 perf events

lore
Share & tools
lore

Namhyung Kim

author maintainer
#0 lore raw

cover perf tools: option parsing improvement

Hello,

This patchset tries to enhance option parser a bit. Patch 1-3 are to reuse existing perf record options for other commands like perf kvm stat record. Patch 4-5 are to support exclusive options that cannot be used at the same time. The perf probe has such options and upcoming sdt-cache command also.

You can get it from 'perf/option-share-v2' branch on my tree:

git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git

Any comments are welcome, thanks
Namhyung

Cc Masami Hiramatsu <[email protected]>
Cc Hemant Kumar <[email protected]>
Cc Alexander Yarygin <[email protected]>

Namhyung Kim (5):
perf tools: Add PARSE_OPT_DISABLED flag
perf tools: Export usage string and option table of perf record perf kvm: Print kvm specific --help output
perf tools: Add support for exclusive option
perf probe: Use PARSE_OPT_EXCLUSIVE flag

 tools/perf/builtin-kvm.c        | 25 +++++++++++++
 tools/perf/builtin-probe.c      | 54 +++++-----------------------
 tools/perf/builtin-record.c     |  7 ++--
 tools/perf/builtin-script.c     |  1 -
 tools/perf/builtin-timechart.c  |  7 ++--
 tools/perf/perf.h               |  3 ++
 tools/perf/util/parse-options.c | 78 ++++++++++++++++++++++++++++++++++-------
 tools/perf/util/parse-options.h |  4 +++
 8 files changed, 116 insertions(+), 63 deletions(-)
Signature · 1 line
2.0.0
author maintainer
#1 lore raw

PATCH 1/5 perf tools: Add PARSE_OPT_DISABLED flag

In some cases, we need to reuse exising options with some of them disabled. To do that, add PARSE_OPT_DISABLED flag and set_option_flag() function.

Cc Masami Hiramatsu <[email protected]>
Cc Hemant Kumar <[email protected]>
Cc Alexander Yarygin <[email protected]>
---
 tools/perf/util/parse-options.c | 17 +++++++++++++++++
 tools/perf/util/parse-options.h |  2 ++
 2 files changed, 19 insertions(+)
tools/perf/util/parse-options.c +17 -0
index bf48092983c6..b6016101b40b 100644
@@ -42,6 +42,8 @@ static int get_value(struct parse_opt_ctx_t *p,
 		return opterror(opt, "takes no value", flags);
 	if (unset && (opt->flags & PARSE_OPT_NONEG))
 		return opterror(opt, "isn't available", flags);
+	if (opt->flags & PARSE_OPT_DISABLED)
+		return opterror(opt, "is not usable", flags);
 
 	if (!(flags & OPT_SHORT) && p->opt) {
 		switch (opt->type) {
@@ -509,6 +511,8 @@ static void print_option_help(const struct option *opts, int full)
 	}
 	if (!full && (opts->flags & PARSE_OPT_HIDDEN))
 		return;
+	if (opts->flags & PARSE_OPT_DISABLED)
+		return;
 
 	pos = fprintf(stderr, "    ");
 	if (opts->short_name)
@@ -679,3 +683,16 @@ int parse_opt_verbosity_cb(const struct option *opt,
 	}
 	return 0;
 }
+
+void set_option_flag(struct option *opts, int shortopt, const char *longopt,
+		     int flag)
+{
+	for (; opts->type != OPTION_END; opts++) {
+		if ((shortopt && opts->short_name == shortopt) ||
+		    (opts->long_name && longopt &&
+		     !strcmp(opts->long_name, longopt))) {
+			opts->flags |= flag;
+			break;
+		}
+	}
+}
tools/perf/util/parse-options.h +2 -0
index b59ba858e73d..b7c80dbc7627 100644
@@ -38,6 +38,7 @@ enum parse_opt_option_flags {
 	PARSE_OPT_NONEG   = 4,
 	PARSE_OPT_HIDDEN  = 8,
 	PARSE_OPT_LASTARG_DEFAULT = 16,
+	PARSE_OPT_DISABLED = 32,
 };
 
 struct option;
@@ -211,4 +212,5 @@ extern int parse_opt_verbosity_cb(const struct option *, const char *, int);
 
 extern const char *parse_options_fix_filename(const char *prefix, const char *file);
 
+void set_option_flag(struct option *opts, int sopt, const char *lopt, int flag);
 #endif /* __PERF_PARSE_OPTIONS_H */
Signature · 1 line
2.0.0
#2 lore raw

[tip:perf/core] perf tools: Add PARSE_OPT_DISABLED flag

perf tools: Add PARSE_OPT_DISABLED flag
Commit-ID:  d152d1be5962ace0706066db71b4f05dff8764eb
Gitweb:     http://git.kernel.org/tip/d152d1be5962ace0706066db71b4f05dff8764eb
Author:     Namhyung Kim <[email protected]>
AuthorDate: Thu, 23 Oct 2014 00:15:45 +0900
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Wed, 29 Oct 2014 10:32:47 -0200

perf tools: Add PARSE_OPT_DISABLED flag

In some cases, we need to reuse exising options with some of them disabled. To do that, add PARSE_OPT_DISABLED flag and set_option_flag() function.

---
 tools/perf/util/parse-options.c | 17 +++++++++++++++++
 tools/perf/util/parse-options.h |  2 ++
 2 files changed, 19 insertions(+)
tools/perf/util/parse-options.c +17 -0
index bf48092..b601610 100644
@@ -42,6 +42,8 @@ static int get_value(struct parse_opt_ctx_t *p,
 		return opterror(opt, "takes no value", flags);
 	if (unset && (opt->flags & PARSE_OPT_NONEG))
 		return opterror(opt, "isn't available", flags);
+	if (opt->flags & PARSE_OPT_DISABLED)
+		return opterror(opt, "is not usable", flags);
 
 	if (!(flags & OPT_SHORT) && p->opt) {
 		switch (opt->type) {
@@ -509,6 +511,8 @@ static void print_option_help(const struct option *opts, int full)
 	}
 	if (!full && (opts->flags & PARSE_OPT_HIDDEN))
 		return;
+	if (opts->flags & PARSE_OPT_DISABLED)
+		return;
 
 	pos = fprintf(stderr, "    ");
 	if (opts->short_name)
@@ -679,3 +683,16 @@ int parse_opt_verbosity_cb(const struct option *opt,
 	}
 	return 0;
 }
+
+void set_option_flag(struct option *opts, int shortopt, const char *longopt,
+		     int flag)
+{
+	for (; opts->type != OPTION_END; opts++) {
+		if ((shortopt && opts->short_name == shortopt) ||
+		    (opts->long_name && longopt &&
+		     !strcmp(opts->long_name, longopt))) {
+			opts->flags |= flag;
+			break;
+		}
+	}
+}
tools/perf/util/parse-options.h +2 -0
index b59ba85..b7c80db 100644
@@ -38,6 +38,7 @@ enum parse_opt_option_flags {
 	PARSE_OPT_NONEG   = 4,
 	PARSE_OPT_HIDDEN  = 8,
 	PARSE_OPT_LASTARG_DEFAULT = 16,
+	PARSE_OPT_DISABLED = 32,
 };
 
 struct option;
@@ -211,4 +212,5 @@ extern int parse_opt_verbosity_cb(const struct option *, const char *, int);
 
 extern const char *parse_options_fix_filename(const char *prefix, const char *file);
 
+void set_option_flag(struct option *opts, int sopt, const char *lopt, int flag);
 #endif /* __PERF_PARSE_OPTIONS_H */
author maintainer
#3 lore raw

PATCH 2/5 perf tools: Export usage string and option table of perf record

In reply to Namhyung Kim

Those are shared with other builtin commands like kvm, script. So make it accessable from them. This is a preparation of later change that limiting possible options.

Cc Masami Hiramatsu <[email protected]>
Cc Hemant Kumar <[email protected]>
Cc Alexander Yarygin <[email protected]>
---
 tools/perf/builtin-record.c    | 7 +++++--
 tools/perf/builtin-script.c    | 1 -
 tools/perf/builtin-timechart.c | 7 ++++---
 tools/perf/perf.h              | 3 +++
 4 files changed, 12 insertions(+), 6 deletions(-)
tools/perf/builtin-record.c +5 -2
index 2583a9b04317..5091a27e6d28 100644
@@ -680,11 +680,12 @@ static int perf_record_config(const char *var, const char *value, void *cb)
 	return perf_default_config(var, value, cb);
 }
 
-static const char * const record_usage[] = {
+static const char * const __record_usage[] = {
 	"perf record [<options>] [<command>]",
 	"perf record [<options>] -- <command> [<options>]",
 	NULL
 };
+const char * const *record_usage = __record_usage;
 
 /*
  * XXX Ideally would be local to cmd_record() and passed to a record__new
@@ -725,7 +726,7 @@ const char record_callchain_help[] = CALLCHAIN_HELP "fp";
  * perf_evlist__prepare_workload, etc instead of fork+exec'in 'perf record',
  * using pipes, etc.
  */
-const struct option record_options[] = {
+struct option __record_options[] = {
 	OPT_CALLBACK('e', "event", &record.evlist, "event",
 		     "event selector. use 'perf list' to list available events",
 		     parse_events_option),
@@ -802,6 +803,8 @@ const struct option record_options[] = {
 	OPT_END()
 };
 
+struct option *record_options = __record_options;
+
 int cmd_record(int argc, const char **argv, const char *prefix __maybe_unused)
 {
 	int err = -ENOMEM;
tools/perf/builtin-script.c +0 -1
index 9708a1290571..28727c4fdc5d 100644
@@ -23,7 +23,6 @@ static char const		*generate_script_lang;
 static bool			debug_mode;
 static u64			last_timestamp;
 static u64			nr_unordered;
-extern const struct option	record_options[];
 static bool			no_callchain;
 static bool			latency_format;
 static bool			system_wide;
tools/perf/builtin-timechart.c +4 -3
index 35b425b6293f..9a68fda17cb5 100644
@@ -1963,7 +1963,7 @@ int cmd_timechart(int argc, const char **argv,
 		NULL
 	};
 
-	const struct option record_options[] = {
+	const struct option timechart_record_options[] = {
 	OPT_BOOLEAN('P', "power-only", &tchart.power_only, "output power data only"),
 	OPT_BOOLEAN('T', "tasks-only", &tchart.tasks_only,
 		    "output processes data only"),
@@ -1972,7 +1972,7 @@ int cmd_timechart(int argc, const char **argv,
 	OPT_BOOLEAN('g', "callchain", &tchart.with_backtrace, "record callchain"),
 	OPT_END()
 	};
-	const char * const record_usage[] = {
+	const char * const timechart_record_usage[] = {
 		"perf timechart record [<options>]",
 		NULL
 	};
@@ -1985,7 +1985,8 @@ int cmd_timechart(int argc, const char **argv,
 	}
 
 	if (argc && !strncmp(argv[0], "rec", 3)) {
-		argc = parse_options(argc, argv, record_options, record_usage,
+		argc = parse_options(argc, argv, timechart_record_options,
+				     timechart_record_usage,
 				     PARSE_OPT_STOP_AT_NON_OPTION);
 
 		if (tchart.power_only && tchart.tasks_only) {
tools/perf/perf.h +3 -0
index 220d44e44c1b..511c2831aa81 100644
@@ -62,4 +62,7 @@ struct record_opts {
 	unsigned     initial_delay;
 };
 
+struct option;
+extern const char * const *record_usage;
+extern struct option *record_options;
 #endif
Signature · 1 line
2.0.0
#4 lore raw

[tip:perf/core] perf tools: Export usage string and option table of perf record

perf tools: Export usage string and option table of perf record
Commit-ID:  e5b2c20755d37d781bb6e1e733faec5c39bd087a
Gitweb:     http://git.kernel.org/tip/e5b2c20755d37d781bb6e1e733faec5c39bd087a
Author:     Namhyung Kim <[email protected]>
AuthorDate: Thu, 23 Oct 2014 00:15:46 +0900
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Wed, 29 Oct 2014 10:32:47 -0200

perf tools: Export usage string and option table of perf record

Those are shared with other builtin commands like kvm, script. So make it accessable from them. This is a preparation of later change that limiting possible options.

---
 tools/perf/builtin-record.c    | 7 +++++--
 tools/perf/builtin-script.c    | 1 -
 tools/perf/builtin-timechart.c | 7 ++++---
 tools/perf/perf.h              | 3 +++
 4 files changed, 12 insertions(+), 6 deletions(-)
tools/perf/builtin-record.c +5 -2
index 2583a9b..5091a27 100644
@@ -680,11 +680,12 @@ static int perf_record_config(const char *var, const char *value, void *cb)
 	return perf_default_config(var, value, cb);
 }
 
-static const char * const record_usage[] = {
+static const char * const __record_usage[] = {
 	"perf record [<options>] [<command>]",
 	"perf record [<options>] -- <command> [<options>]",
 	NULL
 };
+const char * const *record_usage = __record_usage;
 
 /*
  * XXX Ideally would be local to cmd_record() and passed to a record__new
@@ -725,7 +726,7 @@ const char record_callchain_help[] = CALLCHAIN_HELP "fp";
  * perf_evlist__prepare_workload, etc instead of fork+exec'in 'perf record',
  * using pipes, etc.
  */
-const struct option record_options[] = {
+struct option __record_options[] = {
 	OPT_CALLBACK('e', "event", &record.evlist, "event",
 		     "event selector. use 'perf list' to list available events",
 		     parse_events_option),
@@ -802,6 +803,8 @@ const struct option record_options[] = {
 	OPT_END()
 };
 
+struct option *record_options = __record_options;
+
 int cmd_record(int argc, const char **argv, const char *prefix __maybe_unused)
 {
 	int err = -ENOMEM;
tools/perf/builtin-script.c +0 -1
index b35517f..ce304df 100644
@@ -23,7 +23,6 @@ static char const		*generate_script_lang;
 static bool			debug_mode;
 static u64			last_timestamp;
 static u64			nr_unordered;
-extern const struct option	record_options[];
 static bool			no_callchain;
 static bool			latency_format;
 static bool			system_wide;
tools/perf/builtin-timechart.c +4 -3
index f5fb256..f3bb1a4 100644
@@ -1963,7 +1963,7 @@ int cmd_timechart(int argc, const char **argv,
 		NULL
 	};
 
-	const struct option record_options[] = {
+	const struct option timechart_record_options[] = {
 	OPT_BOOLEAN('P', "power-only", &tchart.power_only, "output power data only"),
 	OPT_BOOLEAN('T', "tasks-only", &tchart.tasks_only,
 		    "output processes data only"),
@@ -1972,7 +1972,7 @@ int cmd_timechart(int argc, const char **argv,
 	OPT_BOOLEAN('g', "callchain", &tchart.with_backtrace, "record callchain"),
 	OPT_END()
 	};
-	const char * const record_usage[] = {
+	const char * const timechart_record_usage[] = {
 		"perf timechart record [<options>]",
 		NULL
 	};
@@ -1985,7 +1985,8 @@ int cmd_timechart(int argc, const char **argv,
 	}
 
 	if (argc && !strncmp(argv[0], "rec", 3)) {
-		argc = parse_options(argc, argv, record_options, record_usage,
+		argc = parse_options(argc, argv, timechart_record_options,
+				     timechart_record_usage,
 				     PARSE_OPT_STOP_AT_NON_OPTION);
 
 		if (tchart.power_only && tchart.tasks_only) {
tools/perf/perf.h +3 -0
index 220d44e..511c2831 100644
@@ -62,4 +62,7 @@ struct record_opts {
 	unsigned     initial_delay;
 };
 
+struct option;
+extern const char * const *record_usage;
+extern struct option *record_options;
 #endif
author maintainer
#5 lore raw

PATCH 3/5 perf kvm: Print kvm specific --help output

In reply to Namhyung Kim

The 'perf kvm stat record' tool is an alias of 'perf record' with predefined kvm related options. All options that passed to 'perf kvm stat record' are processed by the 'perf record' tool. So, 'perf kvm stat record --help' prints help of usage for the 'perf record' command. There are a few options useful for 'perf kvm stat record', the rest either break kvm related output or don't change it.

Let's print safe for 'perf kvm stat record' options in addition to general 'perf record' --help output.

With this patch, new output looks like below:

  $ perf kvm stat record -h

usage: perf kvm stat record [<options>]

      -p, --pid <pid>       record events on existing process id
      -t, --tid <tid>       record events on existing thread id
      -r, --realtime <n>    collect data with this RT SCHED_FIFO priority
          --no-buffering    collect data without buffering
      -a, --all-cpus        system-wide collection from all CPUs
      -C, --cpu <cpu>       list of cpus to monitor
      -c, --count <n>       event period to sample
      -o, --output <file>   output file name
      -i, --no-inherit      child tasks do not inherit counters

-m, --mmap-pages <pages>
number of mmap data pages
-v, --verbose be more verbose (show counter open errors, etc)
-q, --quiet don't print any message
-s, --stat per thread counts
-D, --delay <n> ms to wait before starting measurement after program start
-u, --uid <user> user to profile
--per-thread use per-thread mmaps

  $ perf kvm stat record -n sleep 1
    Error: switch `n' is not usable

usage: perf kvm stat record [<options>]

Cc Alexander Yarygin <[email protected]>
---
 tools/perf/builtin-kvm.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)
tools/perf/builtin-kvm.c +25 -0
index b65eb0507b38..3c0f3d4fb021 100644
@@ -1132,6 +1132,10 @@ kvm_events_record(struct perf_kvm_stat *kvm, int argc, const char **argv)
 		"-m", "1024",
 		"-c", "1",
 	};
+	const char * const kvm_stat_record_usage[] = {
+		"perf kvm stat record [<options>]",
+		NULL
+	};
 	const char * const *events_tp;
 	events_tp_size = 0;
 
@@ -1159,6 +1163,27 @@ kvm_events_record(struct perf_kvm_stat *kvm, int argc, const char **argv)
 	for (j = 1; j < (unsigned int)argc; j++, i++)
 		rec_argv[i] = argv[j];
 
+	set_option_flag(record_options, 'e', "event", PARSE_OPT_HIDDEN);
+	set_option_flag(record_options, 0, "filter", PARSE_OPT_HIDDEN);
+	set_option_flag(record_options, 'R', "raw-samples", PARSE_OPT_HIDDEN);
+
+	set_option_flag(record_options, 'F', "freq", PARSE_OPT_DISABLED);
+	set_option_flag(record_options, 0, "group", PARSE_OPT_DISABLED);
+	set_option_flag(record_options, 'g', NULL, PARSE_OPT_DISABLED);
+	set_option_flag(record_options, 0, "call-graph", PARSE_OPT_DISABLED);
+	set_option_flag(record_options, 'd', "data", PARSE_OPT_DISABLED);
+	set_option_flag(record_options, 'T', "timestamp", PARSE_OPT_DISABLED);
+	set_option_flag(record_options, 'P', "period", PARSE_OPT_DISABLED);
+	set_option_flag(record_options, 'n', "no-samples", PARSE_OPT_DISABLED);
+	set_option_flag(record_options, 'N', "no-buildid-cache", PARSE_OPT_DISABLED);
+	set_option_flag(record_options, 'B', "no-buildid", PARSE_OPT_DISABLED);
+	set_option_flag(record_options, 'G', "cgroup", PARSE_OPT_DISABLED);
+	set_option_flag(record_options, 'b', "branch-any", PARSE_OPT_DISABLED);
+	set_option_flag(record_options, 'j', "branch-filter", PARSE_OPT_DISABLED);
+	set_option_flag(record_options, 'W', "weight", PARSE_OPT_DISABLED);
+	set_option_flag(record_options, 0, "transaction", PARSE_OPT_DISABLED);
+
+	record_usage = kvm_stat_record_usage;
 	return cmd_record(i, rec_argv, NULL);
 }
 
Signature · 1 line
2.0.0
#6 lore raw

[tip:perf/core] perf kvm: Print kvm specific --help output

perf kvm: Print kvm specific --help output
Commit-ID:  f45d20ffb654f4559648da402b1608e747d46942
Gitweb:     http://git.kernel.org/tip/f45d20ffb654f4559648da402b1608e747d46942
Author:     Namhyung Kim <[email protected]>
AuthorDate: Thu, 23 Oct 2014 00:15:47 +0900
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Wed, 29 Oct 2014 10:32:47 -0200

perf kvm: Print kvm specific --help output

The 'perf kvm stat record' tool is an alias of 'perf record' with predefined kvm related options. All options that passed to 'perf kvm stat record' are processed by the 'perf record' tool. So, 'perf kvm stat record --help' prints help of usage for the 'perf record' command. There are a few options useful for 'perf kvm stat record', the rest either break kvm related output or don't change it.

Let's print safe for 'perf kvm stat record' options in addition to general 'perf record' --help output.

With this patch, new output looks like below:

  $ perf kvm stat record -h

usage: perf kvm stat record [<options>]

      -p, --pid <pid>       record events on existing process id
      -t, --tid <tid>       record events on existing thread id
      -r, --realtime <n>    collect data with this RT SCHED_FIFO priority
          --no-buffering    collect data without buffering
      -a, --all-cpus        system-wide collection from all CPUs
      -C, --cpu <cpu>       list of cpus to monitor
      -c, --count <n>       event period to sample
      -o, --output <file>   output file name
      -i, --no-inherit      child tasks do not inherit counters

-m, --mmap-pages <pages>
number of mmap data pages
-v, --verbose be more verbose (show counter open errors, etc)
-q, --quiet don't print any message
-s, --stat per thread counts
-D, --delay <n> ms to wait before starting measurement after program start
-u, --uid <user> user to profile
--per-thread use per-thread mmaps

  $ perf kvm stat record -n sleep 1
    Error: switch `n' is not usable

usage: perf kvm stat record [<options>]

---
 tools/perf/builtin-kvm.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)
tools/perf/builtin-kvm.c +25 -0
index b65eb050..3c0f3d4 100644
@@ -1132,6 +1132,10 @@ kvm_events_record(struct perf_kvm_stat *kvm, int argc, const char **argv)
 		"-m", "1024",
 		"-c", "1",
 	};
+	const char * const kvm_stat_record_usage[] = {
+		"perf kvm stat record [<options>]",
+		NULL
+	};
 	const char * const *events_tp;
 	events_tp_size = 0;
 
@@ -1159,6 +1163,27 @@ kvm_events_record(struct perf_kvm_stat *kvm, int argc, const char **argv)
 	for (j = 1; j < (unsigned int)argc; j++, i++)
 		rec_argv[i] = argv[j];
 
+	set_option_flag(record_options, 'e', "event", PARSE_OPT_HIDDEN);
+	set_option_flag(record_options, 0, "filter", PARSE_OPT_HIDDEN);
+	set_option_flag(record_options, 'R', "raw-samples", PARSE_OPT_HIDDEN);
+
+	set_option_flag(record_options, 'F', "freq", PARSE_OPT_DISABLED);
+	set_option_flag(record_options, 0, "group", PARSE_OPT_DISABLED);
+	set_option_flag(record_options, 'g', NULL, PARSE_OPT_DISABLED);
+	set_option_flag(record_options, 0, "call-graph", PARSE_OPT_DISABLED);
+	set_option_flag(record_options, 'd', "data", PARSE_OPT_DISABLED);
+	set_option_flag(record_options, 'T', "timestamp", PARSE_OPT_DISABLED);
+	set_option_flag(record_options, 'P', "period", PARSE_OPT_DISABLED);
+	set_option_flag(record_options, 'n', "no-samples", PARSE_OPT_DISABLED);
+	set_option_flag(record_options, 'N', "no-buildid-cache", PARSE_OPT_DISABLED);
+	set_option_flag(record_options, 'B', "no-buildid", PARSE_OPT_DISABLED);
+	set_option_flag(record_options, 'G', "cgroup", PARSE_OPT_DISABLED);
+	set_option_flag(record_options, 'b', "branch-any", PARSE_OPT_DISABLED);
+	set_option_flag(record_options, 'j', "branch-filter", PARSE_OPT_DISABLED);
+	set_option_flag(record_options, 'W', "weight", PARSE_OPT_DISABLED);
+	set_option_flag(record_options, 0, "transaction", PARSE_OPT_DISABLED);
+
+	record_usage = kvm_stat_record_usage;
 	return cmd_record(i, rec_argv, NULL);
 }
 
author maintainer
#7 lore raw

PATCH 4/5 perf tools: Add support for exclusive option

In reply to Namhyung Kim

Some options cannot be used at the same time. To handle such options add a new PARSE_OPT_EXCLUSIVE flag and show error message if more than one of them is used.

Cc Masami Hiramatsu <[email protected]>
Cc Hemant Kumar <[email protected]>
---
 tools/perf/util/parse-options.c | 59 +++++++++++++++++++++++++++++++++--------
 tools/perf/util/parse-options.h |  2 ++
 2 files changed, 50 insertions(+), 11 deletions(-)
tools/perf/util/parse-options.c +48 -11
index b6016101b40b..f62dee7bd924 100644
@@ -45,6 +45,23 @@ static int get_value(struct parse_opt_ctx_t *p,
 	if (opt->flags & PARSE_OPT_DISABLED)
 		return opterror(opt, "is not usable", flags);
 
+	if (opt->flags & PARSE_OPT_EXCLUSIVE) {
+		if (p->excl_opt) {
+			char msg[128];
+
+			if (((flags & OPT_SHORT) && p->excl_opt->short_name) ||
+			    p->excl_opt->long_name == NULL) {
+				scnprintf(msg, sizeof(msg), "cannot be used with switch `%c'",
+					  p->excl_opt->short_name);
+			} else {
+				scnprintf(msg, sizeof(msg), "cannot be used with %s",
+					  p->excl_opt->long_name);
+			}
+			opterror(opt, msg, flags);
+			return -3;
+		}
+		p->excl_opt = opt;
+	}
 	if (!(flags & OPT_SHORT) && p->opt) {
 		switch (opt->type) {
 		case OPTION_CALLBACK:
@@ -345,13 +362,14 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
 		       const char * const usagestr[])
 {
 	int internal_help = !(ctx->flags & PARSE_OPT_NO_INTERNAL_HELP);
+	int excl_short_opt = 1;
+	const char *arg;
 
 	/* we must reset ->opt, unknown short option leave it dangling */
 	ctx->opt = NULL;
 
 	for (; ctx->argc; ctx->argc--, ctx->argv++) {
-		const char *arg = ctx->argv[0];
-
+		arg = ctx->argv[0];
 		if (*arg != '-' || !arg[1]) {
 			if (ctx->flags & PARSE_OPT_STOP_AT_NON_OPTION)
 				break;
@@ -360,19 +378,21 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
 		}
 
 		if (arg[1] != '-') {
-			ctx->opt = arg + 1;
+			ctx->opt = ++arg;
 			if (internal_help && *ctx->opt == 'h')
 				return usage_with_options_internal(usagestr, options, 0);
 			switch (parse_short_opt(ctx, options)) {
 			case -1:
-				return parse_options_usage(usagestr, options, arg + 1, 1);
+				return parse_options_usage(usagestr, options, arg, 1);
 			case -2:
 				goto unknown;
+			case -3:
+				goto exclusive;
 			default:
 				break;
 			}
 			if (ctx->opt)
-				check_typos(arg + 1, options);
+				check_typos(arg, options);
 			while (ctx->opt) {
 				if (internal_help && *ctx->opt == 'h')
 					return usage_with_options_internal(usagestr, options, 0);
@@ -389,6 +409,8 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
 					ctx->argv[0] = strdup(ctx->opt - 1);
 					*(char *)ctx->argv[0] = '-';
 					goto unknown;
+				case -3:
+					goto exclusive;
 				default:
 					break;
 				}
@@ -404,19 +426,23 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
 			break;
 		}
 
-		if (internal_help && !strcmp(arg + 2, "help-all"))
+		arg += 2;
+		if (internal_help && !strcmp(arg, "help-all"))
 			return usage_with_options_internal(usagestr, options, 1);
-		if (internal_help && !strcmp(arg + 2, "help"))
+		if (internal_help && !strcmp(arg, "help"))
 			return usage_with_options_internal(usagestr, options, 0);
-		if (!strcmp(arg + 2, "list-opts"))
+		if (!strcmp(arg, "list-opts"))
 			return PARSE_OPT_LIST_OPTS;
-		if (!strcmp(arg + 2, "list-cmds"))
+		if (!strcmp(arg, "list-cmds"))
 			return PARSE_OPT_LIST_SUBCMDS;
-		switch (parse_long_opt(ctx, arg + 2, options)) {
+		switch (parse_long_opt(ctx, arg, options)) {
 		case -1:
-			return parse_options_usage(usagestr, options, arg + 2, 0);
+			return parse_options_usage(usagestr, options, arg, 0);
 		case -2:
 			goto unknown;
+		case -3:
+			excl_short_opt = 0;
+			goto exclusive;
 		default:
 			break;
 		}
@@ -428,6 +454,17 @@ unknown:
 		ctx->opt = NULL;
 	}
 	return PARSE_OPT_DONE;
+
+exclusive:
+	parse_options_usage(usagestr, options, arg, excl_short_opt);
+	if ((excl_short_opt && ctx->excl_opt->short_name) ||
+	    ctx->excl_opt->long_name == NULL) {
+		char opt = ctx->excl_opt->short_name;
+		parse_options_usage(NULL, options, &opt, 1);
+	} else {
+		parse_options_usage(NULL, options, ctx->excl_opt->long_name, 0);
+	}
+	return PARSE_OPT_HELP;
 }
 
 int parse_options_end(struct parse_opt_ctx_t *ctx)
tools/perf/util/parse-options.h +2 -0
index b7c80dbc7627..97b153fb4999 100644
@@ -39,6 +39,7 @@ enum parse_opt_option_flags {
 	PARSE_OPT_HIDDEN  = 8,
 	PARSE_OPT_LASTARG_DEFAULT = 16,
 	PARSE_OPT_DISABLED = 32,
+	PARSE_OPT_EXCLUSIVE = 64,
 };
 
 struct option;
@@ -174,6 +175,7 @@ struct parse_opt_ctx_t {
 	const char **out;
 	int argc, cpidx;
 	const char *opt;
+	const struct option *excl_opt;
 	int flags;
 };
 
Signature · 1 line
2.0.0
Reviewed-by on v1 4/5
#8 lore raw

Re: [PATCH 4/5] perf tools: Add support for exclusive option

Quoted 3 lines · Namhyung Kim

Some options cannot be used at the same time. To handle such options add a new PARSE_OPT_EXCLUSIVE flag and show error message if more than one of them is used.

Looks useful for me :)

I just have a comment below;

Quoted 6 lines
tools/perf/util/parse-options.c +1 -1
@@ -360,19 +378,21 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
 		}
 
 		if (arg[1] != '-') {
-			ctx->opt = arg + 1;
+			ctx->opt = ++arg;

tools/perf/util/parse-options.c int parse_options_step(struct parse_opt_ctx_t *ctx, · line 390

Quoted 10 lines
tools/perf/util/parse-options.c +3 -1
 			if (internal_help && *ctx->opt == 'h')
 				return usage_with_options_internal(usagestr, options, 0);
 			switch (parse_short_opt(ctx, options)) {
 			case -1:
-				return parse_options_usage(usagestr, options, arg + 1, 1);
+				return parse_options_usage(usagestr, options, arg, 1);
 			case -2:
 				goto unknown;
+			case -3:
+				goto exclusive;

BTW, it may be a time to define return error codes.

Thank you,

Signature · 4 lines
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Research Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: [email protected]
#9 lore raw

Re: [PATCH 4/5] perf tools: Add support for exclusive option

Em Thu, Oct 23, 2014 at 02:05:08PM +0900, Masami Hiramatsu escreveu:

Quoted 14 lines
Quoted 3 lines · Namhyung Kim

Some options cannot be used at the same time. To handle such options add a new PARSE_OPT_EXCLUSIVE flag and show error message if more than one of them is used.

Looks useful for me :)

I just have a comment below;

Quoted 3 lines
(quoted diff) +0 -0
@@ -360,19 +378,21 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
 		}
 
Quoted 15 lines

tools/perf/util/parse-options.c int parse_options_step(struct parse_opt_ctx_t *ctx, · line 390

Quoted 13 lines
tools/perf/util/parse-options.c +4 -2
 		if (arg[1] != '-') {
-			ctx->opt = arg + 1;
+			ctx->opt = ++arg;
 			if (internal_help && *ctx->opt == 'h')
 				return usage_with_options_internal(usagestr, options, 0);
 			switch (parse_short_opt(ctx, options)) {
 			case -1:
-				return parse_options_usage(usagestr, options, arg + 1, 1);
+				return parse_options_usage(usagestr, options, arg, 1);
 			case -2:
 				goto unknown;
+			case -3:
+				goto exclusive;

BTW, it may be a time to define return error codes.

Yeah, this thing looks unnecessarily complicated :-\

author maintainer
#10 lore raw

Re: [PATCH 4/5] perf tools: Add support for exclusive option

Hi Arnaldo,

Quoted 31 lines · Arnaldo Carvalho de Melo

Em Thu, Oct 23, 2014 at 02:05:08PM +0900, Masami Hiramatsu escreveu:

Quoted 14 lines
Quoted 3 lines · Namhyung Kim

Some options cannot be used at the same time. To handle such options add a new PARSE_OPT_EXCLUSIVE flag and show error message if more than one of them is used.

Looks useful for me :)

I just have a comment below;

Quoted 3 lines
(quoted diff) +0 -0
@@ -360,19 +378,21 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
 		}
 
Quoted 15 lines

tools/perf/util/parse-options.c int parse_options_step(struct parse_opt_ctx_t *ctx, · line 390

Quoted 13 lines
tools/perf/util/parse-options.c +4 -2
 		if (arg[1] != '-') {
-			ctx->opt = arg + 1;
+			ctx->opt = ++arg;
 			if (internal_help && *ctx->opt == 'h')
 				return usage_with_options_internal(usagestr, options, 0);
 			switch (parse_short_opt(ctx, options)) {
 			case -1:
-				return parse_options_usage(usagestr, options, arg + 1, 1);
+				return parse_options_usage(usagestr, options, arg, 1);
 			case -2:
 				goto unknown;
+			case -3:
+				goto exclusive;

BTW, it may be a time to define return error codes.

Quoted 1 line

Yeah, this thing looks unnecessarily complicated :-\

OK, will do it as a separate patch later.

Thanks,
Namhyung

#11 lore raw

[tip:perf/core] perf tools: Add support for exclusive option

In reply to Namhyung Kim
perf tools: Add support for exclusive option
Commit-ID:  42bd71d0812ecd955cf65a14375ebe6a3195d979
Gitweb:     http://git.kernel.org/tip/42bd71d0812ecd955cf65a14375ebe6a3195d979
Author:     Namhyung Kim <[email protected]>
AuthorDate: Thu, 23 Oct 2014 00:15:48 +0900
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Wed, 29 Oct 2014 10:32:47 -0200

perf tools: Add support for exclusive option

Some options cannot be used at the same time. To handle such options add a new PARSE_OPT_EXCLUSIVE flag and show error message if more than one of them is used.

---
 tools/perf/util/parse-options.c | 59 +++++++++++++++++++++++++++++++++--------
 tools/perf/util/parse-options.h |  2 ++
 2 files changed, 50 insertions(+), 11 deletions(-)
tools/perf/util/parse-options.c +48 -11
index b601610..f62dee7 100644
@@ -45,6 +45,23 @@ static int get_value(struct parse_opt_ctx_t *p,
 	if (opt->flags & PARSE_OPT_DISABLED)
 		return opterror(opt, "is not usable", flags);
 
+	if (opt->flags & PARSE_OPT_EXCLUSIVE) {
+		if (p->excl_opt) {
+			char msg[128];
+
+			if (((flags & OPT_SHORT) && p->excl_opt->short_name) ||
+			    p->excl_opt->long_name == NULL) {
+				scnprintf(msg, sizeof(msg), "cannot be used with switch `%c'",
+					  p->excl_opt->short_name);
+			} else {
+				scnprintf(msg, sizeof(msg), "cannot be used with %s",
+					  p->excl_opt->long_name);
+			}
+			opterror(opt, msg, flags);
+			return -3;
+		}
+		p->excl_opt = opt;
+	}
 	if (!(flags & OPT_SHORT) && p->opt) {
 		switch (opt->type) {
 		case OPTION_CALLBACK:
@@ -345,13 +362,14 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
 		       const char * const usagestr[])
 {
 	int internal_help = !(ctx->flags & PARSE_OPT_NO_INTERNAL_HELP);
+	int excl_short_opt = 1;
+	const char *arg;
 
 	/* we must reset ->opt, unknown short option leave it dangling */
 	ctx->opt = NULL;
 
 	for (; ctx->argc; ctx->argc--, ctx->argv++) {
-		const char *arg = ctx->argv[0];
-
+		arg = ctx->argv[0];
 		if (*arg != '-' || !arg[1]) {
 			if (ctx->flags & PARSE_OPT_STOP_AT_NON_OPTION)
 				break;
@@ -360,19 +378,21 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
 		}
 
 		if (arg[1] != '-') {
-			ctx->opt = arg + 1;
+			ctx->opt = ++arg;
 			if (internal_help && *ctx->opt == 'h')
 				return usage_with_options_internal(usagestr, options, 0);
 			switch (parse_short_opt(ctx, options)) {
 			case -1:
-				return parse_options_usage(usagestr, options, arg + 1, 1);
+				return parse_options_usage(usagestr, options, arg, 1);
 			case -2:
 				goto unknown;
+			case -3:
+				goto exclusive;
 			default:
 				break;
 			}
 			if (ctx->opt)
-				check_typos(arg + 1, options);
+				check_typos(arg, options);
 			while (ctx->opt) {
 				if (internal_help && *ctx->opt == 'h')
 					return usage_with_options_internal(usagestr, options, 0);
@@ -389,6 +409,8 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
 					ctx->argv[0] = strdup(ctx->opt - 1);
 					*(char *)ctx->argv[0] = '-';
 					goto unknown;
+				case -3:
+					goto exclusive;
 				default:
 					break;
 				}
@@ -404,19 +426,23 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
 			break;
 		}
 
-		if (internal_help && !strcmp(arg + 2, "help-all"))
+		arg += 2;
+		if (internal_help && !strcmp(arg, "help-all"))
 			return usage_with_options_internal(usagestr, options, 1);
-		if (internal_help && !strcmp(arg + 2, "help"))
+		if (internal_help && !strcmp(arg, "help"))
 			return usage_with_options_internal(usagestr, options, 0);
-		if (!strcmp(arg + 2, "list-opts"))
+		if (!strcmp(arg, "list-opts"))
 			return PARSE_OPT_LIST_OPTS;
-		if (!strcmp(arg + 2, "list-cmds"))
+		if (!strcmp(arg, "list-cmds"))
 			return PARSE_OPT_LIST_SUBCMDS;
-		switch (parse_long_opt(ctx, arg + 2, options)) {
+		switch (parse_long_opt(ctx, arg, options)) {
 		case -1:
-			return parse_options_usage(usagestr, options, arg + 2, 0);
+			return parse_options_usage(usagestr, options, arg, 0);
 		case -2:
 			goto unknown;
+		case -3:
+			excl_short_opt = 0;
+			goto exclusive;
 		default:
 			break;
 		}
@@ -428,6 +454,17 @@ unknown:
 		ctx->opt = NULL;
 	}
 	return PARSE_OPT_DONE;
+
+exclusive:
+	parse_options_usage(usagestr, options, arg, excl_short_opt);
+	if ((excl_short_opt && ctx->excl_opt->short_name) ||
+	    ctx->excl_opt->long_name == NULL) {
+		char opt = ctx->excl_opt->short_name;
+		parse_options_usage(NULL, options, &opt, 1);
+	} else {
+		parse_options_usage(NULL, options, ctx->excl_opt->long_name, 0);
+	}
+	return PARSE_OPT_HELP;
 }
 
 int parse_options_end(struct parse_opt_ctx_t *ctx)
tools/perf/util/parse-options.h +2 -0
index b7c80db..97b153f 100644
@@ -39,6 +39,7 @@ enum parse_opt_option_flags {
 	PARSE_OPT_HIDDEN  = 8,
 	PARSE_OPT_LASTARG_DEFAULT = 16,
 	PARSE_OPT_DISABLED = 32,
+	PARSE_OPT_EXCLUSIVE = 64,
 };
 
 struct option;
@@ -174,6 +175,7 @@ struct parse_opt_ctx_t {
 	const char **out;
 	int argc, cpidx;
 	const char *opt;
+	const struct option *excl_opt;
 	int flags;
 };
 
author maintainer
#12 lore raw

PATCH 5/5 perf probe: Use PARSE_OPT_EXCLUSIVE flag

In reply to Namhyung Kim

The perf probe has some exclusive options. Use new PARSE_OPT_EXCLUSIVE flag to simplify the code and show more compact usage.

  $ perf probe -l -a foo
    Error: switch `a' cannot be used with switch `l'
   usage: perf probe [<options>] 'PROBEDEF' ['PROBEDEF' ...]
      or: perf probe [<options>] --add 'PROBEDEF' [--add 'PROBEDEF' ...]
      or: perf probe [<options>] --del '[GROUP:]EVENT' ...
      or: perf probe --list
      or: perf probe [<options>] --line 'LINEDESC'
      or: perf probe [<options>] --vars 'PROBEPOINT'

-a, --add <[EVENT=]FUNC[@SRC][+OFF|%return|:RL|;PT]|SRC:AL|SRC;PT [[NAME=]ARG ...]>
probe point definition, where
GROUP: Group name (optional)
EVENT: Event name
FUNC: Function name
OFF: Offset from function entry (in byte)
%return: Put the probe at function return
SRC: Source code path
RL: Relative line number from function entry.
AL: Absolute line number in file.
PT: Lazy expression of line code.
ARG: Probe argument (local variable name or
kprobe-tracer argument format.)

-l, --list list up current probe events

Cc Masami Hiramatsu <[email protected]>
Cc Hemant Kumar <[email protected]>
---
 tools/perf/builtin-probe.c | 54 ++++++++--------------------------------------
 1 file changed, 9 insertions(+), 45 deletions(-)
tools/perf/builtin-probe.c +9 -45
index 04412b4770a2..23d6e7f03cf1 100644
@@ -312,7 +312,7 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
 #endif
 		NULL
 };
-	const struct option options[] = {
+	struct option options[] = {
 	OPT_INCR('v', "verbose", &verbose,
 		    "be more verbose (show parsed arguments, etc)"),
 	OPT_BOOLEAN('l', "list", &params.list_events,
@@ -382,6 +382,14 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
 	};
 	int ret;
 
+	set_option_flag(options, 'a', "add", PARSE_OPT_EXCLUSIVE);
+	set_option_flag(options, 'd', "del", PARSE_OPT_EXCLUSIVE);
+	set_option_flag(options, 'l', "list", PARSE_OPT_EXCLUSIVE);
+#ifdef HAVE_DWARF_SUPPORT
+	set_option_flag(options, 'L', "line", PARSE_OPT_EXCLUSIVE);
+	set_option_flag(options, 'V', "vars", PARSE_OPT_EXCLUSIVE);
+#endif
+
 	argc = parse_options(argc, argv, options, probe_usage,
 			     PARSE_OPT_STOP_AT_NON_OPTION);
 	if (argc > 0) {
@@ -409,22 +417,6 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
 	symbol_conf.try_vmlinux_path = (symbol_conf.vmlinux_name == NULL);
 
 	if (params.list_events) {
-		if (params.mod_events) {
-			pr_err("  Error: Don't use --list with --add/--del.\n");
-			usage_with_options(probe_usage, options);
-		}
-		if (params.show_lines) {
-			pr_err("  Error: Don't use --list with --line.\n");
-			usage_with_options(probe_usage, options);
-		}
-		if (params.show_vars) {
-			pr_err(" Error: Don't use --list with --vars.\n");
-			usage_with_options(probe_usage, options);
-		}
-		if (params.show_funcs) {
-			pr_err("  Error: Don't use --list with --funcs.\n");
-			usage_with_options(probe_usage, options);
-		}
 		if (params.uprobes) {
 			pr_warning("  Error: Don't use --list with --exec.\n");
 			usage_with_options(probe_usage, options);
@@ -435,19 +427,6 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
 		return ret;
 	}
 	if (params.show_funcs) {
-		if (params.nevents != 0 || params.dellist) {
-			pr_err("  Error: Don't use --funcs with"
-			       " --add/--del.\n");
-			usage_with_options(probe_usage, options);
-		}
-		if (params.show_lines) {
-			pr_err("  Error: Don't use --funcs with --line.\n");
-			usage_with_options(probe_usage, options);
-		}
-		if (params.show_vars) {
-			pr_err("  Error: Don't use --funcs with --vars.\n");
-			usage_with_options(probe_usage, options);
-		}
 		if (!params.filter)
 			params.filter = strfilter__new(DEFAULT_FUNC_FILTER,
 						       NULL);
@@ -462,16 +441,6 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
 
 #ifdef HAVE_DWARF_SUPPORT
 	if (params.show_lines) {
-		if (params.mod_events) {
-			pr_err("  Error: Don't use --line with"
-			       " --add/--del.\n");
-			usage_with_options(probe_usage, options);
-		}
-		if (params.show_vars) {
-			pr_err(" Error: Don't use --line with --vars.\n");
-			usage_with_options(probe_usage, options);
-		}
-
 		ret = show_line_range(&params.line_range, params.target,
 				      params.uprobes);
 		if (ret < 0)
@@ -479,11 +448,6 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
 		return ret;
 	}
 	if (params.show_vars) {
-		if (params.mod_events) {
-			pr_err("  Error: Don't use --vars with"
-			       " --add/--del.\n");
-			usage_with_options(probe_usage, options);
-		}
 		if (!params.filter)
 			params.filter = strfilter__new(DEFAULT_VAR_FILTER,
 						       NULL);
Signature · 1 line
2.0.0
Acked-by on v1 5/5
#13 lore raw

Re: [PATCH 5/5] perf probe: Use PARSE_OPT_EXCLUSIVE flag

Hi Namhyng,

Quoted 27 lines · Namhyung Kim

The perf probe has some exclusive options. Use new PARSE_OPT_EXCLUSIVE flag to simplify the code and show more compact usage.

  $ perf probe -l -a foo
    Error: switch `a' cannot be used with switch `l'
   usage: perf probe [<options>] 'PROBEDEF' ['PROBEDEF' ...]
      or: perf probe [<options>] --add 'PROBEDEF' [--add 'PROBEDEF' ...]
      or: perf probe [<options>] --del '[GROUP:]EVENT' ...
      or: perf probe --list
      or: perf probe [<options>] --line 'LINEDESC'
      or: perf probe [<options>] --vars 'PROBEPOINT'

-a, --add <[EVENT=]FUNC[@SRC][+OFF|%return|:RL|;PT]|SRC:AL|SRC;PT [[NAME=]ARG ...]>
probe point definition, where
GROUP: Group name (optional)
EVENT: Event name
FUNC: Function name
OFF: Offset from function entry (in byte)
%return: Put the probe at function return
SRC: Source code path
RL: Relative line number from function entry.
AL: Absolute line number in file.
PT: Lazy expression of line code.
ARG: Probe argument (local variable name or
kprobe-tracer argument format.)

Quoted 2 lines

-l, --list list up current probe events

Thanks! this looks very good to me:)

Quoted 108 lines
Cc Masami Hiramatsu <[email protected]>
Cc Hemant Kumar <[email protected]>
---
 tools/perf/builtin-probe.c | 54 ++++++++--------------------------------------
 1 file changed, 9 insertions(+), 45 deletions(-)
tools/perf/builtin-probe.c +9 -45
index 04412b4770a2..23d6e7f03cf1 100644
@@ -312,7 +312,7 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
 #endif
 		NULL
 };
-	const struct option options[] = {
+	struct option options[] = {
 	OPT_INCR('v', "verbose", &verbose,
 		    "be more verbose (show parsed arguments, etc)"),
 	OPT_BOOLEAN('l', "list", &params.list_events,
@@ -382,6 +382,14 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
 	};
 	int ret;
 
+	set_option_flag(options, 'a', "add", PARSE_OPT_EXCLUSIVE);
+	set_option_flag(options, 'd', "del", PARSE_OPT_EXCLUSIVE);
+	set_option_flag(options, 'l', "list", PARSE_OPT_EXCLUSIVE);
+#ifdef HAVE_DWARF_SUPPORT
+	set_option_flag(options, 'L', "line", PARSE_OPT_EXCLUSIVE);
+	set_option_flag(options, 'V', "vars", PARSE_OPT_EXCLUSIVE);
+#endif
+
 	argc = parse_options(argc, argv, options, probe_usage,
 			     PARSE_OPT_STOP_AT_NON_OPTION);
 	if (argc > 0) {
@@ -409,22 +417,6 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
 	symbol_conf.try_vmlinux_path = (symbol_conf.vmlinux_name == NULL);
 
 	if (params.list_events) {
-		if (params.mod_events) {
-			pr_err("  Error: Don't use --list with --add/--del.\n");
-			usage_with_options(probe_usage, options);
-		}
-		if (params.show_lines) {
-			pr_err("  Error: Don't use --list with --line.\n");
-			usage_with_options(probe_usage, options);
-		}
-		if (params.show_vars) {
-			pr_err(" Error: Don't use --list with --vars.\n");
-			usage_with_options(probe_usage, options);
-		}
-		if (params.show_funcs) {
-			pr_err("  Error: Don't use --list with --funcs.\n");
-			usage_with_options(probe_usage, options);
-		}
 		if (params.uprobes) {
 			pr_warning("  Error: Don't use --list with --exec.\n");
 			usage_with_options(probe_usage, options);
@@ -435,19 +427,6 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
 		return ret;
 	}
 	if (params.show_funcs) {
-		if (params.nevents != 0 || params.dellist) {
-			pr_err("  Error: Don't use --funcs with"
-			       " --add/--del.\n");
-			usage_with_options(probe_usage, options);
-		}
-		if (params.show_lines) {
-			pr_err("  Error: Don't use --funcs with --line.\n");
-			usage_with_options(probe_usage, options);
-		}
-		if (params.show_vars) {
-			pr_err("  Error: Don't use --funcs with --vars.\n");
-			usage_with_options(probe_usage, options);
-		}
 		if (!params.filter)
 			params.filter = strfilter__new(DEFAULT_FUNC_FILTER,
 						       NULL);
@@ -462,16 +441,6 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
 
 #ifdef HAVE_DWARF_SUPPORT
 	if (params.show_lines) {
-		if (params.mod_events) {
-			pr_err("  Error: Don't use --line with"
-			       " --add/--del.\n");
-			usage_with_options(probe_usage, options);
-		}
-		if (params.show_vars) {
-			pr_err(" Error: Don't use --line with --vars.\n");
-			usage_with_options(probe_usage, options);
-		}
-
 		ret = show_line_range(&params.line_range, params.target,
 				      params.uprobes);
 		if (ret < 0)
@@ -479,11 +448,6 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
 		return ret;
 	}
 	if (params.show_vars) {
-		if (params.mod_events) {
-			pr_err("  Error: Don't use --vars with"
-			       " --add/--del.\n");
-			usage_with_options(probe_usage, options);
-		}
 		if (!params.filter)
 			params.filter = strfilter__new(DEFAULT_VAR_FILTER,
 						       NULL);
Signature · 4 lines
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Research Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: [email protected]
#14 lore raw

[tip:perf/core] perf probe: Use PARSE_OPT_EXCLUSIVE flag

In reply to Namhyung Kim
perf probe: Use PARSE_OPT_EXCLUSIVE flag
Commit-ID:  13dcbbc0222f9768394b0a58ab84adcd630f48d6
Gitweb:     http://git.kernel.org/tip/13dcbbc0222f9768394b0a58ab84adcd630f48d6
Author:     Namhyung Kim <[email protected]>
AuthorDate: Thu, 23 Oct 2014 00:15:49 +0900
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Wed, 29 Oct 2014 10:32:47 -0200

perf probe: Use PARSE_OPT_EXCLUSIVE flag

The perf probe command has some exclusive options. Use new PARSE_OPT_EXCLUSIVE flag to simplify the code and show more compact usage.

  $ perf probe -l -a foo
    Error: switch `a' cannot be used with switch `l'
   usage: perf probe [<options>] 'PROBEDEF' ['PROBEDEF' ...]
      or: perf probe [<options>] --add 'PROBEDEF' [--add 'PROBEDEF' ...]
      or: perf probe [<options>] --del '[GROUP:]EVENT' ...
      or: perf probe --list
      or: perf probe [<options>] --line 'LINEDESC'
      or: perf probe [<options>] --vars 'PROBEPOINT'

-a, --add <[EVENT=]FUNC[@SRC][+OFF|%return|:RL|;PT]|SRC:AL|SRC;PT [[NAME=]ARG ...]>
probe point definition, where
GROUP: Group name (optional)
EVENT: Event name
FUNC: Function name
OFF: Offset from function entry (in byte)
%return: Put the probe at function return
SRC: Source code path
RL: Relative line number from function entry.
AL: Absolute line number in file.
PT: Lazy expression of line code.
ARG: Probe argument (local variable name or
kprobe-tracer argument format.)

-l, --list list up current probe events

---
 tools/perf/builtin-probe.c | 54 ++++++++--------------------------------------
 1 file changed, 9 insertions(+), 45 deletions(-)
tools/perf/builtin-probe.c +9 -45
index 7af26ac..2d3577d 100644
@@ -312,7 +312,7 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
 #endif
 		NULL
 };
-	const struct option options[] = {
+	struct option options[] = {
 	OPT_INCR('v', "verbose", &verbose,
 		    "be more verbose (show parsed arguments, etc)"),
 	OPT_BOOLEAN('l', "list", &params.list_events,
@@ -382,6 +382,14 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
 	};
 	int ret;
 
+	set_option_flag(options, 'a', "add", PARSE_OPT_EXCLUSIVE);
+	set_option_flag(options, 'd', "del", PARSE_OPT_EXCLUSIVE);
+	set_option_flag(options, 'l', "list", PARSE_OPT_EXCLUSIVE);
+#ifdef HAVE_DWARF_SUPPORT
+	set_option_flag(options, 'L', "line", PARSE_OPT_EXCLUSIVE);
+	set_option_flag(options, 'V', "vars", PARSE_OPT_EXCLUSIVE);
+#endif
+
 	argc = parse_options(argc, argv, options, probe_usage,
 			     PARSE_OPT_STOP_AT_NON_OPTION);
 	if (argc > 0) {
@@ -409,22 +417,6 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
 	symbol_conf.try_vmlinux_path = (symbol_conf.vmlinux_name == NULL);
 
 	if (params.list_events) {
-		if (params.mod_events) {
-			pr_err("  Error: Don't use --list with --add/--del.\n");
-			usage_with_options(probe_usage, options);
-		}
-		if (params.show_lines) {
-			pr_err("  Error: Don't use --list with --line.\n");
-			usage_with_options(probe_usage, options);
-		}
-		if (params.show_vars) {
-			pr_err(" Error: Don't use --list with --vars.\n");
-			usage_with_options(probe_usage, options);
-		}
-		if (params.show_funcs) {
-			pr_err("  Error: Don't use --list with --funcs.\n");
-			usage_with_options(probe_usage, options);
-		}
 		if (params.uprobes) {
 			pr_warning("  Error: Don't use --list with --exec.\n");
 			usage_with_options(probe_usage, options);
@@ -435,19 +427,6 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
 		return ret;
 	}
 	if (params.show_funcs) {
-		if (params.nevents != 0 || params.dellist) {
-			pr_err("  Error: Don't use --funcs with"
-			       " --add/--del.\n");
-			usage_with_options(probe_usage, options);
-		}
-		if (params.show_lines) {
-			pr_err("  Error: Don't use --funcs with --line.\n");
-			usage_with_options(probe_usage, options);
-		}
-		if (params.show_vars) {
-			pr_err("  Error: Don't use --funcs with --vars.\n");
-			usage_with_options(probe_usage, options);
-		}
 		if (!params.filter)
 			params.filter = strfilter__new(DEFAULT_FUNC_FILTER,
 						       NULL);
@@ -462,16 +441,6 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
 
 #ifdef HAVE_DWARF_SUPPORT
 	if (params.show_lines) {
-		if (params.mod_events) {
-			pr_err("  Error: Don't use --line with"
-			       " --add/--del.\n");
-			usage_with_options(probe_usage, options);
-		}
-		if (params.show_vars) {
-			pr_err(" Error: Don't use --line with --vars.\n");
-			usage_with_options(probe_usage, options);
-		}
-
 		ret = show_line_range(&params.line_range, params.target,
 				      params.uprobes);
 		if (ret < 0)
@@ -479,11 +448,6 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
 		return ret;
 	}
 	if (params.show_vars) {
-		if (params.mod_events) {
-			pr_err("  Error: Don't use --vars with"
-			       " --add/--del.\n");
-			usage_with_options(probe_usage, options);
-		}
 		if (!params.filter)
 			params.filter = strfilter__new(DEFAULT_VAR_FILTER,
 						       NULL);
#15 lore raw
In reply to Namhyung Kim

Hi Namhyung,

Quoted 7 lines · Namhyung Kim

Hello,

This patchset tries to enhance option parser a bit. Patch 1-3 are to reuse existing perf record options for other commands like perf kvm stat record. Patch 4-5 are to support exclusive options that cannot be used at the same time. The perf probe has such options and upcoming sdt-cache command also.

Thank you for working on this.

[SNIP]

Signature · 2 lines
Thanks,
Hemant Kumar
maintainer Applied
#16 lore raw
In reply to Namhyung Kim

Em Thu, Oct 23, 2014 at 12:15:44AM +0900, Namhyung Kim escreveu:

Quoted 8 lines

Hello,

This patchset tries to enhance option parser a bit. Patch 1-3 are to reuse existing perf record options for other commands like perf kvm stat record. Patch 4-5 are to support exclusive options that cannot be used at the same time. The perf probe has such options and upcoming sdt-cache command also.

Quoted 6 lines

You can get it from 'perf/option-share-v2' branch on my tree:

git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git

Any comments are welcome, thanks
Namhyung

Thanks, applied the series.

- Arnaldo

Quoted 25 lines
Cc Masami Hiramatsu <[email protected]>
Cc Hemant Kumar <[email protected]>
Cc Alexander Yarygin <[email protected]>

Namhyung Kim (5):
perf tools: Add PARSE_OPT_DISABLED flag
perf tools: Export usage string and option table of perf record perf kvm: Print kvm specific --help output
perf tools: Add support for exclusive option
perf probe: Use PARSE_OPT_EXCLUSIVE flag

 tools/perf/builtin-kvm.c        | 25 +++++++++++++
 tools/perf/builtin-probe.c      | 54 +++++-----------------------
 tools/perf/builtin-record.c     |  7 ++--
 tools/perf/builtin-script.c     |  1 -
 tools/perf/builtin-timechart.c  |  7 ++--
 tools/perf/perf.h               |  3 ++
 tools/perf/util/parse-options.c | 78 ++++++++++++++++++++++++++++++++++-------
 tools/perf/util/parse-options.h |  4 +++
 8 files changed, 116 insertions(+), 63 deletions(-)
Signature · 1 line
2.0.0