Home » Mailing list archives » linux-kernel » 2010 » December » 25

[tip:perf/core] perf probe: Fix wrong warning in __show_one_line() if read(1) errors happen

Previous thread: Slow disks. by Rogier Wolff on Monday, December 20, 2010 - 7:15 am. (38 messages)

Next thread: [PATCH 1/2] staging: tidspbridge: convert dmm_map_lock to sema by Felipe Contreras on Monday, December 20, 2010 - 7:25 am. (1 message)
[view original message]
From: Franck Bui-Huu
Subject: [PATCH 0/6] Minor cleanups & fixes for perf-probe(1)
Date: Monday, December 20, 2010 - 7:17 am

From: Franck Bui-Huu <fbuihuu@gmail.com>

Hi,

This small patchset is just some random cosmetic cleanups I did when
starting to read perf-probe(1) stuffs.

The 3 last patches of this serie are minor fixes though.

---

Franck Bui-Huu (6):
  perf-tools: make perf-probe -L display the absolute path of the dumped file
  perf-probe: rewrite show_one_line() to make it simpler
  perf-probe: clean up redundant tests in show_line_range()
  perf-probe: Fix line range description since a single file is allowed
  perf-probe: don't always consider EOF as an error when listing source code
  perf-probe: handle gracefully some stupid and buggy line syntaxes

 tools/perf/Documentation/perf-probe.txt |    2 +-
 tools/perf/util/probe-event.c           |  190 +++++++++++++++++++------------
 2 files changed, 117 insertions(+), 75 deletions(-)

-- 
1.7.3.2

--


[ message continues ]
[view original message]
From: Franck Bui-Huu
Subject: [PATCH 1/6] perf-tools: make perf-probe -L display the absolute path of the dumped file
Date: Monday, December 20, 2010 - 7:18 am

From: Franck Bui-Huu <fbuihuu@gmail.com>

The actual file used by 'perf probe -L sched.c' is reported in the
ouput of the command.

But it's simply displayed as it has been given to the command (simply
sched.c) which is too ambiguous to be really usefull since several
sched.c files can be found into the same project and we also don't
know which search path has been used.

Signed-off-by: Franck Bui-Huu <fbuihuu@gmail.com>
---
 tools/perf/util/probe-event.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 558545e..6fa0403 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -381,7 +381,7 @@ int show_line_range(struct line_range *lr, const char *module)
 		fprintf(stdout, "<%s:%d>\n", lr->function,
 			lr->start - lr->offset);
 	else
-		fprintf(stdout, "<%s:%d>\n", lr->file, lr->start);
+		fprintf(stdout, "<%s:%d>\n", lr->path, lr->start);
 
 	fp = fopen(lr->path, "r");
 	if (fp == NULL) {
-- 
1.7.3.2

--


[ message continues ]
[view original message]
From: Masami Hiramatsu
Subject: Re: [PATCH 1/6] perf-tools: make perf-probe -L display the absolute path of the dumped file
Date: Monday, December 20, 2010 - 10:30 pm

It's enough reasonable for me.



-- 
Masami HIRAMATSU
2nd Dept. Linux Technology Center
Hitachi, Ltd., Systems Development Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com
--


[ message continues ]
[view original message]
From: tip-bot for Franck Bui-Huu
Subject: [tip:perf/core] perf probe: Make -L display the absolute path of the dumped file
Date: Wednesday, December 22, 2010 - 4:30 am

Commit-ID:  62c15fc49bd1b35d79b34ea96f132ab435e2215a
Gitweb:     http://git.kernel.org/tip/62c15fc49bd1b35d79b34ea96f132ab435e2215a
Author:     Franck Bui-Huu <fbuihuu@gmail.com>
AuthorDate: Mon, 20 Dec 2010 15:18:00 +0100
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Tue, 21 Dec 2010 16:20:11 -0200

perf probe: Make -L display the absolute path of the dumped file

The actual file used by 'perf probe -L sched.c' is reported in the ouput
of the command.

But it's simply displayed as it has been given to the command (simply
sched.c) which is too ambiguous to be really usefull since several
sched.c files can be found into the same project and we also don't know
which search path has been used.

Acked-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
LKML-Reference: <1292854685-8230-2-git-send-email-fbuihuu@gmail.com>
Signed-off-by: Franck Bui-Huu <fbuihuu@gmail.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/probe-event.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 089d78e..0163fc0 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -371,7 +371,7 @@ int show_line_range(struct line_range *lr, const char *module)
 		fprintf(stdout, "<%s:%d>\n", lr->function,
 			lr->start - lr->offset);
 	else
