Messages in this thread | | | Date | Fri, 10 Jul 2020 21:14:53 +0800 | From | Changbin Du <> | Subject | Re: [PATCH v3 09/17] perf: util: add general function to parse sublevel options |
| |
On Wed, Jul 08, 2020 at 02:34:23PM -0300, Arnaldo Carvalho de Melo wrote: > Em Thu, Jul 09, 2020 at 12:45:57AM +0800, Changbin Du escreveu: > > This factors out a general function perf_parse_sublevel_options() to parse > > sublevel options. The 'sublevel' options is something like the '--debug' > > options which allow more sublevel options. > > Please don't add stuff to util.h/util.c, its too generic a name and > we've been trying to move things to more appropriate locations, so > follow the example of: > > [acme@quaco perf]$ ls -la tools/perf/util/parse-regs-options.* > -rw-rw-r--. 1 acme acme 1932 Feb 12 15:11 tools/perf/util/parse-regs-options.c > -rw-rw-r--. 1 acme acme 316 Dec 20 2019 tools/perf/util/parse-regs-options.h > [acme@quaco perf]$ > I crated files util/parse-sublevel-options.c and util/parse-sublevel-options.h to hold this new function.
> Good job identifying this code and reusing it! > > - Arnaldo > > > Signed-off-by: Changbin Du <changbin.du@gmail.com> > > --- > > tools/perf/util/debug.c | 61 ++++++++++++----------------------------- > > tools/perf/util/util.c | 56 +++++++++++++++++++++++++++++++++++++ > > tools/perf/util/util.h | 7 +++++ > > 3 files changed, 80 insertions(+), 44 deletions(-) > > > > diff --git a/tools/perf/util/debug.c b/tools/perf/util/debug.c > > index adb656745ecc..79999c57a609 100644 > > --- a/tools/perf/util/debug.c > > +++ b/tools/perf/util/debug.c > > @@ -20,6 +20,7 @@ > > #include "target.h" > > #include "ui/helpline.h" > > #include "ui/ui.h" > > +#include "util/util.h" > > > > #include <linux/ctype.h> > > > > @@ -173,65 +174,37 @@ void trace_event(union perf_event *event) > > trace_event_printer, event); > > } > > > > -static struct debug_variable { > > - const char *name; > > - int *ptr; > > -} debug_variables[] = { > > - { .name = "verbose", .ptr = &verbose }, > > - { .name = "ordered-events", .ptr = &debug_ordered_events}, > > - { .name = "stderr", .ptr = &redirect_to_stderr}, > > - { .name = "data-convert", .ptr = &debug_data_convert }, > > - { .name = "perf-event-open", .ptr = &debug_peo_args }, > > +static struct sublevel_option debug_opts[] = { > > + { .name = "verbose", .value_ptr = &verbose }, > > + { .name = "ordered-events", .value_ptr = &debug_ordered_events}, > > + { .name = "stderr", .value_ptr = &redirect_to_stderr}, > > + { .name = "data-convert", .value_ptr = &debug_data_convert }, > > + { .name = "perf-event-open", .value_ptr = &debug_peo_args }, > > { .name = NULL, } > > }; > > > > int perf_debug_option(const char *str) > > { > > - struct debug_variable *var = &debug_variables[0]; > > - char *vstr, *s = strdup(str); > > - int v = 1; > > - > > - vstr = strchr(s, '='); > > - if (vstr) > > - *vstr++ = 0; > > - > > - while (var->name) { > > - if (!strcmp(s, var->name)) > > - break; > > - var++; > > - } > > - > > - if (!var->name) { > > - pr_err("Unknown debug variable name '%s'\n", s); > > - free(s); > > - return -1; > > - } > > + int ret; > > > > - if (vstr) { > > - v = atoi(vstr); > > - /* > > - * Allow only values in range (0, 10), > > - * otherwise set 0. > > - */ > > - v = (v < 0) || (v > 10) ? 0 : v; > > - } > > + ret = perf_parse_sublevel_options(str, debug_opts); > > + if (ret) > > + return ret; > > > > - if (quiet) > > - v = -1; > > + /* Allow only verbose value in range (0, 10), otherwise set 0. */ > > + verbose = (verbose < 0) || (verbose > 10) ? 0 : verbose; > > > > - *var->ptr = v; > > - free(s); > > return 0; > > } > > > > int perf_quiet_option(void) > > { > > - struct debug_variable *var = &debug_variables[0]; > > + struct sublevel_option *opt = &debug_opts[0]; > > > > /* disable all debug messages */ > > - while (var->name) { > > - *var->ptr = -1; > > - var++; > > + while (opt->name) { > > + *opt->value_ptr = -1; > > + opt++; > > } > > > > return 0; > > diff --git a/tools/perf/util/util.c b/tools/perf/util/util.c > > index 37a9492edb3e..7e532a93835b 100644 > > --- a/tools/perf/util/util.c > > +++ b/tools/perf/util/util.c > > @@ -416,3 +416,59 @@ char *perf_exe(char *buf, int len) > > } > > return strcpy(buf, "perf"); > > } > > + > > +static int parse_one_sublevel_option(const char *str, > > + struct sublevel_option *opts) > > +{ > > + struct sublevel_option *opt = &opts[0]; > > + char *vstr, *s = strdup(str); > > + int v = 1; > > + > > + vstr = strchr(s, '='); > > + if (vstr) > > + *vstr++ = 0; > > + > > + while (opt->name) { > > + if (!strcmp(s, opt->name)) > > + break; > > + opt++; > > + } > > + > > + if (!opt->name) { > > + pr_err("Unknown option name '%s'\n", s); > > + free(s); > > + return -1; > > + } > > + > > + if (vstr) > > + v = atoi(vstr); > > + > > + *opt->value_ptr = v; > > + free(s); > > + return 0; > > +} > > + > > +/* parse options like --foo a=<n>,b,c... */ > > +int perf_parse_sublevel_options(const char *str, struct sublevel_option *opts) > > +{ > > + char *s = strdup(str); > > + char *p = NULL; > > + int ret; > > + > > + if (!s) > > + return -1; > > + > > + p = strtok(s, ","); > > + while (p) { > > + ret = parse_one_sublevel_option(p, opts); > > + if (ret) { > > + free(s); > > + return ret; > > + } > > + > > + p = strtok(NULL, ","); > > + } > > + > > + free(s); > > + return 0; > > +} > > \ No newline at end of file > > diff --git a/tools/perf/util/util.h b/tools/perf/util/util.h > > index f486fdd3a538..8cb1f980935c 100644 > > --- a/tools/perf/util/util.h > > +++ b/tools/perf/util/util.h > > @@ -52,6 +52,13 @@ void perf_set_multithreaded(void); > > > > char *perf_exe(char *buf, int len); > > > > +struct sublevel_option { > > + const char *name; > > + int *value_ptr; > > +}; > > + > > +int perf_parse_sublevel_options(const char *str, struct sublevel_option *opts); > > + > > #ifndef O_CLOEXEC > > #ifdef __sparc__ > > #define O_CLOEXEC 0x400000 > > -- > > 2.25.1 > > > > -- > > - Arnaldo
-- Cheers, Changbin Du
| |