Messages in this thread | | | Date | Sat, 11 Jul 2020 20:37:25 +0800 | From | Changbin Du <> | Subject | Re: [PATCH v4 08/17] perf: util: add general function to parse sublevel options |
| |
On Fri, Jul 10, 2020 at 11:29:49PM +0900, Namhyung Kim wrote: > On Fri, Jul 10, 2020 at 10:44 PM Changbin Du <changbin.du@gmail.com> wrote: > > > > 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. > > > > Signed-off-by: Changbin Du <changbin.du@gmail.com> > > > > --- > > v2: add util/parse-sublevel-options.c > > --- > [snip] > > diff --git a/tools/perf/util/parse-sublevel-options.c b/tools/perf/util/parse-sublevel-options.c > > new file mode 100644 > > index 000000000000..39798568547c > > --- /dev/null > > +++ b/tools/perf/util/parse-sublevel-options.c > > @@ -0,0 +1,63 @@ > > +#include <stdlib.h> > > +#include <stdint.h> > > +#include <string.h> > > +#include <stdio.h> > > + > > +#include "util/debug.h" > > +#include "util/parse-sublevel-options.h" > > + > > +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; > > I know you just copied the code, but let's add a check for > the return value of strdup(). > yes, the 's' should be checked here.
> Also I think you can just use the opts pointer.. > For this, personally I prefer to opt as it's singular form. Because we are dealing with single element.
> > > + > > + 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; > > +} > > diff --git a/tools/perf/util/parse-sublevel-options.h b/tools/perf/util/parse-sublevel-options.h > > new file mode 100644 > > index 000000000000..9b9efcc2aaad > > --- /dev/null > > +++ b/tools/perf/util/parse-sublevel-options.h > > @@ -0,0 +1,11 @@ > > +#ifndef _PERF_PARSE_SUBLEVEL_OPTIONS_H > > +#define _PERF_PARSE_SUBLEVEL_OPTIONS_H > > + > > +struct sublevel_option { > > + const char *name; > > + int *value_ptr; > > +}; > > + > > +int perf_parse_sublevel_options(const char *str, struct sublevel_option *opts); > > + > > +#endif > > \ No newline at end of file > > Please add a newline at the end. > > Thanks > Namhyung > > > > -- > > 2.25.1 > >
-- Cheers, Changbin Du
| |