[PATCH v3 2/4] perf tools: Prompt proper error message for wrong terms of hw/sw events

[He Kuang]

Prompt proper error message and show valid terms when wrong config
terms is specificed for hw/sw type perf events. This patch makes the
original error prompt function formats_error_string() more generic,
which only outputs the static config terms for hw/sw perf events, and
prepends pmu formats for pmu events.

Before this patch:

  $ perf record -e 'cpu-clock/freqx=200/' -a sleep 1
  invalid or unsupported event: 'cpu-clock/freqx=200/'
  Run 'perf list' for a list of valid events

   usage: perf record [<options>] [<command>]
      or: perf record [<options>] -- <command> [<options>]

      -e, --event <event>   event selector. use 'perf list' to list available events

After this patch:

  $ perf record -e 'cpu-clock/freqx=200/' -a sleep 1
  event syntax error: 'cpu-clock/freqx=200/'
                                 \___ unknown term

  valid terms: config,config1,config2,name,period,freq,branch_type,time,call-graph,stack-size

  Run 'perf list' for a list of valid events

   usage: perf record [<options>] [<command>]
      or: perf record [<options>] -- <command> [<options>]

      -e, --event <event>   event selector. use 'perf list' to list available events

Signed-off-by: He Kuang <hekuang@xxxxxxxxxx>
---

Changes v2-v3:
   - Change example in commit message.
   - Update comment in flex file.
   - Move freeing additional_terms outside.

---
 tools/perf/util/parse-events.c | 29 +++++++++++++++++++++++++++++
 tools/perf/util/parse-events.h |  1 +
 tools/perf/util/parse-events.l |  2 +-
 tools/perf/util/pmu.c          | 37 ++++++++++++++-----------------------
 4 files changed, 45 insertions(+), 24 deletions(-)

diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 9dc3fb6..ea64ec07 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -656,6 +656,9 @@ do {									   \
 		CHECK_TYPE_VAL(STR);
 		break;
 	default:
+		err->str = strdup("unknown term");
+		err->idx = term->err_term;
+		err->help = parse_events_formats_error_string(NULL);
 		return -EINVAL;
 	}
 
@@ -1875,3 +1878,29 @@ void parse_events_evlist_error(struct parse_events_evlist *data,
 	err->str = strdup(str);
 	WARN_ONCE(!err->str, "WARNING: failed to allocate error string");
 }
+
+/*
+ * Return string contains valid config terms of an event.
+ * @additional_terms: For terms such as PMU sysfs terms.
+ */
+char *parse_events_formats_error_string(char *additional_terms)
+{
+	char *str;
+	static const char *static_terms = "config,config1,config2,name,"
+					  "period,freq,branch_type,time,"
+					  "call-graph,stack-size\n";
+
+	/* valid terms */
+	if (additional_terms) {
+		if (!asprintf(&str, "valid terms: %s,%s",
+			      additional_terms, static_terms))
+			goto fail;
+	} else {
+		if (!asprintf(&str, "valid terms: %s", static_terms))
+			goto fail;
+	}
+	return str;
+
+fail:
+	return NULL;
+}
diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
index ffee7ec..c7b904a 100644
--- a/tools/perf/util/parse-events.h
+++ b/tools/perf/util/parse-events.h
@@ -156,5 +156,6 @@ int print_hwcache_events(const char *event_glob, bool name_only);
 extern int is_valid_tracepoint(const char *event_string);
 
 int valid_event_mount(const char *eventfs);
+char *parse_events_formats_error_string(char *additional_terms);
 
 #endif /* __PERF_PARSE_EVENTS_H */
diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
index 936d566..c29832b 100644
--- a/tools/perf/util/parse-events.l
+++ b/tools/perf/util/parse-events.l
@@ -174,7 +174,7 @@ modifier_bp	[rwx]{1,3}
 
 <config>{
 	/*
-	 * Please update formats_error_string any time
+	 * Please update parse_events_formats_error_string any time
 	 * new static term is added.
 	 */
 config			{ return term(yyscanner, PARSE_EVENTS__TERM_TYPE_CONFIG); }
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 89c91a1..ac42c97 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -626,38 +626,26 @@ static int pmu_resolve_param_term(struct parse_events_term *term,
 	return -1;
 }
 
-static char *formats_error_string(struct list_head *formats)
+static char *pmu_formats_string(struct list_head *formats)
 {
 	struct perf_pmu_format *format;
-	char *err, *str;
-	static const char *static_terms = "config,config1,config2,name,"
-					  "period,freq,branch_type,time,"
-					  "call-graph,stack-size\n";
+	char *str;
+	struct strbuf buf;
 	unsigned i = 0;
 
-	if (!asprintf(&str, "valid terms:"))
+	if (!formats)
 		return NULL;
 
+	strbuf_init(&buf, 0);
 	/* sysfs exported terms */
-	list_for_each_entry(format, formats, list) {
-		char c = i++ ? ',' : ' ';
-
-		err = str;
-		if (!asprintf(&str, "%s%c%s", err, c, format->name))
-			goto fail;
-		free(err);
-	}
+	list_for_each_entry(format, formats, list)
+		strbuf_addf(&buf, i++ ? ",%s" : "%s",
+			    format->name);
 
-	/* static terms */
-	err = str;
-	if (!asprintf(&str, "%s,%s", err, static_terms))
-		goto fail;
+	str = strbuf_detach(&buf, NULL);
+	strbuf_release(&buf);
 
-	free(err);
 	return str;
-fail:
-	free(err);
-	return NULL;
 }
 
 /*
@@ -693,9 +681,12 @@ static int pmu_config_term(struct list_head *formats,
 		if (verbose)
 			printf("Invalid event/parameter '%s'\n", term->config);
 		if (err) {
+			char *pmu_term = pmu_formats_string(formats);
+
 			err->idx  = term->err_term;
 			err->str  = strdup("unknown term");
-			err->help = formats_error_string(formats);
+			err->help = parse_events_formats_error_string(pmu_term);
+			free(pmu_term);
 		}
 		return -EINVAL;
 	}
-- 
1.8.5.2

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/