Em Sun, Oct 25, 2015 at 12:49:27AM +0900, Namhyung Kim escreveu: > Now usage_with_options() setup a pager before printing message so normal > printf() or pr_err() will not be shown. The usage_with_options_msg() > can be used to print some help message before usage strings.
Haven't tested, but intent looks good, Masami, could you please check if all is ok and provide an Acked-by and/or Tested-by? - Arnaldo > Cc: Masami Hiramatsu <[email protected]> > Signed-off-by: Namhyung Kim <[email protected]> > --- > tools/perf/builtin-evlist.c | 4 ++-- > tools/perf/builtin-probe.c | 20 ++++++++++++-------- > tools/perf/builtin-record.c | 11 ++++++----- > tools/perf/builtin-sched.c | 4 ++-- > tools/perf/builtin-script.c | 8 ++++---- > tools/perf/util/parse-options.c | 15 +++++++++++++++ > tools/perf/util/parse-options.h | 4 ++++ > tools/perf/util/strbuf.c | 22 +++++++++++++++------- > tools/perf/util/strbuf.h | 2 ++ > 9 files changed, 62 insertions(+), 28 deletions(-) > > diff --git a/tools/perf/builtin-evlist.c b/tools/perf/builtin-evlist.c > index 695ec5a50cf2..f4d62510acbb 100644 > --- a/tools/perf/builtin-evlist.c > +++ b/tools/perf/builtin-evlist.c > @@ -61,8 +61,8 @@ int cmd_evlist(int argc, const char **argv, const char > *prefix __maybe_unused) > usage_with_options(evlist_usage, options); > > if (details.event_group && (details.verbose || details.freq)) { > - pr_err("--group option is not compatible with other options\n"); > - usage_with_options(evlist_usage, options); > + usage_with_options_msg(evlist_usage, options, > + "--group option is not compatible with other > options\n"); > } > > return __cmd_evlist(input_name, &details); > diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c > index 530c3a28a58c..132afc97676c 100644 > --- a/tools/perf/builtin-probe.c > +++ b/tools/perf/builtin-probe.c > @@ -528,12 +528,12 @@ __cmd_probe(int argc, const char **argv, const char > *prefix __maybe_unused) > PARSE_OPT_STOP_AT_NON_OPTION); > if (argc > 0) { > if (strcmp(argv[0], "-") == 0) { > - pr_warning(" Error: '-' is not supported.\n"); > - usage_with_options(probe_usage, options); > + usage_with_options_msg(probe_usage, options, > + "'-' is not supported.\n"); > } > if (params.command && params.command != 'a') { > - pr_warning(" Error: another command except --add is > set.\n"); > - usage_with_options(probe_usage, options); > + usage_with_options_msg(probe_usage, options, > + "another command except --add is set.\n"); > } > ret = parse_probe_event_argv(argc, argv); > if (ret < 0) { > @@ -562,8 +562,10 @@ __cmd_probe(int argc, const char **argv, const char > *prefix __maybe_unused) > switch (params.command) { > case 'l': > if (params.uprobes) { > - pr_warning(" Error: Don't use --list with --exec.\n"); > - usage_with_options(probe_usage, options); > + pr_err(" Error: Don't use --list with --exec.\n"); > + parse_options_usage(probe_usage, options, "l", true); > + parse_options_usage(NULL, options, "x", true); > + return -EINVAL; > } > ret = show_perf_probe_events(params.filter); > if (ret < 0) > @@ -603,8 +605,10 @@ __cmd_probe(int argc, const char **argv, const char > *prefix __maybe_unused) > case 'a': > /* Ensure the last given target is used */ > if (params.target && !params.target_used) { > - pr_warning(" Error: -x/-m must follow the probe > definitions.\n"); > - usage_with_options(probe_usage, options); > + pr_err(" Error: -x/-m must follow the probe > definitions.\n"); > + parse_options_usage(probe_usage, options, "m", true); > + parse_options_usage(NULL, options, "x", true); > + return -EINVAL; > } > > ret = perf_add_probe_events(params.events, params.nevents); > diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c > index 2740d7a82ae8..de02267c73d8 100644 > --- a/tools/perf/builtin-record.c > +++ b/tools/perf/builtin-record.c > @@ -1135,14 +1135,15 @@ int cmd_record(int argc, const char **argv, const > char *prefix __maybe_unused) > usage_with_options(record_usage, record_options); > > if (nr_cgroups && !rec->opts.target.system_wide) { > - ui__error("cgroup monitoring only available in" > - " system-wide mode\n"); > - usage_with_options(record_usage, record_options); > + usage_with_options_msg(record_usage, record_options, > + "cgroup monitoring only available in system-wide mode"); > + > } > if (rec->opts.record_switch_events && > !perf_can_record_switch_events()) { > - ui__error("kernel does not support recording context switch > events (--switch-events option)\n"); > - usage_with_options(record_usage, record_options); > + ui__error("kernel does not support recording context switch > events\n"); > + parse_options_usage(record_usage, record_options, > "switch-events", 0); > + return -EINVAL; > } > > if (!rec->itr) { > diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c > index 33962612a5e9..0ee6d900e100 100644 > --- a/tools/perf/builtin-sched.c > +++ b/tools/perf/builtin-sched.c > @@ -1728,8 +1728,8 @@ static void setup_sorting(struct perf_sched *sched, > const struct option *options > for (tok = strtok_r(str, ", ", &tmp); > tok; tok = strtok_r(NULL, ", ", &tmp)) { > if (sort_dimension__add(tok, &sched->sort_list) < 0) { > - error("Unknown --sort key: `%s'", tok); > - usage_with_options(usage_msg, options); > + usage_with_options_msg(usage_msg, options, > + "Unknown --sort key: `%s'", tok); > } > } > > diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c > index 2653c0273b89..278acb22f029 100644 > --- a/tools/perf/builtin-script.c > +++ b/tools/perf/builtin-script.c > @@ -1767,9 +1767,9 @@ int cmd_script(int argc, const char **argv, const char > *prefix __maybe_unused) > rep_script_path = get_script_path(argv[0], REPORT_SUFFIX); > > if (!rec_script_path && !rep_script_path) { > - fprintf(stderr, " Couldn't find script %s\n\n See perf" > + usage_with_options_msg(script_usage, options, > + "Couldn't find script `%s'\n\n See perf" > " script -l for available scripts.\n", argv[0]); > - usage_with_options(script_usage, options); > } > > if (is_top_script(argv[0])) { > @@ -1780,10 +1780,10 @@ int cmd_script(int argc, const char **argv, const > char *prefix __maybe_unused) > rep_args = has_required_arg(rep_script_path); > rec_args = (argc - 1) - rep_args; > if (rec_args < 0) { > - fprintf(stderr, " %s script requires options." > + usage_with_options_msg(script_usage, options, > + "`%s' script requires options." > "\n\n See perf script -l for available " > "scripts and options.\n", argv[0]); > - usage_with_options(script_usage, options); > } > } > > diff --git a/tools/perf/util/parse-options.c b/tools/perf/util/parse-options.c > index eeeed98eb26d..230e771407a3 100644 > --- a/tools/perf/util/parse-options.c > +++ b/tools/perf/util/parse-options.c > @@ -760,6 +760,21 @@ void usage_with_options(const char * const *usagestr, > exit(129); > } > > +void usage_with_options_msg(const char * const *usagestr, > + const struct option *opts, const char *fmt, ...) > +{ > + va_list ap; > + > + exit_browser(false); > + > + va_start(ap, fmt); > + strbuf_addv(&error_buf, fmt, ap); > + va_end(ap); > + > + usage_with_options_internal(usagestr, opts, 0, NULL); > + exit(129); > +} > + > int parse_options_usage(const char * const *usagestr, > const struct option *opts, > const char *optstr, bool short_opt) > diff --git a/tools/perf/util/parse-options.h b/tools/perf/util/parse-options.h > index 182c86099330..a8e407bc251e 100644 > --- a/tools/perf/util/parse-options.h > +++ b/tools/perf/util/parse-options.h > @@ -161,6 +161,10 @@ extern int parse_options_subcommand(int argc, const char > **argv, > > extern NORETURN void usage_with_options(const char * const *usagestr, > const struct option *options); > +extern NORETURN __attribute__((format(printf,3,4))) > +void usage_with_options_msg(const char * const *usagestr, > + const struct option *options, > + const char *fmt, ...); > > /*----- incremantal advanced APIs -----*/ > > diff --git a/tools/perf/util/strbuf.c b/tools/perf/util/strbuf.c > index 4abe23550c73..25671fa16618 100644 > --- a/tools/perf/util/strbuf.c > +++ b/tools/perf/util/strbuf.c > @@ -82,23 +82,22 @@ void strbuf_add(struct strbuf *sb, const void *data, > size_t len) > strbuf_setlen(sb, sb->len + len); > } > > -void strbuf_addf(struct strbuf *sb, const char *fmt, ...) > +void strbuf_addv(struct strbuf *sb, const char *fmt, va_list ap) > { > int len; > - va_list ap; > + va_list ap_saved; > > if (!strbuf_avail(sb)) > strbuf_grow(sb, 64); > - va_start(ap, fmt); > + > + va_copy(ap_saved, ap); > len = vsnprintf(sb->buf + sb->len, sb->alloc - sb->len, fmt, ap); > - va_end(ap); > if (len < 0) > die("your vsnprintf is broken"); > if (len > strbuf_avail(sb)) { > strbuf_grow(sb, len); > - va_start(ap, fmt); > - len = vsnprintf(sb->buf + sb->len, sb->alloc - sb->len, fmt, > ap); > - va_end(ap); > + len = vsnprintf(sb->buf + sb->len, sb->alloc - sb->len, fmt, > ap_saved); > + va_end(ap_saved); > if (len > strbuf_avail(sb)) { > die("this should not happen, your vsnprintf is broken"); > } > @@ -106,6 +105,15 @@ void strbuf_addf(struct strbuf *sb, const char *fmt, ...) > strbuf_setlen(sb, sb->len + len); > } > > +void strbuf_addf(struct strbuf *sb, const char *fmt, ...) > +{ > + va_list ap; > + > + va_start(ap, fmt); > + strbuf_addv(sb, fmt, ap); > + va_end(ap); > +} > + > ssize_t strbuf_read(struct strbuf *sb, int fd, ssize_t hint) > { > size_t oldlen = sb->len; > diff --git a/tools/perf/util/strbuf.h b/tools/perf/util/strbuf.h > index 436ac319f6c7..529f2f035249 100644 > --- a/tools/perf/util/strbuf.h > +++ b/tools/perf/util/strbuf.h > @@ -39,6 +39,7 @@ > */ > > #include <assert.h> > +#include <stdarg.h> > > extern char strbuf_slopbuf[]; > struct strbuf { > @@ -85,6 +86,7 @@ static inline void strbuf_addstr(struct strbuf *sb, const > char *s) { > > __attribute__((format(printf,2,3))) > extern void strbuf_addf(struct strbuf *sb, const char *fmt, ...); > +extern void strbuf_addv(struct strbuf *sb, const char *fmt, va_list ap); > > /* XXX: if read fails, any partial read is undone */ > extern ssize_t strbuf_read(struct strbuf *, int fd, ssize_t hint); > -- > 2.6.0 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
