[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1278677899.21394.56.camel@mini>
Date: Fri, 09 Jul 2010 08:18:19 -0400
From: Chase Douglas <chase.douglas@...onical.com>
To: Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>
Cc: linux-kernel@...r.kernel.org,
Arnaldo Carvalho de Melo <acme@...hat.com>,
Ingo Molnar <mingo@...e.hu>
Subject: Re: [PATCH 1/3] perf probe: Fix error message if get_real_path()
failed
On Fri, 2010-07-09 at 18:28 +0900, Masami Hiramatsu wrote:
> Perf probe -L shows incorrect error message (Dwarf error)
> if it fails to find source file. This can confuse users.
>
> # ./perf probe -s /nowhere -L vfs_read
> Debuginfo analysis failed. (-2)
> Error: Failed to show lines. (-2)
>
> With this patch, it shows correct message.
>
> # ./perf probe -s /nowhere -L vfs_read
> Failed to find source file. (-2)
> Error: Failed to show lines. (-2)
>
> Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>
> Cc: Chase Douglas <chase.douglas@...onical.com>
> Cc: Arnaldo Carvalho de Melo <acme@...hat.com>
> Cc: Ingo Molnar <mingo@...e.hu>
> ---
>
> tools/perf/util/probe-event.c | 59 +++++++++++++++++++++++++++++++++++++++
> tools/perf/util/probe-finder.c | 61 +++-------------------------------------
> 2 files changed, 64 insertions(+), 56 deletions(-)
>
> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> index 09cf546..8d08e75 100644
> --- a/tools/perf/util/probe-event.c
> +++ b/tools/perf/util/probe-event.c
> @@ -195,6 +195,55 @@ static int try_to_find_kprobe_trace_events(struct perf_probe_event *pev,
> return ntevs;
> }
>
> +/*
> + * Find a src file from a DWARF tag path. Prepend optional source path prefix
> + * and chop off leading directories that do not exist. Result is passed back as
> + * a newly allocated path on success.
> + * Return 0 if file was found and readable, -errno otherwise.
> + */
> +static int get_real_path(const char *raw_path, char **new_path)
> +{
> + if (!symbol_conf.source_prefix) {
> + if (access(raw_path, R_OK) == 0) {
> + *new_path = strdup(raw_path);
> + return 0;
> + } else
> + return -errno;
> + }
> +
> + *new_path = malloc((strlen(symbol_conf.source_prefix) +
> + strlen(raw_path) + 2));
> + if (!*new_path)
> + return -ENOMEM;
> +
> + for (;;) {
> + sprintf(*new_path, "%s/%s", symbol_conf.source_prefix,
> + raw_path);
> +
> + if (access(*new_path, R_OK) == 0)
> + return 0;
> +
> + switch (errno) {
> + case ENAMETOOLONG:
> + case ENOENT:
> + case EROFS:
> + case EFAULT:
> + raw_path = strchr(++raw_path, '/');
> + if (!raw_path) {
> + free(*new_path);
> + *new_path = NULL;
> + return -ENOENT;
> + }
> + continue;
> +
> + default:
> + free(*new_path);
> + *new_path = NULL;
> + return -errno;
> + }
> + }
> +}
> +
> #define LINEBUF_SIZE 256
> #define NR_ADDITIONAL_LINES 2
>
> @@ -244,6 +293,7 @@ int show_line_range(struct line_range *lr)
> struct line_node *ln;
> FILE *fp;
> int fd, ret;
> + char *tmp;
>
> /* Search a line range */
> ret = init_vmlinux();
> @@ -266,6 +316,15 @@ int show_line_range(struct line_range *lr)
> return ret;
> }
>
> + /* Convert source file path */
> + tmp = lr->path;
> + ret = get_real_path(tmp, &lr->path);
> + free(tmp); /* Free old path */
> + if (ret < 0) {
> + pr_warning("Failed to find source file. (%d)\n", ret);
> + return ret;
> + }
> +
> setup_pager();
>
> if (lr->function)
> diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
> index 3e64e1f..a934a36 100644
> --- a/tools/perf/util/probe-finder.c
> +++ b/tools/perf/util/probe-finder.c
> @@ -58,55 +58,6 @@ static int strtailcmp(const char *s1, const char *s2)
> return 0;
> }
>
> -/*
> - * Find a src file from a DWARF tag path. Prepend optional source path prefix
> - * and chop off leading directories that do not exist. Result is passed back as
> - * a newly allocated path on success.
> - * Return 0 if file was found and readable, -errno otherwise.
> - */
> -static int get_real_path(const char *raw_path, char **new_path)
> -{
> - if (!symbol_conf.source_prefix) {
> - if (access(raw_path, R_OK) == 0) {
> - *new_path = strdup(raw_path);
> - return 0;
> - } else
> - return -errno;
> - }
> -
> - *new_path = malloc((strlen(symbol_conf.source_prefix) +
> - strlen(raw_path) + 2));
> - if (!*new_path)
> - return -ENOMEM;
> -
> - for (;;) {
> - sprintf(*new_path, "%s/%s", symbol_conf.source_prefix,
> - raw_path);
> -
> - if (access(*new_path, R_OK) == 0)
> - return 0;
> -
> - switch (errno) {
> - case ENAMETOOLONG:
> - case ENOENT:
> - case EROFS:
> - case EFAULT:
> - raw_path = strchr(++raw_path, '/');
> - if (!raw_path) {
> - free(*new_path);
> - *new_path = NULL;
> - return -ENOENT;
> - }
> - continue;
> -
> - default:
> - free(*new_path);
> - *new_path = NULL;
> - return -errno;
> - }
> - }
> -}
> -
> /* Line number list operations */
>
> /* Add a line to line number list */
> @@ -1256,13 +1207,11 @@ end:
> static int line_range_add_line(const char *src, unsigned int lineno,
> struct line_range *lr)
> {
> - int ret;
> -
> - /* Copy real path */
> + /* Copy source path */
> if (!lr->path) {
> - ret = get_real_path(src, &lr->path);
> - if (ret != 0)
> - return ret;
> + lr->path = strdup(src);
> + if (lr->path == NULL)
> + return -ENOMEM;
> }
> return line_list__add_line(&lr->line_list, lineno);
> }
> @@ -1460,7 +1409,7 @@ int find_line_range(int fd, struct line_range *lr)
> }
> off = noff;
> }
> - pr_debug("path: %lx\n", (unsigned long)lr->path);
> + pr_debug("path: %s\n", lr->path);
> dwarf_end(dbg);
>
> return (ret < 0) ? ret : lf.found;
This seems fine to me. I don't think any functionality has really
changed, just moving the function into probe-event.c and printing out an
accurate error message.
Acked-by: Chase Douglas <chase.douglas@...onical.com>
Thanks!
--
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