Skip to contentSkip to conversation

perf: fix missing event name init for default event (v2)

PATCH In review Stephane Eranian 1 participant 4 messages Jun 7, 2011 – Aug 18, 2011 perf events

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 perf: fix missing event name init for default event (v2)

When no event is given to perf record, perf top, a default event is initialized (cycles). However, perf_evlist__add_default() was not setting the symbolic name for the event. Perf top worked simply because it was reconstructing the name from the event code. But it should not have to do this. This patch initializes the evsel->name field properly.

This second version improves the code flow on the non error path.

---

tools/perf/util/evlist.c +10 -1
index b021ea9..9a98c28 100644
@@ -85,10 +85,19 @@ int perf_evlist__add_default(struct perf_evlist *evlist)
 	struct perf_evsel *evsel = perf_evsel__new(&attr, 0);
 
 	if (evsel == NULL)
-		return -ENOMEM;
+		goto error;
+
+	/* use strdup() because free(evsel) assumes name is allocated */
+	evsel->name = strdup("cycles");
+	if (!evsel->name)
+		goto error_free;
 
 	perf_evlist__add(evlist, evsel);
 	return 0;
+error_free:
+	free(evsel);
+error:
+	return -ENOMEM;
 }
 
 int perf_evlist__alloc_pollfd(struct perf_evlist *evlist)
#1 lore raw

PATCH 1/1 : perf: fix error handling of unknown events

There was a problem with the parse_events() code not printing the correct event name when an event was unknown and starting with an 'r'. The source of the problem was the way raw notation was parsed.

Without the patch:

	$ perf stat -e retired_foo
	invalid event modifier: 'tired_foo'

With the patch:

	$ perf stat -e retired_foo
	invalid or unsupported event: 'retired_foo'

This also covers the case where the name of the event was not printed at all when perf was linked with libpfm4.

---

tools/perf/util/parse-events.c +5 -1
index 4ea7e19..8c1cb10 100644
@@ -697,7 +697,11 @@ parse_raw_event(const char **strp, struct perf_event_attr *attr)
 		return EVT_FAILED;
 	n = hex2u64(str + 1, &config);
 	if (n > 0) {
-		*strp = str + n + 1;
+		const char *end = str + n + 1;
+		if (*end != '\0' && *end != ',' && *end != ':')
+			return EVT_FAILED;
+
+		*strp = end;
 		attr->type = PERF_TYPE_RAW;
 		attr->config = config;
 		return EVT_HANDLED;
#2 lore raw

[tip:perf/core] perf tools: Fix error handling of unknown events

Commit-ID:  777d1d71db622a5e1ff703495741c3d257b532e5
Gitweb:     http://git.kernel.org/tip/777d1d71db622a5e1ff703495741c3d257b532e5
Author:     Stephane Eranian <[email protected]>
AuthorDate: Sat, 23 Jul 2011 04:10:43 +0200
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Thu, 18 Aug 2011 07:21:13 -0300

perf tools: Fix error handling of unknown events

There was a problem with the parse_events() code not printing the correct event name when an event was unknown and starting with an 'r'. The source of the problem was the way raw notation was parsed.

Without the patch:

	$ perf stat -e retired_foo
	invalid event modifier: 'tired_foo'

With the patch:

	$ perf stat -e retired_foo
	invalid or unsupported event: 'retired_foo'

This also covers the case where the name of the event was not printed at all when perf was linked with libpfm4.

---
 tools/perf/util/parse-events.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)
tools/perf/util/parse-events.c +5 -1
index d93f3ce..928918b 100644
@@ -697,7 +697,11 @@ parse_raw_event(const char **strp, struct perf_event_attr *attr)
 		return EVT_FAILED;
 	n = hex2u64(str + 1, &config);
 	if (n > 0) {
-		*strp = str + n + 1;
+		const char *end = str + n + 1;
+		if (*end != '\0' && *end != ',' && *end != ':')
+			return EVT_FAILED;
+
+		*strp = end;
 		attr->type = PERF_TYPE_RAW;
 		attr->config = config;
 		return EVT_HANDLED;
#3 lore raw

[tip:perf/core] perf evlist: Fix missing event name init for default event

In reply to Stephane Eranian
Commit-ID:  cc2d86b04d9ac28a6be6cb05da6ea8f014fd5aa0
Gitweb:     http://git.kernel.org/tip/cc2d86b04d9ac28a6be6cb05da6ea8f014fd5aa0
Author:     Stephane Eranian <[email protected]>
AuthorDate: Tue, 7 Jun 2011 18:19:36 +0200
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Thu, 18 Aug 2011 07:20:31 -0300

perf evlist: Fix missing event name init for default event

When no event is given to perf record, perf top, a default event is initialized (cycles). However, perf_evlist__add_default() was not setting the symbolic name for the event. Perf top worked simply because it was reconstructing the name from the event code. But it should not have to do this. This patch initializes the evsel->name field properly.

This second version improves the code flow on the non error path.

Cc Ingo Molnar <[email protected]>
Cc Peter Zijlstra <[email protected]>
[committer note: Use perf_evsel__delete() instead of plain free()]
---
 tools/perf/util/evlist.c |   11 ++++++++++-
 1 files changed, 10 insertions(+), 1 deletions(-)
tools/perf/util/evlist.c +10 -1
index e03e7bc..c12bd47 100644
@@ -85,10 +85,19 @@ int perf_evlist__add_default(struct perf_evlist *evlist)
 	struct perf_evsel *evsel = perf_evsel__new(&attr, 0);
 
 	if (evsel == NULL)
-		return -ENOMEM;
+		goto error;
+
+	/* use strdup() because free(evsel) assumes name is allocated */
+	evsel->name = strdup("cycles");
+	if (!evsel->name)
+		goto error_free;
 
 	perf_evlist__add(evlist, evsel);
 	return 0;
+error_free:
+	perf_evsel__delete(evsel);
+error:
+	return -ENOMEM;
 }
 
 void perf_evlist__disable(struct perf_evlist *evlist)