| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Wed, 06 May 2015 00:06:16 +0900
From: Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>
To: Arnaldo Carvalho de Melo <acme@...nel.org>
CC: Peter Zijlstra <peterz@...radead.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
David Ahern <dsahern@...il.com>, namhyung@...nel.org,
Jiri Olsa <jolsa@...hat.com>, Ingo Molnar <mingo@...nel.org>
Subject: Re: [PATCH perf/core v4 4/4] perf probe: Cleanup and consolidate
command parsers
On 2015/05/05 11:29, Masami Hiramatsu wrote:
> To simplify the perf-probe command code, consolidate some
> similar functions and use command short-name for command
> classification, instead of separated booleans.
Oops, I've found my mistake on this patch. perf-probe without
any command but probe defs should imply "--add", but this patch
drops that.
I'll update this for v5 patch.
Thank you,
>
> Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>
> ---
> tools/perf/builtin-probe.c | 105 ++++++++++++++++----------------------------
> 1 file changed, 37 insertions(+), 68 deletions(-)
>
> diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
> index 08c9481..17870b8 100644
> --- a/tools/perf/builtin-probe.c
> +++ b/tools/perf/builtin-probe.c
> @@ -48,14 +48,10 @@
>
> /* Session management structure */
> static struct {
> + int command; /* Command short_name */
> bool list_events;
> bool force_add;
> - bool show_lines;
> - bool show_vars;
> bool show_ext_vars;
> - bool show_funcs;
> - bool mod_events;
> - bool del_events;
> bool uprobes;
> bool quiet;
> bool target_used;
> @@ -175,44 +171,11 @@ static int parse_probe_event_argv(int argc, const char **argv)
>
> len += sprintf(&buf[len], "%s ", argv[i]);
> }
> - params.mod_events = true;
> ret = parse_probe_event(buf);
> free(buf);
> return ret;
> }
>
> -static int opt_add_probe_event(const struct option *opt __maybe_unused,
> - const char *str, int unset __maybe_unused)
> -{
> - if (str) {
> - params.mod_events = true;
> - return parse_probe_event(str);
> - } else
> - return 0;
> -}
> -
> -static int opt_del_probe_event(const struct option *opt __maybe_unused,
> - const char *str, int unset __maybe_unused)
> -{
> - if (str) {
> - params.del_events = true;
> - return params_add_filter(str);
> - }
> - return 0;
> -}
> -
> -static int opt_list_probe_event(const struct option *opt __maybe_unused,
> - const char *str, int unset)
> -{
> - if (!unset)
> - params.list_events = true;
> -
> - if (str)
> - return params_add_filter(str);
> -
> - return 0;
> -}
> -
> static int opt_set_target(const struct option *opt, const char *str,
> int unset __maybe_unused)
> {
> @@ -250,8 +213,10 @@ static int opt_set_target(const struct option *opt, const char *str,
> return ret;
> }
>
> +/* Command option callbacks */
> +
> #ifdef HAVE_DWARF_SUPPORT
> -static int opt_show_lines(const struct option *opt __maybe_unused,
> +static int opt_show_lines(const struct option *opt,
> const char *str, int unset __maybe_unused)
> {
> int ret = 0;
> @@ -259,19 +224,19 @@ static int opt_show_lines(const struct option *opt __maybe_unused,
> if (!str)
> return 0;
>
> - if (params.show_lines) {
> + if (params.command == 'L') {
> pr_warning("Warning: more than one --line options are"
> " detected. Only the first one is valid.\n");
> return 0;
> }
>
> - params.show_lines = true;
> + params.command = opt->short_name;
> ret = parse_line_range_desc(str, ¶ms.line_range);
>
> return ret;
> }
>
> -static int opt_show_vars(const struct option *opt __maybe_unused,
> +static int opt_show_vars(const struct option *opt,
> const char *str, int unset __maybe_unused)
> {
> struct perf_probe_event *pev = ¶ms.events[params.nevents];
> @@ -285,16 +250,27 @@ static int opt_show_vars(const struct option *opt __maybe_unused,
> pr_err(" Error: '--vars' doesn't accept arguments.\n");
> return -EINVAL;
> }
> - params.show_vars = true;
> + params.command = opt->short_name;
>
> return ret;
> }
> #endif
> -static int opt_show_funcs(const struct option *opt __maybe_unused,
> - const char *str, int unset)
> +static int opt_add_probe_event(const struct option *opt,
> + const char *str, int unset __maybe_unused)
> +{
> + if (str) {
> + params.command = opt->short_name;
> + return parse_probe_event(str);
> + }
> +
> + return 0;
> +}
> +
> +static int opt_set_filter_with_command(const struct option *opt,
> + const char *str, int unset)
> {
> if (!unset)
> - params.show_funcs = true;
> + params.command = opt->short_name;
>
> if (str)
> return params_add_filter(str);
> @@ -360,10 +336,10 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
> OPT_BOOLEAN('q', "quiet", ¶ms.quiet,
> "be quiet (do not show any mesages)"),
> OPT_CALLBACK_DEFAULT('l', "list", NULL, "[GROUP:]EVENT",
> - "list up probe events", opt_list_probe_event,
> - DEFAULT_LIST_FILTER),
> + "list up probe events",
> + opt_set_filter_with_command, DEFAULT_LIST_FILTER),
> OPT_CALLBACK('d', "del", NULL, "[GROUP:]EVENT", "delete a probe event.",
> - opt_del_probe_event),
> + opt_set_filter_with_command),
> OPT_CALLBACK('a', "add", NULL,
> #ifdef HAVE_DWARF_SUPPORT
> "[EVENT=]FUNC[@SRC][+OFF|%return|:RL|;PT]|SRC:AL|SRC;PT"
> @@ -412,7 +388,7 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
> "Set how many probe points can be found for a probe."),
> OPT_CALLBACK_DEFAULT('F', "funcs", NULL, "[FILTER]",
> "Show potential probe-able functions.",
> - opt_show_funcs, DEFAULT_FUNC_FILTER),
> + opt_set_filter_with_command, DEFAULT_FUNC_FILTER),
> OPT_CALLBACK('\0', "filter", NULL,
> "[!]FILTER", "Set a filter (with --vars/funcs only)\n"
> "\t\t\t(default: \"" DEFAULT_VAR_FILTER "\" for --vars,\n"
> @@ -462,16 +438,13 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
> if (params.max_probe_points == 0)
> params.max_probe_points = MAX_PROBES;
>
> - if ((!params.nevents && !params.del_events && !params.list_events &&
> - !params.show_lines && !params.show_funcs))
> - usage_with_options(probe_usage, options);
> -
> /*
> * Only consider the user's kernel image path if given.
> */
> symbol_conf.try_vmlinux_path = (symbol_conf.vmlinux_name == NULL);
>
> - if (params.list_events) {
> + switch (params.command) {
> + case 'l':
> if (params.uprobes) {
> pr_warning(" Error: Don't use --list with --exec.\n");
> usage_with_options(probe_usage, options);
> @@ -480,24 +453,20 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
> if (ret < 0)
> pr_err_with_code(" Error: Failed to show event list.", ret);
> return ret;
> - }
> - if (params.show_funcs) {
> + case 'F':
> ret = show_available_funcs(params.target, params.filter,
> params.uprobes);
> if (ret < 0)
> pr_err_with_code(" Error: Failed to show functions.", ret);
> return ret;
> - }
> -
> #ifdef HAVE_DWARF_SUPPORT
> - if (params.show_lines) {
> + case 'L':
> ret = show_line_range(¶ms.line_range, params.target,
> params.uprobes);
> if (ret < 0)
> pr_err_with_code(" Error: Failed to show lines.", ret);
> return ret;
> - }
> - if (params.show_vars) {
> + case 'V':
> if (!params.filter)
> params.filter = strfilter__new(DEFAULT_VAR_FILTER,
> NULL);
> @@ -510,18 +479,15 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
> if (ret < 0)
> pr_err_with_code(" Error: Failed to show vars.", ret);
> return ret;
> - }
> #endif
> -
> - if (params.del_events) {
> + case 'd':
> ret = del_perf_probe_events(params.filter);
> if (ret < 0) {
> pr_err_with_code(" Error: Failed to delete events.", ret);
> return ret;
> }
> - }
> -
> - if (params.nevents) {
> + break;
> + 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");
> @@ -535,6 +501,9 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
> pr_err_with_code(" Error: Failed to add events.", ret);
> return ret;
> }
> + break;
> + default:
> + usage_with_options(probe_usage, options);
> }
> return 0;
> }
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
--
Masami HIRAMATSU
Linux Technology Research Center, System Productivity Research Dept.
Center for Technology Innovation - Systems Engineering
Hitachi, Ltd., Research & Development Group
E-mail: masami.hiramatsu.pt@...achi.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists