All of lore.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.