-		fprintf(stdout, "<%s:%d>\n", lr->file, lr->start);
+		fprintf(stdout, "<%s:%d>\n", lr->path, lr->start);
 
 	fp = fopen(lr->path, "r");
 	if (fp == NULL) {
--


[ message continues ]
[view original message]
From: Franck Bui-Huu
Subject: [PATCH 2/6] perf-probe: rewrite show_one_line() to make it simpler
Date: Monday, December 20, 2010 - 7:18 am

From: Franck Bui-Huu <fbuihuu@gmail.com>

Signed-off-by: Franck Bui-Huu <fbuihuu@gmail.com>
---
 tools/perf/util/probe-event.c |   29 +++++++++++------------------
 1 files changed, 11 insertions(+), 18 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 6fa0403..5cc8f6b 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -300,28 +300,21 @@ static int get_real_path(const char *raw_path, const char *comp_dir,
 static int show_one_line(FILE *fp, int l, bool skip, bool show_num)
 {
 	char buf[LINEBUF_SIZE];
-	const char *color = PERF_COLOR_BLUE;
+	const char *color = show_num ? "" : PERF_COLOR_BLUE;
+	const char *prefix = NULL;
 
-	if (fgets(buf, LINEBUF_SIZE, fp) == NULL)
-		goto error;
-	if (!skip) {
-		if (show_num)
-			fprintf(stdout, "%7d  %s", l, buf);
-		else
-			color_fprintf(stdout, color, "         %s", buf);
-	}
-
-	while (strlen(buf) == LINEBUF_SIZE - 1 &&
-	       buf[LINEBUF_SIZE - 2] != '\n') {
+	do {
 		if (fgets(buf, LINEBUF_SIZE, fp) == NULL)
 			goto error;
-		if (!skip) {
-			if (show_num)
-				fprintf(stdout, "%s", buf);
-			else
-				color_fprintf(stdout, color, "%s", buf);
+		if (skip)
+			continue;
+		if (!prefix) {
+			prefix = show_num ? "%7d  " : "         ";
+			fprintf(stdout, prefix, l);
 		}
-	}
+		color_fprintf(stdout, color, "%s", buf);
+
+	} while (strchr(buf, '\n') == NULL);
 
 	return 0;
 error:
-- 
1.7.3.2

--


[ message continues ]
[view original message]
From: Masami Hiramatsu
Subject: Re: [PATCH 2/6] perf-probe: rewrite show_one_line() to make it simpler
Date: Tuesday, December 21, 2010 - 1:55 am

Could you use color_fprintf() here too?
I know currently it's meaningless, but from the view of maintaining,
it's better to use same function.



-- 
Masami HIRAMATSU
2nd Dept. Linux Technology Center
Hitachi, Ltd., Systems Development Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com
--


[ message continues ]
[view original message]
From: tip-bot for Franck Bui-Huu
Subject: [tip:perf/core] perf probe: Rewrite show_one_line() to make it simpler
Date: Wednesday, December 22, 2010 - 4:30 am

Commit-ID:  befe341468f4e61ecaf337a0237f2aab76817437
Gitweb:     http://git.kernel.org/tip/befe341468f4e61ecaf337a0237f2aab76817437
Author:     Franck Bui-Huu <fbuihuu@gmail.com>
AuthorDate: Mon, 20 Dec 2010 15:18:01 +0100
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Tue, 21 Dec 2010 16:20:11 -0200

perf probe: Rewrite show_one_line() to make it simpler

Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
LKML-Reference: <1292854685-8230-3-git-send-email-fbuihuu@gmail.com>
Signed-off-by: Franck Bui-Huu <fbuihuu@gmail.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/probe-event.c |   29 +++++++++++------------------
 1 files changed, 11 insertions(+), 18 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 0163fc0..327604c 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -290,28 +290,21 @@ static int get_real_path(const char *raw_path, const char *comp_dir,
 static int show_one_line(FILE *fp, int l, bool skip, bool show_num)
 {
 	char buf[LINEBUF_SIZE];
-	const char *color = PERF_COLOR_BLUE;
+	const char *color = show_num ? "" : PERF_COLOR_BLUE;
+	const char *prefix = NULL;
 
-	if (fgets(buf, LINEBUF_SIZE, fp) == NULL)
-		goto error;
-	if (!skip) {
-		if (show_num)
-			fprintf(stdout, "%7d  %s", l, buf);
-		else
-			color_fprintf(stdout, color, "         %s", buf);
-	}
-
-	while (strlen(buf) == LINEBUF_SIZE - 1 &&
-	       buf[LINEBUF_SIZE - 2] != '\n') {
+	do {
 		if (fgets(buf, LINEBUF_SIZE, fp) == NULL)
 			goto error;
-		if (!skip) {
-			if (show_num)
-				fprintf(stdout, "%s", buf);
-			else
-				color_fprintf(stdout, color, "%s", buf);
+		if (skip)
+			continue;
+		if (!prefix) {
+			prefix = show_num ? "%7d  " : "         ";
+			color_fprintf(stdout, color, prefix, l);
 		}
-	}
+		color_fprintf(stdout, color, "%s", buf);
+
+	} while (strchr(buf, '\n') == NULL);
 
 	return 0;
 error:
--


[ message continues ]
[view original message]
From: Franck Bui-Huu
Subject: [PATCH 3/6] perf-probe: clean up redundant tests in show_line_range()
Date: Monday, December 20, 2010 - 7:18 am

From: Franck Bui-Huu <fbuihuu@gmail.com>

It also removes some superflous parentheses.

Signed-off-by: Franck Bui-Huu <fbuihuu@gmail.com>
---
 tools/perf/util/probe-event.c |   26 +++++++++++++++-----------
 1 files changed, 15 insertions(+), 11 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 5cc8f6b..3c92b92 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -383,26 +383,30 @@ int show_line_range(struct line_range *lr, const char *module)
 		return -errno;
 	}
 	/* Skip to starting line number */
-	while (l < lr->start && ret >= 0)
+	while (l < lr->start) {
 		ret = show_one_line(fp, l++, true, false);
-	if (ret < 0)
-		goto end;
+		if (ret < 0)
+			goto end;
+	}
 
 	list_for_each_entry(ln, &lr->line_list, list) {
-		while (ln->line > l && ret >= 0)
-			ret = show_one_line(fp, (l++) - lr->offset,
-					    false, false);
-		if (ret >= 0)
-			ret = show_one_line(fp, (l++) - lr->offset,
-					    false, true);
+		for (; ln->line > l; l++) {
+			ret = show_one_line(fp, l - lr->offset, false, false);
+			if (ret < 0)
+				goto end;
+		}
+		ret = show_one_line(fp, l++ - lr->offset, false, true);
 		if (ret < 0)
 			goto end;
 	}
 
 	if (lr->end == INT_MAX)
 		lr->end = l + NR_ADDITIONAL_LINES;
-	while (l <= lr->end && !feof(fp) && ret >= 0)
-		ret = show_one_line(fp, (l++) - lr->offset, false, false);
+	while (l <= lr->end && !feof(fp)) {
+		ret = show_one_line(fp, l++ - lr->offset, false, false);
+		if (ret < 0)
+			break;
+	}
 end:
 	fclose(fp);
 	return ret;
-- 
1.7.3.2

--


[ message continues ]
[view original message]
From: Masami Hiramatsu
Subject: Re: [PATCH 3/6] perf-probe: clean up redundant tests in show_line_range()
Date: Tuesday, December 21, 2010 - 2:47 am

-- 
Masami HIRAMATSU
2nd Dept. Linux Technology Center
Hitachi, Ltd., Systems Development Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com
--


[ message continues ]
[view original message]
From: tip-bot for Franck Bui-Huu
Subject: [tip:perf/core] perf probe: Clean up redundant tests in show_line_range()
Date: Wednesday, December 22, 2010 - 4:31 am

Commit-ID:  44b81e929b0c00e703a31a3d634b668bb27eb1c8
Gitweb:     http://git.kernel.org/tip/44b81e929b0c00e703a31a3d634b668bb27eb1c8
Author:     Franck Bui-Huu <fbuihuu@gmail.com>
AuthorDate: Mon, 20 Dec 2010 15:18:02 +0100
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Tue, 21 Dec 2010 16:20:12 -0200

perf probe: Clean up redundant tests in show_line_range()

It also removes some superflous parentheses.

Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
LKML-Reference: <1292854685-8230-4-git-send-email-fbuihuu@gmail.com>
Signed-off-by: Franck Bui-Huu <fbuihuu@gmail.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/probe-event.c |   26 +++++++++++++++-----------
 1 files changed, 15 insertions(+), 11 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 327604c..b812f14 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -373,26 +373,30 @@ int show_line_range(struct line_range *lr, const char *module)
 		return -errno;
 	}
 	/* Skip to starting line number */
-	while (l < lr->start && ret >= 0)
+	while (l < lr->start) {
 		ret = show_one_line(fp, l++, true, false);
-	if (ret < 0)
-		goto end;
+		if (ret < 0)
+			goto end;
+	}
 
 	list_for_each_entry(ln, &lr->line_list, list) {
-		while (ln->line > l && ret >= 0)
-			ret = show_one_line(fp, (l++) - lr->offset,
-					    false, false);
-		if (ret >= 0)
-			ret = show_one_line(fp, (l++) - lr->offset,
-					    false, true);
+		for (; ln->line > l; l++) {
+			ret = show_one_line(fp, l - lr->offset, false, false);
+			if (ret < 0)
+				goto end;
+		}
+		ret = show_one_line(fp, l++ - lr->offset, false, true);
 		if (ret < 0)
 			goto end;
 	}
 
 	if (lr->end == INT_MAX)
 		lr->end = l + NR_ADDITIONAL_LINES;
-	while (l <= lr->end && !feof(fp) && ret >= 0)
-		ret = show_one_line(fp, (l++) - lr->offset, false, false);
+	while (l <= lr->end && !feof(fp)) {
+		ret = show_one_line(fp, ...
[ message continues ]
[view original message]
From: Franck Bui-Huu
Subject: [PATCH 4/6] perf-probe: Fix line range description since a single file is allowed
Date: Monday, December 20, 2010 - 7:18 am

From: Franck Bui-Huu <fbuihuu@gmail.com>

	$ perf-probe -L sched.c

is currently allowed but not documented.

Signed-off-by: Franck Bui-Huu <fbuihuu@gmail.com>
---
 tools/perf/Documentation/perf-probe.txt |    2 +-
 tools/perf/util/probe-event.c           |   13 ++++++++-----
 2 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/tools/perf/Documentation/perf-probe.txt b/tools/perf/Documentation/perf-probe.txt
index 62de1b7..562501f 100644
--- a/tools/perf/Documentation/perf-probe.txt
+++ b/tools/perf/Documentation/perf-probe.txt
@@ -117,7 +117,7 @@ LINE SYNTAX
 -----------
 Line range is descripted by following syntax.
 
- "FUNC[:RLN[+NUM|-RLN2]]|SRC:ALN[+NUM|-ALN2]"
+ "FUNC[:RLN[+NUM|-RLN2]]|SRC[:ALN[+NUM|-ALN2]]"
 
 FUNC specifies the function name of showing lines. 'RLN' is the start line
 number from function entry line, and 'RLN2' is the end line number. As same as
diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 3c92b92..8e5f5ff 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -525,15 +525,18 @@ int show_available_vars(struct perf_probe_event *pevs __unused,
 }
 #endif
 
+/*
+ * Stuff 'lr' according to the line range described by 'arg'.
+ * The line range syntax is described by:
+ *
+ *         SRC[:SLN[+NUM|-ELN]]
+ *         FNC[:SLN[+NUM|-ELN]]
+ */
 int parse_line_range_desc(const char *arg, struct line_range *lr)
 {
 	const char *ptr;
 	char *tmp;
-	/*
-	 * <Syntax>
-	 * SRC:SLN[+NUM|-ELN]
-	 * FUNC[:SLN[+NUM|-ELN]]
-	 */
+
 	ptr = strchr(arg, ':');
 	if (ptr) {
 		lr->start = (int)strtoul(ptr + 1, &tmp, 0);
-- 
1.7.3.2

--


[ message continues ]
[view original message]
From: tip-bot for Franck Bui-Huu
Subject: [tip:perf/core] perf probe: Fix line range description since a single file is allowed
Date: Wednesday, December 22, 2010 - 4:31 am

Commit-ID:  9d95b580a8d64ef4d1660a21a9de0658fe29f041
Gitweb:     http://git.kernel.org/tip/9d95b580a8d64ef4d1660a21a9de0658fe29f041
Author:     Franck Bui-Huu <fbuihuu@gmail.com>
AuthorDate: Mon, 20 Dec 2010 15:18:03 +0100
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Tue, 21 Dec 2010 16:20:12 -0200

perf probe: Fix line range description since a single file is allowed

	$ perf-probe -L sched.c

is currently allowed but not documented.

Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
LKML-Reference: <1292854685-8230-5-git-send-email-fbuihuu@gmail.com>
Signed-off-by: Franck Bui-Huu <fbuihuu@gmail.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/Documentation/perf-probe.txt |    2 +-
 tools/perf/util/probe-event.c           |   13 ++++++++-----
 2 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/tools/perf/Documentation/perf-probe.txt b/tools/perf/Documentation/perf-probe.txt
index 4e23232..86b797a 100644
--- a/tools/perf/Documentation/perf-probe.txt
+++ b/tools/perf/Documentation/perf-probe.txt
@@ -117,7 +117,7 @@ LINE SYNTAX
 -----------
 Line range is described by following syntax.
 
- "FUNC[:RLN[+NUM|-RLN2]]|SRC:ALN[+NUM|-ALN2]"
+ "FUNC[:RLN[+NUM|-RLN2]]|SRC[:ALN[+NUM|-ALN2]]"
 
 FUNC specifies the function name of showing lines. 'RLN' is the start line
 number from function entry line, and 'RLN2' is the end line number. As same as
diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index b812f14..3ba9c53 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -515,15 +515,18 @@ int show_available_vars(struct perf_probe_event *pevs __unused,
 }
 #endif
 
+/*
+ * Stuff 'lr' according to the line range described by 'arg'.
+ * The line range syntax is described by:
+ *
+ *         SRC[:SLN[+NUM|-ELN]]
+ *         FNC[:SLN[+NUM|-ELN]]
+ */
 int parse_line_range_desc(const char *arg, struct line_range *lr)
 {
 	const char *ptr;
 	char ...
[ message continues ]
[view original message]
From: Franck Bui-Huu
Subject: [PATCH 5/6] perf-probe: don't always consider EOF as an error when listing source code
Date: Monday, December 20, 2010 - 7:18 am

From: Franck Bui-Huu <fbuihuu@gmail.com>

When listing a whole file or a function which is located at the end,
perf-probe -L output wrongly: "Source file is shorter than expected.".

This is because show_one_line() always consider EOF as an error.

This patch fixes this by not considering EOF as an error when dumping
the trailing lines. Otherwise it's still an error and perf-probe still
outputs its warning.

Signed-off-by: Franck Bui-Huu <fbuihuu@gmail.com>
---
 tools/perf/util/probe-event.c |   38 ++++++++++++++++++++++++++------------
 1 files changed, 26 insertions(+), 12 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 8e5f5ff..1e81936 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -297,7 +297,7 @@ static int get_real_path(const char *raw_path, const char *comp_dir,
 #define LINEBUF_SIZE 256
 #define NR_ADDITIONAL_LINES 2
 
-static int show_one_line(FILE *fp, int l, bool skip, bool show_num)
+static int __show_one_line(FILE *fp, int l, bool skip, bool show_num)
 {
 	char buf[LINEBUF_SIZE];
 	const char *color = show_num ? "" : PERF_COLOR_BLUE;
@@ -316,16 +316,30 @@ static int show_one_line(FILE *fp, int l, bool skip, bool show_num)
 
 	} while (strchr(buf, '\n') == NULL);
 
-	return 0;
+	return 1;
 error:
-	if (feof(fp))
+	if (ferror(fp)) {
 		pr_warning("Source file is shorter than expected.\n");
-	else
-		pr_warning("File read error: %s\n", strerror(errno));
+		return -1;
+	}
+	return 0;
+}
 
-	return -1;
+static int _show_one_line(FILE *fp, int l, bool skip, bool show_num)
+{
+	int rv = __show_one_line(fp, l, skip, show_num);
+	if (rv == 0) {
+		pr_warning("Source file is shorter than expected.\n");
+		rv = -1;
+	}
+	return rv;
 }
 
+#define show_one_line_with_num(f,l)	_show_one_line(f,l,false,true)
+#define show_one_line(f,l)		_show_one_line(f,l,false,false)
+#define skip_one_line(f,l)		_show_one_line(f,l,true,false)
+#define ...
[ message continues ]
[view original message]
From: Masami Hiramatsu
Subject: Re: [PATCH 5/6] perf-probe: don't always consider EOF as an error when listing source code
Date: Tuesday, December 21, 2010 - 4:54 am

Thank you for fixing the bug! :)



-- 
Masami HIRAMATSU
2nd Dept. Linux Technology Center
Hitachi, Ltd., Systems Development Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com
--


[ message continues ]
[view original message]
From: tip-bot for Franck Bui-Huu
Subject: [tip:perf/core] perf probe: Don't always consider EOF as an error when listing source code
Date: Wednesday, December 22, 2010 - 4:31 am

Commit-ID:  fde52dbd7f71934aba4e150f3d1d51e826a08850
Gitweb:     http://git.kernel.org/tip/fde52dbd7f71934aba4e150f3d1d51e826a08850
Author:     Franck Bui-Huu <fbuihuu@gmail.com>
AuthorDate: Mon, 20 Dec 2010 15:18:04 +0100
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Tue, 21 Dec 2010 16:20:12 -0200

perf probe: Don't always consider EOF as an error when listing source code

When listing a whole file or a function which is located at the end,
perf-probe -L output wrongly: "Source file is shorter than expected.".

This is because show_one_line() always consider EOF as an error.

This patch fixes this by not considering EOF as an error when dumping
the trailing lines. Otherwise it's still an error and perf-probe still
outputs its warning.

Acked-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
LKML-Reference: <1292854685-8230-6-git-send-email-fbuihuu@gmail.com>
Signed-off-by: Franck Bui-Huu <fbuihuu@gmail.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/probe-event.c |   38 ++++++++++++++++++++++++++------------
 1 files changed, 26 insertions(+), 12 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 3ba9c53..80cc0bc 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -287,7 +287,7 @@ static int get_real_path(const char *raw_path, const char *comp_dir,
 #define LINEBUF_SIZE 256
 #define NR_ADDITIONAL_LINES 2
 
-static int show_one_line(FILE *fp, int l, bool skip, bool show_num)
+static int __show_one_line(FILE *fp, int l, bool skip, bool show_num)
 {
 	char buf[LINEBUF_SIZE];
 	const char *color = show_num ? "" : PERF_COLOR_BLUE;
@@ -306,16 +306,30 @@ static int show_one_line(FILE *fp, int l, bool skip, bool show_num)
 
 	} while (strchr(buf, '\n') == NULL);
 
-	return 0;
+	return 1;
 error:
-	if (feof(fp))
+	if (ferror(fp)) {
 		pr_warning("Source file is shorter than ...
[ message continues ]
[view original message]
From: Franck Bui-Huu
Subject: Re: [tip:perf/core] perf probe: Don't always consider EOF as an error when listing source code
Date: Wednesday, December 22, 2010 - 6:14 am

tip-bot for Franck Bui-Huu <fbuihuu@gmail.com> writes:



Argh, something wrong here.

The warning left here, is the wrong one, I should have left the other
one.

Sorry for the mistake.

Should I send a patch to fix that ?
-- 
		Franck
--


[ message continues ]
[view original message]
From: Arnaldo Carvalho de Melo
Subject: Re: [tip:perf/core] perf probe: Don't always consider EOF as an error when listing source code
Date: Wednesday, December 22, 2010 - 7:43 am

Please.

- Arnaldo
--


[ message continues ]
[view original message]
From: Franck Bui-Huu
Subject: Re: [tip:perf/core] perf probe: Don't always consider EOF as an error when listing source code
Date: Wednesday, December 22, 2010 - 9:37 am

[PATCH] perf-probe: Fix wrong warning in __show_one_line() if read(1) errors happen

From: Franck Bui-Huu <fbuihuu@gmail.com>

This was introduced by commit fde52dbd7f71934aba4e150f3d1d51e826a08850.

Signed-off-by: Franck Bui-Huu <fbuihuu@gmail.com>
---
 tools/perf/util/probe-event.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 469ad35..10ad1ad 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -319,7 +319,7 @@ static int __show_one_line(FILE *fp, int l, bool skip, bool show_num)
 	return 1;
 error:
 	if (ferror(fp)) {
-		pr_warning("Source file is shorter than expected.\n");
+		pr_warning("File read error: %s\n", strerror(errno));
 		return -1;
 	}
 	return 0;
-- 
1.7.3.2



-- 
		Franck
--


[ message continues ]
[view original message]
From: tip-bot for Franck Bui-Huu
Subject: [tip:perf/core] perf probe: Fix wrong warning in __show_one_line() if read(1) errors happen
Date: Saturday, December 25, 2010 - 1:58 am

Commit-ID:  32b2b6ec57a3adb3ab7215fbf36ec61c15de06ee
Gitweb:     http://git.kernel.org/tip/32b2b6ec57a3adb3ab7215fbf36ec61c15de06ee
Author:     Franck Bui-Huu <vagabon.xyz@gmail.com>
AuthorDate: Wed, 22 Dec 2010 17:37:13 +0100
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 22 Dec 2010 20:32:08 -0200

perf probe: Fix wrong warning in __show_one_line() if read(1) errors happen

This was introduced by commit fde52dbd7f71934aba4e150f3d1d51e826a08850.

Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Thomas Gleixner <tglx@linutronix.de>
LKML-Reference: <m3y67hsr0m.fsf@gmail.com>
Signed-off-by: Franck Bui-Huu <fbuihuu@gmail.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/probe-event.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 099336e..4bde988 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -309,7 +309,7 @@ static int __show_one_line(FILE *fp, int l, bool skip, bool show_num)
 	return 1;
 error:
 	if (ferror(fp)) {
-		pr_warning("Source file is shorter than expected.\n");
+		pr_warning("File read error: %s\n", strerror(errno));
 		return -1;
 	}
 	return 0;
--


[ message continues ]
[view original message]
From: Masami Hiramatsu
Subject: Re: [tip:perf/core] perf probe: Fix wrong warning in __show_one_line() if read(1) errors happen
Date: Sunday, December 26, 2010 - 7:11 am

It's a good fix for me.



-- 
Masami HIRAMATSU
2nd Dept. Linux Technology Center
Hitachi, Ltd., Systems Development Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com
--


[ message continues ]
[view original message]
From: Franck Bui-Huu
Subject: [PATCH 6/6] perf-probe: handle gracefully some stupid and buggy line syntaxes
Date: Monday, December 20, 2010 - 7:18 am

From: Franck Bui-Huu <fbuihuu@gmail.com>

Currently perf-probe doesn't handle those incorrect syntaxes

   $ perf probe -L sched.c:++13
   $ perf probe -L sched.c:-+13
   $ perf probe -L sched.c:10000000000000000000000000000+13

This patches rewrites parse_line_range_desc() to handle them.

As a bonus side, it reports more usefull error messages instead of:
"Tailing with invalid character...".

Signed-off-by: Franck Bui-Huu <fbuihuu@gmail.com>
---
 tools/perf/util/probe-event.c |   92 ++++++++++++++++++++++++++--------------
 1 files changed, 60 insertions(+), 32 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 1e81936..469ad35 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -539,6 +539,19 @@ int show_available_vars(struct perf_probe_event *pevs __unused,
 }
 #endif
 
+static int parse_line_num(char **ptr, int *val, const char *what)
+{
+	const char *start = *ptr;
+
+	errno = 0;
+	*val = strtol(*ptr, ptr, 0);
+	if (errno || *ptr == start) {
+		semantic_error("'%s' is not a valid number.\n", what);
+		return -EINVAL;
+	}
+	return 0;
+}
+
 /*
  * Stuff 'lr' according to the line range described by 'arg'.
  * The line range syntax is described by:
@@ -548,50 +561,65 @@ int show_available_vars(struct perf_probe_event *pevs __unused,
  */
 int parse_line_range_desc(const char *arg, struct line_range *lr)
 {
-	const char *ptr;
-	char *tmp;
+	char *range, *name = strdup(arg);
+	int err;
+
+	if (!name)
+		return -ENOMEM;
+
+	lr->start = 0;
+	lr->end = INT_MAX;
+
+	range = strchr(name, ':');
+	if (range) {
+		*range++ = '\0';
+
+		err = parse_line_num(&range, &lr->start, "start line");
+		if (err)
+			goto err;
+
+		if (*range == '+' || *range == '-') {
+			const char c = *range++;
+
+			err = parse_line_num(&range, &lr->end, "end line");
+			if (err)
+				goto err;
+
+			if (c == '+') {
+				lr->end += lr->start;
+				/*
+				 * Adjust the number of lines ...
[ message continues ]
[view original message]
From: Masami Hiramatsu
Subject: Re: [PATCH 6/6] perf-probe: handle gracefully some stupid and buggy line syntaxes
Date: Tuesday, December 21, 2010 - 6:10 am

OK, without this patch, perf probe accepted that as "0+13".
With this, perf probe rejects it as an invalid input.




-- 
Masami HIRAMATSU
2nd Dept. Linux Technology Center
Hitachi, Ltd., Systems Development Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com
--


[ message continues ]
[view original message]
From: tip-bot for Franck Bui-Huu
Subject: [tip:perf/core] perf probe: Handle gracefully some stupid and buggy line syntaxes
Date: Wednesday, December 22, 2010 - 4:32 am

Commit-ID:  21dd9ae5a4e9f717f3957ec934dd3158129436b8
Gitweb:     http://git.kernel.org/tip/21dd9ae5a4e9f717f3957ec934dd3158129436b8
Author:     Franck Bui-Huu <fbuihuu@gmail.com>
AuthorDate: Mon, 20 Dec 2010 15:18:05 +0100
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Tue, 21 Dec 2010 17:20:13 -0200

perf probe: Handle gracefully some stupid and buggy line syntaxes

Currently perf probe doesn't handle those incorrect syntaxes:

   $ perf probe -L sched.c:++13
   $ perf probe -L sched.c:-+13
   $ perf probe -L sched.c:10000000000000000000000000000+13

This patches rewrites parse_line_range_desc() to handle them.

As a bonus, it reports more useful error messages instead of: "Tailing
with invalid character...".

Acked-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
LKML-Reference: <1292854685-8230-7-git-send-email-fbuihuu@gmail.com>
Signed-off-by: Franck Bui-Huu <fbuihuu@gmail.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/probe-event.c |   92 ++++++++++++++++++++++++++--------------
 1 files changed, 60 insertions(+), 32 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 80cc0bc..099336e 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -529,6 +529,19 @@ int show_available_vars(struct perf_probe_event *pevs __unused,
 }
 #endif
 
+static int parse_line_num(char **ptr, int *val, const char *what)
+{
+	const char *start = *ptr;
+
+	errno = 0;
+	*val = strtol(*ptr, ptr, 0);
+	if (errno || *ptr == start) {
+		semantic_error("'%s' is not a valid number.\n", what);
+		return -EINVAL;
+	}
+	return 0;
+}
+
 /*
  * Stuff 'lr' according to the line range described by 'arg'.
  * The line range syntax is described by:
@@ -538,50 +551,65 @@ int show_available_vars(struct perf_probe_event *pevs __unused,
  */
 int parse_line_range_desc(const char *arg, struct line_range *lr)
 ...
[ message continues ]
[view original message]
From: Masami Hiramatsu
Subject: Re: [PATCH 0/6] Minor cleanups & fixes for perf-probe(1)
Date: Tuesday, December 21, 2010 - 6:11 am

Hi Franck,

Basically the contains of this series are good to me.
(I've sent my comments)
however, it's less comments on each patch. Please explain
what you did in each patch! (not only on the subject line:-))



-- 
Masami HIRAMATSU
2nd Dept. Linux Technology Center
Hitachi, Ltd., Systems Development Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com
--


[ message continues ]
[view original message]
From: Franck Bui-Huu
Subject: Re: [PATCH 0/6] Minor cleanups & fixes for perf-probe(1)
Date: Tuesday, December 21, 2010 - 11:01 am

You're welcome.

-- 
		Franck
--


[ message continues ]
[view original message]
From: Arnaldo Carvalho de Melo
Subject: Re: [PATCH 0/6] Minor cleanups & fixes for perf-probe(1)
Date: Tuesday, December 21, 2010 - 2:05 pm

I did just that s/fprintf/color_fprintf/g change Masami asked and
stashed everything in my perf/core branch.

Thanks,

- Arnaldo
--


[ message continues ]
[view original message]
From: Franck Bui-Huu
Subject: Re: [PATCH 0/6] Minor cleanups & fixes for perf-probe(1)
Date: Wednesday, December 22, 2010 - 6:11 am

Arnaldo Carvalho de Melo <acme@ghostprotocols.net> writes:


Thanks.

Should I still send some details about what patches do ?

-- 
		Franck
--


[ message continues ]
[view original message]
From: Masami Hiramatsu
Subject: Re: [PATCH 0/6] Minor cleanups & fixes for perf-probe(1)
Date: Thursday, December 23, 2010 - 5:36 pm

No, it seems that Arnaldo has already fixed it in his tree.

Thank you,

-- 
Masami HIRAMATSU
2nd Dept. Linux Technology Center
Hitachi, Ltd., Systems Development Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com
--


[ message continues ]
Previous thread: Slow disks. by Rogier Wolff on Monday, December 20, 2010 - 7:15 am. (38 messages)

Next thread: [PATCH 1/2] staging: tidspbridge: convert dmm_map_lock to sema by Felipe Contreras on Monday, December 20, 2010 - 7:25 am. (1 message)