[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20200708173423.GC22437@kernel.org>
Date: Wed, 8 Jul 2020 14:34:23 -0300
From: Arnaldo Carvalho de Melo <acme@...nel.org>
To: Changbin Du <changbin.du@...il.com>
Cc: Jiri Olsa <jolsa@...hat.com>,
Peter Zijlstra <peterz@...radead.org>,
Ingo Molnar <mingo@...hat.com>,
Namhyung Kim <namhyung@...nel.org>,
Steven Rostedt <rostedt@...dmis.org>,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 09/17] perf: util: add general function to parse
sublevel options
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@...co 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@...co perf]$
Good job identifying this code and reusing it!
- Arnaldo
> Signed-off-by: Changbin Du <changbin.du@...il.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
Powered by blists - more mailing lists