* [PATCHSET 0/5] perf tools: Enhance option parsing error message
@ 2013-11-01 7:33 Namhyung Kim
2013-11-01 7:33 ` [PATCH 1/5] perf tools: Show single option when failed to parse Namhyung Kim
` (5 more replies)
0 siblings, 6 replies; 12+ messages in thread
From: Namhyung Kim @ 2013-11-01 7:33 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, Namhyung Kim, LKML,
Jiri Olsa, David Ahern
Hi,
This patchset tries to enhance error message when perf failed to parse
option argument. Currently it just shows entire usage and options
which may fill two pages of screen. It's hard for me to say it's
really helpful to users. ;-)
With this patch series, it changed to show only related option(s).
There're also more points to improve yet - like auto-breaking long
lines, dealing with UIs and so on. But I believe this will be a good
start.
$ perf report -g help
usage: perf report [<options>]
-g, --call-graph <output_type,min_percent[,print_limit],call_order>
Display callchains using output_type (graph, flat, fractal, or none) , min percent threshold, optional print limit, callchain order, key (function or address). Default: fractal,0.5,callee,function
$ perf stat -Bx, ls
-B option not supported with -x
usage: perf stat [<options>] [<command>]
-B, --big-num print large numbers with thousands' separators
-x, --field-separator <separator>
print counts with custom separator
You can get this on 'perf/option-v1' branch in my tree
git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git
Thanks,
Namhyung
Namhyung Kim (5):
perf tools: Show single option when failed to parse
perf report: Postpone setting up browser after parsing options
perf report: Use parse_options_usage() for -s option failure
perf top: Use parse_options_usage() for -s option failure
perf stat: Enhance option parse error message
tools/perf/builtin-report.c | 30 +++---
tools/perf/builtin-stat.c | 42 +++++---
tools/perf/builtin-top.c | 8 +-
tools/perf/util/parse-options.c | 218 ++++++++++++++++++++++++----------------
tools/perf/util/parse-options.h | 4 +-
5 files changed, 181 insertions(+), 121 deletions(-)
--
1.7.11.7
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 1/5] perf tools: Show single option when failed to parse
2013-11-01 7:33 [PATCHSET 0/5] perf tools: Enhance option parsing error message Namhyung Kim
@ 2013-11-01 7:33 ` Namhyung Kim
2013-11-04 20:21 ` [tip:perf/core] " tip-bot for Namhyung Kim
2013-11-01 7:33 ` [PATCH 2/5] perf report: Postpone setting up browser after parsing options Namhyung Kim
` (4 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Namhyung Kim @ 2013-11-01 7:33 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, Namhyung Kim, LKML,
Jiri Olsa, David Ahern
From: Namhyung Kim <namhyung.kim@lge.com>
Current option parser outputs whole option help string when it failed
to parse an option. However this is not good for user if the command
has many option, she might feel hard which one is related easily.
Fix it by just showing the help message of the given option only.
Requested-by: Ingo Molnar <mingo@kernel.org>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/util/parse-options.c | 217 ++++++++++++++++++++++++----------------
tools/perf/util/parse-options.h | 4 +-
2 files changed, 132 insertions(+), 89 deletions(-)
diff --git a/tools/perf/util/parse-options.c b/tools/perf/util/parse-options.c
index 2bc9e70df7e2..1caf7b9a01ee 100644
--- a/tools/perf/util/parse-options.c
+++ b/tools/perf/util/parse-options.c
@@ -339,10 +339,10 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
if (arg[1] != '-') {
ctx->opt = arg + 1;
if (internal_help && *ctx->opt == 'h')
- return parse_options_usage(usagestr, options);
+ return usage_with_options_internal(usagestr, options, 0);
switch (parse_short_opt(ctx, options)) {
case -1:
- return parse_options_usage(usagestr, options);
+ return parse_options_usage(usagestr, options, arg + 1, 1);
case -2:
goto unknown;
default:
@@ -352,10 +352,11 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
check_typos(arg + 1, options);
while (ctx->opt) {
if (internal_help && *ctx->opt == 'h')
- return parse_options_usage(usagestr, options);
+ return usage_with_options_internal(usagestr, options, 0);
+ arg = ctx->opt;
switch (parse_short_opt(ctx, options)) {
case -1:
- return parse_options_usage(usagestr, options);
+ return parse_options_usage(usagestr, options, arg, 1);
case -2:
/* fake a short option thing to hide the fact that we may have
* started to parse aggregated stuff
@@ -383,12 +384,12 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
if (internal_help && !strcmp(arg + 2, "help-all"))
return usage_with_options_internal(usagestr, options, 1);
if (internal_help && !strcmp(arg + 2, "help"))
- return parse_options_usage(usagestr, options);
+ return usage_with_options_internal(usagestr, options, 0);
if (!strcmp(arg + 2, "list-opts"))
return PARSE_OPT_LIST;
switch (parse_long_opt(ctx, arg + 2, options)) {
case -1:
- return parse_options_usage(usagestr, options);
+ return parse_options_usage(usagestr, options, arg + 2, 0);
case -2:
goto unknown;
default:
@@ -445,6 +446,89 @@ int parse_options(int argc, const char **argv, const struct option *options,
#define USAGE_OPTS_WIDTH 24
#define USAGE_GAP 2
+static void print_option_help(const struct option *opts, int full)
+{
+ size_t pos;
+ int pad;
+
+ if (opts->type == OPTION_GROUP) {
+ fputc('\n', stderr);
+ if (*opts->help)
+ fprintf(stderr, "%s\n", opts->help);
+ return;
+ }
+ if (!full && (opts->flags & PARSE_OPT_HIDDEN))
+ return;
+
+ pos = fprintf(stderr, " ");
+ if (opts->short_name)
+ pos += fprintf(stderr, "-%c", opts->short_name);
+ else
+ pos += fprintf(stderr, " ");
+
+ if (opts->long_name && opts->short_name)
+ pos += fprintf(stderr, ", ");
+ if (opts->long_name)
+ pos += fprintf(stderr, "--%s", opts->long_name);
+
+ switch (opts->type) {
+ case OPTION_ARGUMENT:
+ break;
+ case OPTION_LONG:
+ case OPTION_U64:
+ case OPTION_INTEGER:
+ case OPTION_UINTEGER:
+ if (opts->flags & PARSE_OPT_OPTARG)
+ if (opts->long_name)
+ pos += fprintf(stderr, "[=<n>]");
+ else
+ pos += fprintf(stderr, "[<n>]");
+ else
+ pos += fprintf(stderr, " <n>");
+ break;
+ case OPTION_CALLBACK:
+ if (opts->flags & PARSE_OPT_NOARG)
+ break;
+ /* FALLTHROUGH */
+ case OPTION_STRING:
+ if (opts->argh) {
+ if (opts->flags & PARSE_OPT_OPTARG)
+ if (opts->long_name)
+ pos += fprintf(stderr, "[=<%s>]", opts->argh);
+ else
+ pos += fprintf(stderr, "[<%s>]", opts->argh);
+ else
+ pos += fprintf(stderr, " <%s>", opts->argh);
+ } else {
+ if (opts->flags & PARSE_OPT_OPTARG)
+ if (opts->long_name)
+ pos += fprintf(stderr, "[=...]");
+ else
+ pos += fprintf(stderr, "[...]");
+ else
+ pos += fprintf(stderr, " ...");
+ }
+ break;
+ default: /* OPTION_{BIT,BOOLEAN,SET_UINT,SET_PTR} */
+ case OPTION_END:
+ case OPTION_GROUP:
+ case OPTION_BIT:
+ case OPTION_BOOLEAN:
+ case OPTION_INCR:
+ case OPTION_SET_UINT:
+ case OPTION_SET_PTR:
+ break;
+ }
+
+ if (pos <= USAGE_OPTS_WIDTH)
+ pad = USAGE_OPTS_WIDTH - pos;
+ else {
+ fputc('\n', stderr);
+ pad = USAGE_OPTS_WIDTH;
+ }
+ fprintf(stderr, "%*s%s\n", pad + USAGE_GAP, "", opts->help);
+}
+
int usage_with_options_internal(const char * const *usagestr,
const struct option *opts, int full)
{
@@ -464,87 +548,9 @@ int usage_with_options_internal(const char * const *usagestr,
if (opts->type != OPTION_GROUP)
fputc('\n', stderr);
- for (; opts->type != OPTION_END; opts++) {
- size_t pos;
- int pad;
-
- if (opts->type == OPTION_GROUP) {
- fputc('\n', stderr);
- if (*opts->help)
- fprintf(stderr, "%s\n", opts->help);
- continue;
- }
- if (!full && (opts->flags & PARSE_OPT_HIDDEN))
- continue;
-
- pos = fprintf(stderr, " ");
- if (opts->short_name)
- pos += fprintf(stderr, "-%c", opts->short_name);
- else
- pos += fprintf(stderr, " ");
-
- if (opts->long_name && opts->short_name)
- pos += fprintf(stderr, ", ");
- if (opts->long_name)
- pos += fprintf(stderr, "--%s", opts->long_name);
+ for ( ; opts->type != OPTION_END; opts++)
+ print_option_help(opts, full);
- switch (opts->type) {
- case OPTION_ARGUMENT:
- break;
- case OPTION_LONG:
- case OPTION_U64:
- case OPTION_INTEGER:
- case OPTION_UINTEGER:
- if (opts->flags & PARSE_OPT_OPTARG)
- if (opts->long_name)
- pos += fprintf(stderr, "[=<n>]");
- else
- pos += fprintf(stderr, "[<n>]");
- else
- pos += fprintf(stderr, " <n>");
- break;
- case OPTION_CALLBACK:
- if (opts->flags & PARSE_OPT_NOARG)
- break;
- /* FALLTHROUGH */
- case OPTION_STRING:
- if (opts->argh) {
- if (opts->flags & PARSE_OPT_OPTARG)
- if (opts->long_name)
- pos += fprintf(stderr, "[=<%s>]", opts->argh);
- else
- pos += fprintf(stderr, "[<%s>]", opts->argh);
- else
- pos += fprintf(stderr, " <%s>", opts->argh);
- } else {
- if (opts->flags & PARSE_OPT_OPTARG)
- if (opts->long_name)
- pos += fprintf(stderr, "[=...]");
- else
- pos += fprintf(stderr, "[...]");
- else
- pos += fprintf(stderr, " ...");
- }
- break;
- default: /* OPTION_{BIT,BOOLEAN,SET_UINT,SET_PTR} */
- case OPTION_END:
- case OPTION_GROUP:
- case OPTION_BIT:
- case OPTION_BOOLEAN:
- case OPTION_INCR:
- case OPTION_SET_UINT:
- case OPTION_SET_PTR:
- break;
- }
-
- if (pos <= USAGE_OPTS_WIDTH)
- pad = USAGE_OPTS_WIDTH - pos;
- else {
- fputc('\n', stderr);
- pad = USAGE_OPTS_WIDTH;
- }
- fprintf(stderr, "%*s%s\n", pad + USAGE_GAP, "", opts->help);
- }
fputc('\n', stderr);
return PARSE_OPT_HELP;
@@ -559,9 +565,44 @@ void usage_with_options(const char * const *usagestr,
}
int parse_options_usage(const char * const *usagestr,
- const struct option *opts)
+ const struct option *opts,
+ const char *optstr, bool short_opt)
{
- return usage_with_options_internal(usagestr, opts, 0);
+ if (!usagestr)
+ return PARSE_OPT_HELP;
+
+ fprintf(stderr, "\n usage: %s\n", *usagestr++);
+ while (*usagestr && **usagestr)
+ fprintf(stderr, " or: %s\n", *usagestr++);
+ while (*usagestr) {
+ fprintf(stderr, "%s%s\n",
+ **usagestr ? " " : "",
+ *usagestr);
+ usagestr++;
+ }
+ fputc('\n', stderr);
+
+ for ( ; opts->type != OPTION_END; opts++) {
+ if (short_opt) {
+ if (opts->short_name == *optstr)
+ break;
+ continue;
+ }
+
+ if (opts->long_name == NULL)
+ continue;
+
+ if (!prefixcmp(optstr, opts->long_name))
+ break;
+ if (!prefixcmp(optstr, "no-") &&
+ !prefixcmp(optstr + 3, opts->long_name))
+ break;
+ }
+
+ if (opts->type != OPTION_END)
+ print_option_help(opts, 0);
+
+ return PARSE_OPT_HELP;
}
diff --git a/tools/perf/util/parse-options.h b/tools/perf/util/parse-options.h
index 7bb5999940ca..b0241e28eaf7 100644
--- a/tools/perf/util/parse-options.h
+++ b/tools/perf/util/parse-options.h
@@ -158,7 +158,9 @@ struct parse_opt_ctx_t {
};
extern int parse_options_usage(const char * const *usagestr,
- const struct option *opts);
+ const struct option *opts,
+ const char *optstr,
+ bool short_opt);
extern void parse_options_start(struct parse_opt_ctx_t *ctx,
int argc, const char **argv, int flags);
--
1.7.11.7
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 2/5] perf report: Postpone setting up browser after parsing options
2013-11-01 7:33 [PATCHSET 0/5] perf tools: Enhance option parsing error message Namhyung Kim
2013-11-01 7:33 ` [PATCH 1/5] perf tools: Show single option when failed to parse Namhyung Kim
@ 2013-11-01 7:33 ` Namhyung Kim
2013-11-04 20:21 ` [tip:perf/core] " tip-bot for Namhyung Kim
2013-11-01 7:33 ` [PATCH 3/5] perf report: Use parse_options_usage() for -s option failure Namhyung Kim
` (3 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Namhyung Kim @ 2013-11-01 7:33 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, Namhyung Kim, LKML,
Jiri Olsa, David Ahern
From: Namhyung Kim <namhyung.kim@lge.com>
If setup_browser() called earlier than option parsing, the actual
error message can be discarded during the terminal reset. So move it
after setup_sorting() checks whether the sort keys are valid.
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/builtin-report.c | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 98d3891392e2..4df3161c7df2 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -905,13 +905,6 @@ int cmd_report(int argc, const char **argv, const char *prefix __maybe_unused)
input_name = "perf.data";
}
- if (strcmp(input_name, "-") != 0)
- setup_browser(true);
- else {
- use_browser = 0;
- perf_hpp__init();
- }
-
file.path = input_name;
file.force = report.force;
@@ -957,6 +950,18 @@ repeat:
if (setup_sorting() < 0)
usage_with_options(report_usage, options);
+ if (parent_pattern != default_parent_pattern) {
+ if (sort_dimension__add("parent") < 0)
+ goto error;
+ }
+
+ if (strcmp(input_name, "-") != 0)
+ setup_browser(true);
+ else {
+ use_browser = 0;
+ perf_hpp__init();
+ }
+
/*
* Only in the TUI browser we are doing integrated annotation,
* so don't allocate extra space that won't be used in the stdio
@@ -986,11 +991,6 @@ repeat:
if (symbol__init() < 0)
goto error;
- if (parent_pattern != default_parent_pattern) {
- if (sort_dimension__add("parent") < 0)
- goto error;
- }
-
if (argc) {
/*
* Special case: if there's an argument left then assume that
--
1.7.11.7
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 3/5] perf report: Use parse_options_usage() for -s option failure
2013-11-01 7:33 [PATCHSET 0/5] perf tools: Enhance option parsing error message Namhyung Kim
2013-11-01 7:33 ` [PATCH 1/5] perf tools: Show single option when failed to parse Namhyung Kim
2013-11-01 7:33 ` [PATCH 2/5] perf report: Postpone setting up browser after parsing options Namhyung Kim
@ 2013-11-01 7:33 ` Namhyung Kim
2013-11-04 20:21 ` [tip:perf/core] " tip-bot for Namhyung Kim
2013-11-01 7:33 ` [PATCH 4/5] perf top: " Namhyung Kim
` (2 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Namhyung Kim @ 2013-11-01 7:33 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, Namhyung Kim, LKML,
Jiri Olsa, David Ahern
From: Namhyung Kim <namhyung.kim@lge.com>
The -s (--sort) option was processed after normal option parsing so
that it cannot call the parse_options_usage() automatically.
Currently it calls usage_with_options() which shows entire help
messages for event option. Fix it by showing just -s options.
$ perf report -s help
Error: Unknown --sort key: `help'
usage: perf report [<options>]
-s, --sort <key[,key2...]>
sort by key(s): pid, comm, dso, symbol, ...
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/builtin-report.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 4df3161c7df2..25f83d5d66fd 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -947,8 +947,10 @@ repeat:
sort_order = "local_weight,mem,sym,dso,symbol_daddr,dso_daddr,snoop,tlb,locked";
}
- if (setup_sorting() < 0)
- usage_with_options(report_usage, options);
+ if (setup_sorting() < 0) {
+ parse_options_usage(report_usage, options, "s", 1);
+ goto error;
+ }
if (parent_pattern != default_parent_pattern) {
if (sort_dimension__add("parent") < 0)
--
1.7.11.7
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 4/5] perf top: Use parse_options_usage() for -s option failure
2013-11-01 7:33 [PATCHSET 0/5] perf tools: Enhance option parsing error message Namhyung Kim
` (2 preceding siblings ...)
2013-11-01 7:33 ` [PATCH 3/5] perf report: Use parse_options_usage() for -s option failure Namhyung Kim
@ 2013-11-01 7:33 ` Namhyung Kim
2013-11-04 20:22 ` [tip:perf/core] " tip-bot for Namhyung Kim
2013-11-01 7:33 ` [PATCH 5/5] perf stat: Enhance option parse error message Namhyung Kim
2013-11-01 7:38 ` [PATCHSET 0/5] perf tools: Enhance option parsing " Ingo Molnar
5 siblings, 1 reply; 12+ messages in thread
From: Namhyung Kim @ 2013-11-01 7:33 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, Namhyung Kim, LKML,
Jiri Olsa, David Ahern
From: Namhyung Kim <namhyung.kim@lge.com>
The -s (--sort) option was processed after normal option parsing so
that it cannot call the parse_options_usage() automatically.
Currently it calls usage_with_options() which shows entire help
messages for event option. Fix it by showing just -s options.
$ perf top -s help
Error: Unknown --sort key: `help'
usage: perf top [<options>]
-s, --sort <key[,key2...]>
sort by key(s): pid, comm, dso, symbol, ...
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/builtin-top.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 04f5bf2d8e10..a6d166539c23 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -1040,7 +1040,7 @@ parse_percent_limit(const struct option *opt, const char *arg,
int cmd_top(int argc, const char **argv, const char *prefix __maybe_unused)
{
- int status;
+ int status = -1;
char errbuf[BUFSIZ];
struct perf_top top = {
.count_filter = 5,
@@ -1156,8 +1156,10 @@ int cmd_top(int argc, const char **argv, const char *prefix __maybe_unused)
if (sort_order == default_sort_order)
sort_order = "dso,symbol";
- if (setup_sorting() < 0)
- usage_with_options(top_usage, options);
+ if (setup_sorting() < 0) {
+ parse_options_usage(top_usage, options, "s", 1);
+ goto out_delete_evlist;
+ }
/* display thread wants entries to be collapsed in a different tree */
sort__need_collapse = 1;
--
1.7.11.7
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 5/5] perf stat: Enhance option parse error message
2013-11-01 7:33 [PATCHSET 0/5] perf tools: Enhance option parsing error message Namhyung Kim
` (3 preceding siblings ...)
2013-11-01 7:33 ` [PATCH 4/5] perf top: " Namhyung Kim
@ 2013-11-01 7:33 ` Namhyung Kim
2013-11-04 20:22 ` [tip:perf/core] " tip-bot for Namhyung Kim
2013-11-01 7:38 ` [PATCHSET 0/5] perf tools: Enhance option parsing " Ingo Molnar
5 siblings, 1 reply; 12+ messages in thread
From: Namhyung Kim @ 2013-11-01 7:33 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, Namhyung Kim, LKML,
Jiri Olsa, David Ahern
From: Namhyung Kim <namhyung.kim@lge.com>
Print related option help messages only when it failed to process
options. While at it, modify parse_options_usage() to skip usage part
so that it can be used for showing multiple option help messages
naturally like below:
$ perf stat -Bx, ls
-B option not supported with -x
usage: perf stat [<options>] [<command>]
-B, --big-num print large numbers with thousands' separators
-x, --field-separator <separator>
print counts with custom separator
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/builtin-stat.c | 42 ++++++++++++++++++++++++++---------------
tools/perf/util/parse-options.c | 3 ++-
2 files changed, 29 insertions(+), 16 deletions(-)
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 1a9c95d270aa..0fc1c941a73c 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -1596,7 +1596,7 @@ int cmd_stat(int argc, const char **argv, const char *prefix __maybe_unused)
"perf stat [<options>] [<command>]",
NULL
};
- int status = -ENOMEM, run_idx;
+ int status = -EINVAL, run_idx;
const char *mode;
setlocale(LC_ALL, "");
@@ -1614,12 +1614,15 @@ int cmd_stat(int argc, const char **argv, const char *prefix __maybe_unused)
if (output_name && output_fd) {
fprintf(stderr, "cannot use both --output and --log-fd\n");
- usage_with_options(stat_usage, options);
+ parse_options_usage(stat_usage, options, "o", 1);
+ parse_options_usage(NULL, options, "log-fd", 0);
+ goto out;
}
if (output_fd < 0) {
fprintf(stderr, "argument to --log-fd must be a > 0\n");
- usage_with_options(stat_usage, options);
+ parse_options_usage(stat_usage, options, "log-fd", 0);
+ goto out;
}
if (!output) {
@@ -1656,7 +1659,9 @@ int cmd_stat(int argc, const char **argv, const char *prefix __maybe_unused)
/* User explicitly passed -B? */
if (big_num_opt == 1) {
fprintf(stderr, "-B option not supported with -x\n");
- usage_with_options(stat_usage, options);
+ parse_options_usage(stat_usage, options, "B", 1);
+ parse_options_usage(NULL, options, "x", 1);
+ goto out;
} else /* Nope, so disable big number formatting */
big_num = false;
} else if (big_num_opt == 0) /* User passed --no-big-num */
@@ -1666,7 +1671,9 @@ int cmd_stat(int argc, const char **argv, const char *prefix __maybe_unused)
usage_with_options(stat_usage, options);
if (run_count < 0) {
- usage_with_options(stat_usage, options);
+ pr_err("Run count must be a positive number\n");
+ parse_options_usage(stat_usage, options, "r", 1);
+ goto out;
} else if (run_count == 0) {
forever = true;
run_count = 1;
@@ -1678,8 +1685,10 @@ int cmd_stat(int argc, const char **argv, const char *prefix __maybe_unused)
fprintf(stderr, "both cgroup and no-aggregation "
"modes only available in system-wide mode\n");
- usage_with_options(stat_usage, options);
- return -1;
+ parse_options_usage(stat_usage, options, "G", 1);
+ parse_options_usage(NULL, options, "A", 1);
+ parse_options_usage(NULL, options, "a", 1);
+ goto out;
}
if (add_default_attributes())
@@ -1688,25 +1697,28 @@ int cmd_stat(int argc, const char **argv, const char *prefix __maybe_unused)
perf_target__validate(&target);
if (perf_evlist__create_maps(evsel_list, &target) < 0) {
- if (perf_target__has_task(&target))
+ if (perf_target__has_task(&target)) {
pr_err("Problems finding threads of monitor\n");
- if (perf_target__has_cpu(&target))
+ parse_options_usage(stat_usage, options, "p", 1);
+ parse_options_usage(NULL, options, "t", 1);
+ } else if (perf_target__has_cpu(&target)) {
perror("failed to parse CPUs map");
-
- usage_with_options(stat_usage, options);
- return -1;
+ parse_options_usage(stat_usage, options, "C", 1);
+ parse_options_usage(NULL, options, "a", 1);
+ }
+ goto out;
}
if (interval && interval < 100) {
pr_err("print interval must be >= 100ms\n");
- usage_with_options(stat_usage, options);
- return -1;
+ parse_options_usage(stat_usage, options, "I", 1);
+ goto out_free_maps;
}
if (perf_evlist__alloc_stats(evsel_list, interval))
goto out_free_maps;
if (perf_stat_init_aggr_mode())
- goto out;
+ goto out_free_maps;
/*
* We dont want to block the signals - that would cause
diff --git a/tools/perf/util/parse-options.c b/tools/perf/util/parse-options.c
index 1caf7b9a01ee..31f404a032a9 100644
--- a/tools/perf/util/parse-options.c
+++ b/tools/perf/util/parse-options.c
@@ -569,7 +569,7 @@ int parse_options_usage(const char * const *usagestr,
const char *optstr, bool short_opt)
{
if (!usagestr)
- return PARSE_OPT_HELP;
+ goto opt;
fprintf(stderr, "\n usage: %s\n", *usagestr++);
while (*usagestr && **usagestr)
@@ -582,6 +582,7 @@ int parse_options_usage(const char * const *usagestr,
}
fputc('\n', stderr);
+opt:
for ( ; opts->type != OPTION_END; opts++) {
if (short_opt) {
if (opts->short_name == *optstr)
--
1.7.11.7
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCHSET 0/5] perf tools: Enhance option parsing error message
2013-11-01 7:33 [PATCHSET 0/5] perf tools: Enhance option parsing error message Namhyung Kim
` (4 preceding siblings ...)
2013-11-01 7:33 ` [PATCH 5/5] perf stat: Enhance option parse error message Namhyung Kim
@ 2013-11-01 7:38 ` Ingo Molnar
5 siblings, 0 replies; 12+ messages in thread
From: Ingo Molnar @ 2013-11-01 7:38 UTC (permalink / raw)
To: Namhyung Kim
Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, Paul Mackerras,
Namhyung Kim, LKML, Jiri Olsa, David Ahern
* Namhyung Kim <namhyung@kernel.org> wrote:
> Hi,
>
> This patchset tries to enhance error message when perf failed to parse
> option argument. Currently it just shows entire usage and options
> which may fill two pages of screen. It's hard for me to say it's
> really helpful to users. ;-)
>
> With this patch series, it changed to show only related option(s).
> There're also more points to improve yet - like auto-breaking long
> lines, dealing with UIs and so on. But I believe this will be a good
> start.
>
> $ perf report -g help
>
> usage: perf report [<options>]
>
> -g, --call-graph <output_type,min_percent[,print_limit],call_order>
> Display callchains using output_type (graph, flat, fractal, or none) , min percent threshold, optional print limit, callchain order, key (function or address). Default: fractal,0.5,callee,function
>
> $ perf stat -Bx, ls
> -B option not supported with -x
>
> usage: perf stat [<options>] [<command>]
>
> -B, --big-num print large numbers with thousands' separators
> -x, --field-separator <separator>
> print counts with custom separator
>
>
> You can get this on 'perf/option-v1' branch in my tree
>
> git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git
Cool!
Acked-by: Ingo Molnar <mingo@kernel.org>
Reviewed-by: Ingo Molnar <mingo@kernel.org>
Enthusiastically-Supported-by: Ingo Molnar <mingo@kernel.org>
/me runs out of tags
Thanks!
Ingo
^ permalink raw reply [flat|nested] 12+ messages in thread
* [tip:perf/core] perf tools: Show single option when failed to parse
2013-11-01 7:33 ` [PATCH 1/5] perf tools: Show single option when failed to parse Namhyung Kim
@ 2013-11-04 20:21 ` tip-bot for Namhyung Kim
0 siblings, 0 replies; 12+ messages in thread
From: tip-bot for Namhyung Kim @ 2013-11-04 20:21 UTC (permalink / raw)
To: linux-tip-commits
Cc: acme, linux-kernel, paulus, hpa, mingo, a.p.zijlstra,
namhyung.kim, namhyung, jolsa, dsahern, tglx
Commit-ID: ac6976255076d4bf761abfbecb19d46af5b88046
Gitweb: http://git.kernel.org/tip/ac6976255076d4bf761abfbecb19d46af5b88046
Author: Namhyung Kim <namhyung.kim@lge.com>
AuthorDate: Fri, 1 Nov 2013 16:33:11 +0900
Committer: Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 4 Nov 2013 12:51:45 -0300
perf tools: Show single option when failed to parse
Current option parser outputs whole option help string when it failed to
parse an option. However this is not good for user if the command has
many option, she might feel hard which one is related easily.
Fix it by just showing the help message of the given option only.
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Requested-by: Ingo Molnar <mingo@kernel.org>
Acked-by: Ingo Molnar <mingo@kernel.org>
Reviewed-by: Ingo Molnar <mingo@kernel.org>
Enthusiastically-Supported-by: Ingo Molnar <mingo@kernel.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1383291195-24386-2-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/util/parse-options.c | 217 ++++++++++++++++++++++++----------------
tools/perf/util/parse-options.h | 4 +-
2 files changed, 132 insertions(+), 89 deletions(-)
diff --git a/tools/perf/util/parse-options.c b/tools/perf/util/parse-options.c
index 2bc9e70..1caf7b9 100644
--- a/tools/perf/util/parse-options.c
+++ b/tools/perf/util/parse-options.c
@@ -339,10 +339,10 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
if (arg[1] != '-') {
ctx->opt = arg + 1;
if (internal_help && *ctx->opt == 'h')
- return parse_options_usage(usagestr, options);
+ return usage_with_options_internal(usagestr, options, 0);
switch (parse_short_opt(ctx, options)) {
case -1:
- return parse_options_usage(usagestr, options);
+ return parse_options_usage(usagestr, options, arg + 1, 1);
case -2:
goto unknown;
default:
@@ -352,10 +352,11 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
check_typos(arg + 1, options);
while (ctx->opt) {
if (internal_help && *ctx->opt == 'h')
- return parse_options_usage(usagestr, options);
+ return usage_with_options_internal(usagestr, options, 0);
+ arg = ctx->opt;
switch (parse_short_opt(ctx, options)) {
case -1:
- return parse_options_usage(usagestr, options);
+ return parse_options_usage(usagestr, options, arg, 1);
case -2:
/* fake a short option thing to hide the fact that we may have
* started to parse aggregated stuff
@@ -383,12 +384,12 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
if (internal_help && !strcmp(arg + 2, "help-all"))
return usage_with_options_internal(usagestr, options, 1);
if (internal_help && !strcmp(arg + 2, "help"))
- return parse_options_usage(usagestr, options);
+ return usage_with_options_internal(usagestr, options, 0);
if (!strcmp(arg + 2, "list-opts"))
return PARSE_OPT_LIST;
switch (parse_long_opt(ctx, arg + 2, options)) {
case -1:
- return parse_options_usage(usagestr, options);
+ return parse_options_usage(usagestr, options, arg + 2, 0);
case -2:
goto unknown;
default:
@@ -445,6 +446,89 @@ int parse_options(int argc, const char **argv, const struct option *options,
#define USAGE_OPTS_WIDTH 24
#define USAGE_GAP 2
+static void print_option_help(const struct option *opts, int full)
+{
+ size_t pos;
+ int pad;
+
+ if (opts->type == OPTION_GROUP) {
+ fputc('\n', stderr);
+ if (*opts->help)
+ fprintf(stderr, "%s\n", opts->help);
+ return;
+ }
+ if (!full && (opts->flags & PARSE_OPT_HIDDEN))
+ return;
+
+ pos = fprintf(stderr, " ");
+ if (opts->short_name)
+ pos += fprintf(stderr, "-%c", opts->short_name);
+ else
+ pos += fprintf(stderr, " ");
+
+ if (opts->long_name && opts->short_name)
+ pos += fprintf(stderr, ", ");
+ if (opts->long_name)
+ pos += fprintf(stderr, "--%s", opts->long_name);
+
+ switch (opts->type) {
+ case OPTION_ARGUMENT:
+ break;
+ case OPTION_LONG:
+ case OPTION_U64:
+ case OPTION_INTEGER:
+ case OPTION_UINTEGER:
+ if (opts->flags & PARSE_OPT_OPTARG)
+ if (opts->long_name)
+ pos += fprintf(stderr, "[=<n>]");
+ else
+ pos += fprintf(stderr, "[<n>]");
+ else
+ pos += fprintf(stderr, " <n>");
+ break;
+ case OPTION_CALLBACK:
+ if (opts->flags & PARSE_OPT_NOARG)
+ break;
+ /* FALLTHROUGH */
+ case OPTION_STRING:
+ if (opts->argh) {
+ if (opts->flags & PARSE_OPT_OPTARG)
+ if (opts->long_name)
+ pos += fprintf(stderr, "[=<%s>]", opts->argh);
+ else
+ pos += fprintf(stderr, "[<%s>]", opts->argh);
+ else
+ pos += fprintf(stderr, " <%s>", opts->argh);
+ } else {
+ if (opts->flags & PARSE_OPT_OPTARG)
+ if (opts->long_name)
+ pos += fprintf(stderr, "[=...]");
+ else
+ pos += fprintf(stderr, "[...]");
+ else
+ pos += fprintf(stderr, " ...");
+ }
+ break;
+ default: /* OPTION_{BIT,BOOLEAN,SET_UINT,SET_PTR} */
+ case OPTION_END:
+ case OPTION_GROUP:
+ case OPTION_BIT:
+ case OPTION_BOOLEAN:
+ case OPTION_INCR:
+ case OPTION_SET_UINT:
+ case OPTION_SET_PTR:
+ break;
+ }
+
+ if (pos <= USAGE_OPTS_WIDTH)
+ pad = USAGE_OPTS_WIDTH - pos;
+ else {
+ fputc('\n', stderr);
+ pad = USAGE_OPTS_WIDTH;
+ }
+ fprintf(stderr, "%*s%s\n", pad + USAGE_GAP, "", opts->help);
+}
+
int usage_with_options_internal(const char * const *usagestr,
const struct option *opts, int full)
{
@@ -464,87 +548,9 @@ int usage_with_options_internal(const char * const *usagestr,
if (opts->type != OPTION_GROUP)
fputc('\n', stderr);
- for (; opts->type != OPTION_END; opts++) {
- size_t pos;
- int pad;
-
- if (opts->type == OPTION_GROUP) {
- fputc('\n', stderr);
- if (*opts->help)
- fprintf(stderr, "%s\n", opts->help);
- continue;
- }
- if (!full && (opts->flags & PARSE_OPT_HIDDEN))
- continue;
-
- pos = fprintf(stderr, " ");
- if (opts->short_name)
- pos += fprintf(stderr, "-%c", opts->short_name);
- else
- pos += fprintf(stderr, " ");
-
- if (opts->long_name && opts->short_name)
- pos += fprintf(stderr, ", ");
- if (opts->long_name)
- pos += fprintf(stderr, "--%s", opts->long_name);
+ for ( ; opts->type != OPTION_END; opts++)
+ print_option_help(opts, full);
- switch (opts->type) {
- case OPTION_ARGUMENT:
- break;
- case OPTION_LONG:
- case OPTION_U64:
- case OPTION_INTEGER:
- case OPTION_UINTEGER:
- if (opts->flags & PARSE_OPT_OPTARG)
- if (opts->long_name)
- pos += fprintf(stderr, "[=<n>]");
- else
- pos += fprintf(stderr, "[<n>]");
- else
- pos += fprintf(stderr, " <n>");
- break;
- case OPTION_CALLBACK:
- if (opts->flags & PARSE_OPT_NOARG)
- break;
- /* FALLTHROUGH */
- case OPTION_STRING:
- if (opts->argh) {
- if (opts->flags & PARSE_OPT_OPTARG)
- if (opts->long_name)
- pos += fprintf(stderr, "[=<%s>]", opts->argh);
- else
- pos += fprintf(stderr, "[<%s>]", opts->argh);
- else
- pos += fprintf(stderr, " <%s>", opts->argh);
- } else {
- if (opts->flags & PARSE_OPT_OPTARG)
- if (opts->long_name)
- pos += fprintf(stderr, "[=...]");
- else
- pos += fprintf(stderr, "[...]");
- else
- pos += fprintf(stderr, " ...");
- }
- break;
- default: /* OPTION_{BIT,BOOLEAN,SET_UINT,SET_PTR} */
- case OPTION_END:
- case OPTION_GROUP:
- case OPTION_BIT:
- case OPTION_BOOLEAN:
- case OPTION_INCR:
- case OPTION_SET_UINT:
- case OPTION_SET_PTR:
- break;
- }
-
- if (pos <= USAGE_OPTS_WIDTH)
- pad = USAGE_OPTS_WIDTH - pos;
- else {
- fputc('\n', stderr);
- pad = USAGE_OPTS_WIDTH;
- }
- fprintf(stderr, "%*s%s\n", pad + USAGE_GAP, "", opts->help);
- }
fputc('\n', stderr);
return PARSE_OPT_HELP;
@@ -559,9 +565,44 @@ void usage_with_options(const char * const *usagestr,
}
int parse_options_usage(const char * const *usagestr,
- const struct option *opts)
+ const struct option *opts,
+ const char *optstr, bool short_opt)
{
- return usage_with_options_internal(usagestr, opts, 0);
+ if (!usagestr)
+ return PARSE_OPT_HELP;
+
+ fprintf(stderr, "\n usage: %s\n", *usagestr++);
+ while (*usagestr && **usagestr)
+ fprintf(stderr, " or: %s\n", *usagestr++);
+ while (*usagestr) {
+ fprintf(stderr, "%s%s\n",
+ **usagestr ? " " : "",
+ *usagestr);
+ usagestr++;
+ }
+ fputc('\n', stderr);
+
+ for ( ; opts->type != OPTION_END; opts++) {
+ if (short_opt) {
+ if (opts->short_name == *optstr)
+ break;
+ continue;
+ }
+
+ if (opts->long_name == NULL)
+ continue;
+
+ if (!prefixcmp(optstr, opts->long_name))
+ break;
+ if (!prefixcmp(optstr, "no-") &&
+ !prefixcmp(optstr + 3, opts->long_name))
+ break;
+ }
+
+ if (opts->type != OPTION_END)
+ print_option_help(opts, 0);
+
+ return PARSE_OPT_HELP;
}
diff --git a/tools/perf/util/parse-options.h b/tools/perf/util/parse-options.h
index 7bb5999..b0241e2 100644
--- a/tools/perf/util/parse-options.h
+++ b/tools/perf/util/parse-options.h
@@ -158,7 +158,9 @@ struct parse_opt_ctx_t {
};
extern int parse_options_usage(const char * const *usagestr,
- const struct option *opts);
+ const struct option *opts,
+ const char *optstr,
+ bool short_opt);
extern void parse_options_start(struct parse_opt_ctx_t *ctx,
int argc, const char **argv, int flags);
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [tip:perf/core] perf report: Postpone setting up browser after parsing options
2013-11-01 7:33 ` [PATCH 2/5] perf report: Postpone setting up browser after parsing options Namhyung Kim
@ 2013-11-04 20:21 ` tip-bot for Namhyung Kim
0 siblings, 0 replies; 12+ messages in thread
From: tip-bot for Namhyung Kim @ 2013-11-04 20:21 UTC (permalink / raw)
To: linux-tip-commits
Cc: acme, linux-kernel, paulus, hpa, mingo, a.p.zijlstra,
namhyung.kim, namhyung, jolsa, dsahern, tglx
Commit-ID: 4bceffbc26fab2444742db59c6f8124c29e41369
Gitweb: http://git.kernel.org/tip/4bceffbc26fab2444742db59c6f8124c29e41369
Author: Namhyung Kim <namhyung.kim@lge.com>
AuthorDate: Fri, 1 Nov 2013 16:33:12 +0900
Committer: Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 4 Nov 2013 12:54:32 -0300
perf report: Postpone setting up browser after parsing options
If setup_browser() called earlier than option parsing, the actual error
message can be discarded during the terminal reset. So move it after
setup_sorting() checks whether the sort keys are valid.
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Acked-by: Ingo Molnar <mingo@kernel.org>
Reviewed-by: Ingo Molnar <mingo@kernel.org>
Enthusiastically-Supported-by: Ingo Molnar <mingo@kernel.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1383291195-24386-3-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/builtin-report.c | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 98d3891..4df3161 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -905,13 +905,6 @@ int cmd_report(int argc, const char **argv, const char *prefix __maybe_unused)
input_name = "perf.data";
}
- if (strcmp(input_name, "-") != 0)
- setup_browser(true);
- else {
- use_browser = 0;
- perf_hpp__init();
- }
-
file.path = input_name;
file.force = report.force;
@@ -957,6 +950,18 @@ repeat:
if (setup_sorting() < 0)
usage_with_options(report_usage, options);
+ if (parent_pattern != default_parent_pattern) {
+ if (sort_dimension__add("parent") < 0)
+ goto error;
+ }
+
+ if (strcmp(input_name, "-") != 0)
+ setup_browser(true);
+ else {
+ use_browser = 0;
+ perf_hpp__init();
+ }
+
/*
* Only in the TUI browser we are doing integrated annotation,
* so don't allocate extra space that won't be used in the stdio
@@ -986,11 +991,6 @@ repeat:
if (symbol__init() < 0)
goto error;
- if (parent_pattern != default_parent_pattern) {
- if (sort_dimension__add("parent") < 0)
- goto error;
- }
-
if (argc) {
/*
* Special case: if there's an argument left then assume that
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [tip:perf/core] perf report: Use parse_options_usage() for -s option failure
2013-11-01 7:33 ` [PATCH 3/5] perf report: Use parse_options_usage() for -s option failure Namhyung Kim
@ 2013-11-04 20:21 ` tip-bot for Namhyung Kim
0 siblings, 0 replies; 12+ messages in thread
From: tip-bot for Namhyung Kim @ 2013-11-04 20:21 UTC (permalink / raw)
To: linux-tip-commits
Cc: acme, linux-kernel, paulus, hpa, mingo, a.p.zijlstra,
namhyung.kim, namhyung, jolsa, dsahern, tglx
Commit-ID: 91aba0a62e24ff7a567e13e1d88deab711df6f0f
Gitweb: http://git.kernel.org/tip/91aba0a62e24ff7a567e13e1d88deab711df6f0f
Author: Namhyung Kim <namhyung.kim@lge.com>
AuthorDate: Fri, 1 Nov 2013 16:33:13 +0900
Committer: Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 4 Nov 2013 12:55:17 -0300
perf report: Use parse_options_usage() for -s option failure
The -s (--sort) option was processed after normal option parsing so that
it cannot call the parse_options_usage() automatically. Currently it
calls usage_with_options() which shows entire help messages for event
option. Fix it by showing just -s options.
$ perf report -s help
Error: Unknown --sort key: `help'
usage: perf report [<options>]
-s, --sort <key[,key2...]>
sort by key(s): pid, comm, dso, symbol, ...
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Acked-by: Ingo Molnar <mingo@kernel.org>
Reviewed-by: Ingo Molnar <mingo@kernel.org>
Enthusiastically-Supported-by: Ingo Molnar <mingo@kernel.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1383291195-24386-4-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/builtin-report.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 4df3161..25f83d5 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -947,8 +947,10 @@ repeat:
sort_order = "local_weight,mem,sym,dso,symbol_daddr,dso_daddr,snoop,tlb,locked";
}
- if (setup_sorting() < 0)
- usage_with_options(report_usage, options);
+ if (setup_sorting() < 0) {
+ parse_options_usage(report_usage, options, "s", 1);
+ goto error;
+ }
if (parent_pattern != default_parent_pattern) {
if (sort_dimension__add("parent") < 0)
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [tip:perf/core] perf top: Use parse_options_usage() for -s option failure
2013-11-01 7:33 ` [PATCH 4/5] perf top: " Namhyung Kim
@ 2013-11-04 20:22 ` tip-bot for Namhyung Kim
0 siblings, 0 replies; 12+ messages in thread
From: tip-bot for Namhyung Kim @ 2013-11-04 20:22 UTC (permalink / raw)
To: linux-tip-commits
Cc: acme, linux-kernel, paulus, hpa, mingo, a.p.zijlstra,
namhyung.kim, namhyung, jolsa, dsahern, tglx
Commit-ID: d37a92dcb45094dc02836c8a77c693c6f9916fb2
Gitweb: http://git.kernel.org/tip/d37a92dcb45094dc02836c8a77c693c6f9916fb2
Author: Namhyung Kim <namhyung.kim@lge.com>
AuthorDate: Fri, 1 Nov 2013 16:33:14 +0900
Committer: Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 4 Nov 2013 12:56:41 -0300
perf top: Use parse_options_usage() for -s option failure
The -s (--sort) option was processed after normal option parsing so that
it cannot call the parse_options_usage() automatically. Currently it
calls usage_with_options() which shows entire help messages for event
option. Fix it by showing just -s options.
$ perf top -s help
Error: Unknown --sort key: `help'
usage: perf top [<options>]
-s, --sort <key[,key2...]>
sort by key(s): pid, comm, dso, symbol, ...
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Acked-by: Ingo Molnar <mingo@kernel.org>
Reviewed-by: Ingo Molnar <mingo@kernel.org>
Enthusiastically-Supported-by: Ingo Molnar <mingo@kernel.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1383291195-24386-5-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/builtin-top.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 21db76d..ca5ca37 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -1040,7 +1040,7 @@ parse_percent_limit(const struct option *opt, const char *arg,
int cmd_top(int argc, const char **argv, const char *prefix __maybe_unused)
{
- int status;
+ int status = -1;
char errbuf[BUFSIZ];
struct perf_top top = {
.count_filter = 5,
@@ -1159,8 +1159,10 @@ int cmd_top(int argc, const char **argv, const char *prefix __maybe_unused)
if (sort_order == default_sort_order)
sort_order = "dso,symbol";
- if (setup_sorting() < 0)
- usage_with_options(top_usage, options);
+ if (setup_sorting() < 0) {
+ parse_options_usage(top_usage, options, "s", 1);
+ goto out_delete_evlist;
+ }
/* display thread wants entries to be collapsed in a different tree */
sort__need_collapse = 1;
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [tip:perf/core] perf stat: Enhance option parse error message
2013-11-01 7:33 ` [PATCH 5/5] perf stat: Enhance option parse error message Namhyung Kim
@ 2013-11-04 20:22 ` tip-bot for Namhyung Kim
0 siblings, 0 replies; 12+ messages in thread
From: tip-bot for Namhyung Kim @ 2013-11-04 20:22 UTC (permalink / raw)
To: linux-tip-commits
Cc: acme, linux-kernel, paulus, hpa, mingo, a.p.zijlstra,
namhyung.kim, namhyung, jolsa, dsahern, tglx
Commit-ID: cc03c54296ccbeca5363dfe8f49af42d14960f28
Gitweb: http://git.kernel.org/tip/cc03c54296ccbeca5363dfe8f49af42d14960f28
Author: Namhyung Kim <namhyung.kim@lge.com>
AuthorDate: Fri, 1 Nov 2013 16:33:15 +0900
Committer: Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 4 Nov 2013 12:57:36 -0300
perf stat: Enhance option parse error message
Print related option help messages only when it failed to process
options. While at it, modify parse_options_usage() to skip usage part
so that it can be used for showing multiple option help messages
naturally like below:
$ perf stat -Bx, ls
-B option not supported with -x
usage: perf stat [<options>] [<command>]
-B, --big-num print large numbers with thousands' separators
-x, --field-separator <separator>
print counts with custom separator
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Acked-by: Ingo Molnar <mingo@kernel.org>
Reviewed-by: Ingo Molnar <mingo@kernel.org>
Enthusiastically-Supported-by: Ingo Molnar <mingo@kernel.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1383291195-24386-6-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/builtin-stat.c | 42 ++++++++++++++++++++++++++---------------
tools/perf/util/parse-options.c | 3 ++-
2 files changed, 29 insertions(+), 16 deletions(-)
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 1a9c95d..0fc1c94 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -1596,7 +1596,7 @@ int cmd_stat(int argc, const char **argv, const char *prefix __maybe_unused)
"perf stat [<options>] [<command>]",
NULL
};
- int status = -ENOMEM, run_idx;
+ int status = -EINVAL, run_idx;
const char *mode;
setlocale(LC_ALL, "");
@@ -1614,12 +1614,15 @@ int cmd_stat(int argc, const char **argv, const char *prefix __maybe_unused)
if (output_name && output_fd) {
fprintf(stderr, "cannot use both --output and --log-fd\n");
- usage_with_options(stat_usage, options);
+ parse_options_usage(stat_usage, options, "o", 1);
+ parse_options_usage(NULL, options, "log-fd", 0);
+ goto out;
}
if (output_fd < 0) {
fprintf(stderr, "argument to --log-fd must be a > 0\n");
- usage_with_options(stat_usage, options);
+ parse_options_usage(stat_usage, options, "log-fd", 0);
+ goto out;
}
if (!output) {
@@ -1656,7 +1659,9 @@ int cmd_stat(int argc, const char **argv, const char *prefix __maybe_unused)
/* User explicitly passed -B? */
if (big_num_opt == 1) {
fprintf(stderr, "-B option not supported with -x\n");
- usage_with_options(stat_usage, options);
+ parse_options_usage(stat_usage, options, "B", 1);
+ parse_options_usage(NULL, options, "x", 1);
+ goto out;
} else /* Nope, so disable big number formatting */
big_num = false;
} else if (big_num_opt == 0) /* User passed --no-big-num */
@@ -1666,7 +1671,9 @@ int cmd_stat(int argc, const char **argv, const char *prefix __maybe_unused)
usage_with_options(stat_usage, options);
if (run_count < 0) {
- usage_with_options(stat_usage, options);
+ pr_err("Run count must be a positive number\n");
+ parse_options_usage(stat_usage, options, "r", 1);
+ goto out;
} else if (run_count == 0) {
forever = true;
run_count = 1;
@@ -1678,8 +1685,10 @@ int cmd_stat(int argc, const char **argv, const char *prefix __maybe_unused)
fprintf(stderr, "both cgroup and no-aggregation "
"modes only available in system-wide mode\n");
- usage_with_options(stat_usage, options);
- return -1;
+ parse_options_usage(stat_usage, options, "G", 1);
+ parse_options_usage(NULL, options, "A", 1);
+ parse_options_usage(NULL, options, "a", 1);
+ goto out;
}
if (add_default_attributes())
@@ -1688,25 +1697,28 @@ int cmd_stat(int argc, const char **argv, const char *prefix __maybe_unused)
perf_target__validate(&target);
if (perf_evlist__create_maps(evsel_list, &target) < 0) {
- if (perf_target__has_task(&target))
+ if (perf_target__has_task(&target)) {
pr_err("Problems finding threads of monitor\n");
- if (perf_target__has_cpu(&target))
+ parse_options_usage(stat_usage, options, "p", 1);
+ parse_options_usage(NULL, options, "t", 1);
+ } else if (perf_target__has_cpu(&target)) {
perror("failed to parse CPUs map");
-
- usage_with_options(stat_usage, options);
- return -1;
+ parse_options_usage(stat_usage, options, "C", 1);
+ parse_options_usage(NULL, options, "a", 1);
+ }
+ goto out;
}
if (interval && interval < 100) {
pr_err("print interval must be >= 100ms\n");
- usage_with_options(stat_usage, options);
- return -1;
+ parse_options_usage(stat_usage, options, "I", 1);
+ goto out_free_maps;
}
if (perf_evlist__alloc_stats(evsel_list, interval))
goto out_free_maps;
if (perf_stat_init_aggr_mode())
- goto out;
+ goto out_free_maps;
/*
* We dont want to block the signals - that would cause
diff --git a/tools/perf/util/parse-options.c b/tools/perf/util/parse-options.c
index 1caf7b9..31f404a 100644
--- a/tools/perf/util/parse-options.c
+++ b/tools/perf/util/parse-options.c
@@ -569,7 +569,7 @@ int parse_options_usage(const char * const *usagestr,
const char *optstr, bool short_opt)
{
if (!usagestr)
- return PARSE_OPT_HELP;
+ goto opt;
fprintf(stderr, "\n usage: %s\n", *usagestr++);
while (*usagestr && **usagestr)
@@ -582,6 +582,7 @@ int parse_options_usage(const char * const *usagestr,
}
fputc('\n', stderr);
+opt:
for ( ; opts->type != OPTION_END; opts++) {
if (short_opt) {
if (opts->short_name == *optstr)
^ permalink raw reply related [flat|nested] 12+ messages in thread
end of thread, other threads:[~2013-11-04 20:22 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-11-01 7:33 [PATCHSET 0/5] perf tools: Enhance option parsing error message Namhyung Kim
2013-11-01 7:33 ` [PATCH 1/5] perf tools: Show single option when failed to parse Namhyung Kim
2013-11-04 20:21 ` [tip:perf/core] " tip-bot for Namhyung Kim
2013-11-01 7:33 ` [PATCH 2/5] perf report: Postpone setting up browser after parsing options Namhyung Kim
2013-11-04 20:21 ` [tip:perf/core] " tip-bot for Namhyung Kim
2013-11-01 7:33 ` [PATCH 3/5] perf report: Use parse_options_usage() for -s option failure Namhyung Kim
2013-11-04 20:21 ` [tip:perf/core] " tip-bot for Namhyung Kim
2013-11-01 7:33 ` [PATCH 4/5] perf top: " Namhyung Kim
2013-11-04 20:22 ` [tip:perf/core] " tip-bot for Namhyung Kim
2013-11-01 7:33 ` [PATCH 5/5] perf stat: Enhance option parse error message Namhyung Kim
2013-11-04 20:22 ` [tip:perf/core] " tip-bot for Namhyung Kim
2013-11-01 7:38 ` [PATCHSET 0/5] perf tools: Enhance option parsing " Ingo Molnar
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).