public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/3] perf probe: Fix error message if get_real_path() failed
@ 2010-07-09  9:28 Masami Hiramatsu
  2010-07-09 12:18 ` Chase Douglas
  2010-07-17 11:10 ` [tip:perf/core] " tip-bot for Masami Hiramatsu
  0 siblings, 2 replies; 6+ messages in thread
From: Masami Hiramatsu @ 2010-07-09  9:28 UTC (permalink / raw)
  To: linux-kernel
  Cc: Masami Hiramatsu, Chase Douglas, Arnaldo Carvalho de Melo,
	Ingo Molnar

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@hitachi.com>
Cc: Chase Douglas <chase.douglas@canonical.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Ingo Molnar <mingo@elte.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;



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/3] perf probe: Fix error message if get_real_path() failed
  2010-07-09  9:28 [PATCH 1/3] perf probe: Fix error message if get_real_path() failed Masami Hiramatsu
@ 2010-07-09 12:18 ` Chase Douglas
  2010-07-15 16:38   ` Masami Hiramatsu
  2010-07-17 11:10 ` [tip:perf/core] " tip-bot for Masami Hiramatsu
  1 sibling, 1 reply; 6+ messages in thread
From: Chase Douglas @ 2010-07-09 12:18 UTC (permalink / raw)
  To: Masami Hiramatsu; +Cc: linux-kernel, Arnaldo Carvalho de Melo, Ingo Molnar

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@hitachi.com>
> Cc: Chase Douglas <chase.douglas@canonical.com>
> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> Cc: Ingo Molnar <mingo@elte.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@canonical.com>

Thanks!


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/3] perf probe: Fix error message if get_real_path() failed
  2010-07-09 12:18 ` Chase Douglas
@ 2010-07-15 16:38   ` Masami Hiramatsu
  2010-07-15 17:13     ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 6+ messages in thread
From: Masami Hiramatsu @ 2010-07-15 16:38 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo; +Cc: Chase Douglas, linux-kernel, Ingo Molnar

Hi Arnaldo,

Could you please merge this series, because this series
fixes some bugs which people easily hit ?

Thank you,

Chase Douglas wrote:
> 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@hitachi.com>
>> Cc: Chase Douglas <chase.douglas@canonical.com>
>> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
>> Cc: Ingo Molnar <mingo@elte.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@canonical.com>
> 
> Thanks!
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/3] perf probe: Fix error message if get_real_path() failed
  2010-07-15 16:38   ` Masami Hiramatsu
@ 2010-07-15 17:13     ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 6+ messages in thread
From: Arnaldo Carvalho de Melo @ 2010-07-15 17:13 UTC (permalink / raw)
  To: Masami Hiramatsu; +Cc: Chase Douglas, linux-kernel, Ingo Molnar

Em Fri, Jul 16, 2010 at 01:38:59AM +0900, Masami Hiramatsu escreveu:
> Hi Arnaldo,
> 
> Could you please merge this series, because this series
> fixes some bugs which people easily hit ?

I'll take a look at it, thanks for pointing it out.

- Arnaldo
 
> Thank you,
> 
> Chase Douglas wrote:
> > 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@hitachi.com>
> >> Cc: Chase Douglas <chase.douglas@canonical.com>
> >> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> >> Cc: Ingo Molnar <mingo@elte.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@canonical.com>
> > 
> > Thanks!
> > 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/3] perf probe: Fix error message if get_real_path() failed
  2010-07-16 18:09 [GIT PULL 0/3] perf/core fixes and improvements Arnaldo Carvalho de Melo
@ 2010-07-16 18:09 ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 6+ messages in thread
From: Arnaldo Carvalho de Melo @ 2010-07-16 18:09 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Masami Hiramatsu, Chase Douglas,
	Arnaldo Carvalho de Melo, Ingo Molnar

From: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>

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)

LKML-Reference: <4C36EBDB.4020308@hitachi.com>
Cc: Chase Douglas <chase.douglas@canonical.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Ingo Molnar <mingo@elte.hu>
Acked-by: Chase Douglas <chase.douglas@canonical.com>
Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 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;
-- 
1.6.2.5


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [tip:perf/core] perf probe: Fix error message if get_real_path() failed
  2010-07-09  9:28 [PATCH 1/3] perf probe: Fix error message if get_real_path() failed Masami Hiramatsu
  2010-07-09 12:18 ` Chase Douglas
@ 2010-07-17 11:10 ` tip-bot for Masami Hiramatsu
  1 sibling, 0 replies; 6+ messages in thread
From: tip-bot for Masami Hiramatsu @ 2010-07-17 11:10 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: acme, linux-kernel, hpa, mingo, masami.hiramatsu.pt,
	chase.douglas, tglx, mingo

Commit-ID:  7cf0b79e6ffd04bba5d4e625a0fe2e30a5b383e5
Gitweb:     http://git.kernel.org/tip/7cf0b79e6ffd04bba5d4e625a0fe2e30a5b383e5
Author:     Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
AuthorDate: Fri, 9 Jul 2010 18:28:59 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Fri, 16 Jul 2010 11:46:34 -0300

perf probe: Fix error message if get_real_path() failed

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)

LKML-Reference: <4C36EBDB.4020308@hitachi.com>
Cc: Chase Douglas <chase.douglas@canonical.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Ingo Molnar <mingo@elte.hu>
Acked-by: Chase Douglas <chase.douglas@canonical.com>
Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 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;

^ permalink raw reply related	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2010-07-17 11:11 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-07-09  9:28 [PATCH 1/3] perf probe: Fix error message if get_real_path() failed Masami Hiramatsu
2010-07-09 12:18 ` Chase Douglas
2010-07-15 16:38   ` Masami Hiramatsu
2010-07-15 17:13     ` Arnaldo Carvalho de Melo
2010-07-17 11:10 ` [tip:perf/core] " tip-bot for Masami Hiramatsu
  -- strict thread matches above, loose matches on Subject: below --
2010-07-16 18:09 [GIT PULL 0/3] perf/core fixes and improvements Arnaldo Carvalho de Melo
2010-07-16 18:09 ` [PATCH 1/3] perf probe: Fix error message if get_real_path() failed Arnaldo Carvalho de Melo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox