>From: Namhyung Kim [mailto:[email protected]] On Behalf Of Namhyung Kim > >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.
Thanks! looks good to me :) Acked-by: Masami Hiramatsu <[email protected]> > >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 N�����r��y����b�X��ǧv�^�){.n�+����{����zX����ܨ}���Ơz�&j:+v�������zZ+��+zf���h���~����i���z� �w���?�����&�)ߢf��^jǫy�m��@A�a��� 0��h���i
