perf parse-events: Introduce 'struct parse_events_terms'

parse_events_terms() existed in function names but was passed a
'struct list_head'.

As many parse_events functions take an evsel_config list as well as a
parse_event_term list, and the naming head_terms and head_config is
inconsistent, there's a potential to switch the lists and get errors.

Introduce a 'struct parse_events_terms', that just wraps a list_head, to
avoid this. Add the regular init/exit functions and transition the code
to use them.

Reviewed-by: James Clark <james.clark@arm.com>
Signed-off-by: Ian Rogers <irogers@google.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Kan Liang <kan.liang@linux.intel.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Rob Herring <robh@kernel.org>
Link: https://lore.kernel.org/r/20230901233949.2930562-6-irogers@google.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

1 files changed, 82 insertions, 76 deletions

diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 06a844bcce4a..c56e07bd7dd6 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -34,8 +34,9 @@
 #ifdef PARSER_DEBUG
 extern int parse_events_debug;
 #endif
-static int get_config_terms(struct list_head *head_config, struct list_head *head_terms);
-static int parse_events_terms__copy(const struct list_head *src, struct list_head *dest);
+static int get_config_terms(struct parse_events_terms *head_config, struct list_head *head_terms);
+static int parse_events_terms__copy(const struct parse_events_terms *src,
+				    struct parse_events_terms *dest);
 
 struct event_symbol event_symbols_hw[PERF_COUNT_HW_MAX] = {
 	[PERF_COUNT_HW_CPU_CYCLES] = {
@@ -153,26 +154,27 @@ const char *event_type(int type)
 	return "unknown";
 }
 
-static char *get_config_str(struct list_head *head_terms, enum parse_events__term_type type_term)
+static char *get_config_str(struct parse_events_terms *head_terms,
+			    enum parse_events__term_type type_term)
 {
 	struct parse_events_term *term;
 
 	if (!head_terms)
 		return NULL;
 
-	list_for_each_entry(term, head_terms, list)
+	list_for_each_entry(term, &head_terms->terms, list)
 		if (term->type_term == type_term)
 			return term->val.str;
 
 	return NULL;
 }
 
-static char *get_config_metric_id(struct list_head *head_terms)
+static char *get_config_metric_id(struct parse_events_terms *head_terms)
 {
 	return get_config_str(head_terms, PARSE_EVENTS__TERM_TYPE_METRIC_ID);
 }
 
-static char *get_config_name(struct list_head *head_terms)
+static char *get_config_name(struct parse_events_terms *head_terms)
 {
 	return get_config_str(head_terms, PARSE_EVENTS__TERM_TYPE_NAME);
 }
@@ -188,11 +190,11 @@ static char *get_config_name(struct list_head *head_terms)
  * @config_terms: the list of terms that may contain a raw term.
  * @pmu: the PMU to scan for events from.
  */
-static void fix_raw(struct list_head *config_terms, struct perf_pmu *pmu)
+static void fix_raw(struct parse_events_terms *config_terms, struct perf_pmu *pmu)
 {
 	struct parse_events_term *term;
 
-	list_for_each_entry(term, config_terms, list) {
+	list_for_each_entry(term, &config_terms->terms, list) {
 		u64 num;
 
 		if (term->type_term != PARSE_EVENTS__TERM_TYPE_RAW)
@@ -356,7 +358,7 @@ static int config_term_common(struct perf_event_attr *attr,
 			      struct parse_events_term *term,
 			      struct parse_events_error *err);
 static int config_attr(struct perf_event_attr *attr,
-		       struct list_head *head,
+		       struct parse_events_terms *head,
 		       struct parse_events_error *err,
 		       config_term_func_t config_term);
 
@@ -442,7 +444,7 @@ bool parse_events__filter_pmu(const struct parse_events_state *parse_state,
 
 int parse_events_add_cache(struct list_head *list, int *idx, const char *name,
 			   struct parse_events_state *parse_state,
-			   struct list_head *head_config)
+			   struct parse_events_terms *head_config)
 {
 	struct perf_pmu *pmu = NULL;
 	bool found_supported = false;
@@ -520,7 +522,7 @@ static void tracepoint_error(struct parse_events_error *e, int err,
 static int add_tracepoint(struct list_head *list, int *idx,
 			  const char *sys_name, const char *evt_name,
 			  struct parse_events_error *err,
-			  struct list_head *head_config, void *loc_)
+			  struct parse_events_terms *head_config, void *loc_)
 {
 	YYLTYPE *loc = loc_;
 	struct evsel *evsel = evsel__newtp_idx(sys_name, evt_name, (*idx)++);
@@ -545,7 +547,7 @@ static int add_tracepoint(struct list_head *list, int *idx,
 static int add_tracepoint_multi_event(struct list_head *list, int *idx,
 				      const char *sys_name, const char *evt_name,
 				      struct parse_events_error *err,
-				      struct list_head *head_config, YYLTYPE *loc)
+				      struct parse_events_terms *head_config, YYLTYPE *loc)
 {
 	char *evt_path;
 	struct dirent *evt_ent;
@@ -593,7 +595,7 @@ static int add_tracepoint_multi_event(struct list_head *list, int *idx,
 static int add_tracepoint_event(struct list_head *list, int *idx,
 				const char *sys_name, const char *evt_name,
 				struct parse_events_error *err,
-				struct list_head *head_config, YYLTYPE *loc)
+				struct parse_events_terms *head_config, YYLTYPE *loc)
 {
 	return strpbrk(evt_name, "*?") ?
 		add_tracepoint_multi_event(list, idx, sys_name, evt_name,
@@ -605,7 +607,7 @@ static int add_tracepoint_event(struct list_head *list, int *idx,
 static int add_tracepoint_multi_sys(struct list_head *list, int *idx,
 				    const char *sys_name, const char *evt_name,
 				    struct parse_events_error *err,
-				    struct list_head *head_config, YYLTYPE *loc)
+				    struct parse_events_terms *head_config, YYLTYPE *loc)
 {
 	struct dirent *events_ent;
 	DIR *events_dir;
@@ -680,7 +682,7 @@ do {					\
 int parse_events_add_breakpoint(struct parse_events_state *parse_state,
 				struct list_head *list,
 				u64 addr, char *type, u64 len,
-				struct list_head *head_config __maybe_unused)
+				struct parse_events_terms *head_config)
 {
 	struct perf_event_attr attr;
 	LIST_HEAD(config_terms);
@@ -1066,20 +1068,20 @@ static int config_term_tracepoint(struct perf_event_attr *attr,
 #endif
 
 static int config_attr(struct perf_event_attr *attr,
-		       struct list_head *head,
+		       struct parse_events_terms *head,
 		       struct parse_events_error *err,
 		       config_term_func_t config_term)
 {
 	struct parse_events_term *term;
 
-	list_for_each_entry(term, head, list)
+	list_for_each_entry(term, &head->terms, list)
 		if (config_term(attr, term, err))
 			return -EINVAL;
 
 	return 0;
 }
 
-static int get_config_terms(struct list_head *head_config, struct list_head *head_terms)
+static int get_config_terms(struct parse_events_terms *head_config, struct list_head *head_terms)
 {
 #define ADD_CONFIG_TERM(__type, __weak)				\
 	struct evsel_config_term *__t;			\
@@ -1112,7 +1114,7 @@ do {								\
 
 	struct parse_events_term *term;
 
-	list_for_each_entry(term, head_config, list) {
+	list_for_each_entry(term, &head_config->terms, list) {
 		switch (term->type_term) {
 		case PARSE_EVENTS__TERM_TYPE_SAMPLE_PERIOD:
 			ADD_CONFIG_TERM_VAL(PERIOD, period, term->val.num, term->weak);
@@ -1193,14 +1195,14 @@ do {								\
  * Add EVSEL__CONFIG_TERM_CFG_CHG where cfg_chg will have a bit set for
  * each bit of attr->config that the user has changed.
  */
-static int get_config_chgs(struct perf_pmu *pmu, struct list_head *head_config,
+static int get_config_chgs(struct perf_pmu *pmu, struct parse_events_terms *head_config,
 			   struct list_head *head_terms)
 {
 	struct parse_events_term *term;
 	u64 bits = 0;
 	int type;
 
-	list_for_each_entry(term, head_config, list) {
+	list_for_each_entry(term, &head_config->terms, list) {
 		switch (term->type_term) {
 		case PARSE_EVENTS__TERM_TYPE_USER:
 			type = perf_pmu__format_type(pmu, term->config);
@@ -1250,7 +1252,7 @@ static int get_config_chgs(struct perf_pmu *pmu, struct list_head *head_config,
 int parse_events_add_tracepoint(struct list_head *list, int *idx,
 				const char *sys, const char *event,
 				struct parse_events_error *err,
-				struct list_head *head_config, void *loc_)
+				struct parse_events_terms *head_config, void *loc_)
 {
 	YYLTYPE *loc = loc_;
 #ifdef HAVE_LIBTRACEEVENT
@@ -1283,7 +1285,7 @@ int parse_events_add_tracepoint(struct list_head *list, int *idx,
 static int __parse_events_add_numeric(struct parse_events_state *parse_state,
 				struct list_head *list,
 				struct perf_pmu *pmu, u32 type, u32 extended_type,
-				u64 config, struct list_head *head_config)
+				u64 config, struct parse_events_terms *head_config)
 {
 	struct perf_event_attr attr;
 	LIST_HEAD(config_terms);
@@ -1319,7 +1321,7 @@ static int __parse_events_add_numeric(struct parse_events_state *parse_state,
 int parse_events_add_numeric(struct parse_events_state *parse_state,
 			     struct list_head *list,
 			     u32 type, u64 config,
-			     struct list_head *head_config,
+			     struct parse_events_terms *head_config,
 			     bool wildcard)
 {
 	struct perf_pmu *pmu = NULL;
@@ -1368,7 +1370,7 @@ static bool config_term_percore(struct list_head *config_terms)
 
 int parse_events_add_pmu(struct parse_events_state *parse_state,
 			 struct list_head *list, const char *name,
-			 const struct list_head *const_head_terms,
+			 const struct parse_events_terms *const_parsed_terms,
 			 bool auto_merge_stats, void *loc_)
 {
 	struct perf_event_attr attr;
@@ -1378,7 +1380,7 @@ int parse_events_add_pmu(struct parse_events_state *parse_state,
 	struct parse_events_error *err = parse_state->error;
 	YYLTYPE *loc = loc_;
 	LIST_HEAD(config_terms);
-	LIST_HEAD(head_terms);
+	struct parse_events_terms parsed_terms;
 
 	pmu = parse_state->fake_pmu ?: perf_pmus__find(name);
 
@@ -1392,8 +1394,9 @@ int parse_events_add_pmu(struct parse_events_state *parse_state,
 		return -EINVAL;
 	}
 
-	if (const_head_terms) {
-		int ret = parse_events_terms__copy(const_head_terms, &head_terms);
+	parse_events_terms__init(&parsed_terms);
+	if (const_parsed_terms) {
+		int ret = parse_events_terms__copy(const_parsed_terms, &parsed_terms);
 
 		if (ret)
 			return ret;
@@ -1403,17 +1406,17 @@ int parse_events_add_pmu(struct parse_events_state *parse_state,
 		struct strbuf sb;
 
 		strbuf_init(&sb, /*hint=*/ 0);
-		if (pmu->selectable && list_empty(&head_terms)) {
+		if (pmu->selectable && list_empty(&parsed_terms.terms)) {
 			strbuf_addf(&sb, "%s//", name);
 		} else {
 			strbuf_addf(&sb, "%s/", name);
-			parse_events_term__to_strbuf(&head_terms, &sb);
+			parse_events_terms__to_strbuf(&parsed_terms, &sb);
 			strbuf_addch(&sb, '/');
 		}
 		fprintf(stderr, "Attempt to add: %s\n", sb.buf);
 		strbuf_release(&sb);
 	}
-	fix_raw(&head_terms, pmu);
+	fix_raw(&parsed_terms, pmu);
 
 	if (pmu->default_config) {
 		memcpy(&attr, pmu->default_config, sizeof(struct perf_event_attr));
@@ -1422,7 +1425,7 @@ int parse_events_add_pmu(struct parse_events_state *parse_state,
 	}
 	attr.type = pmu->type;
 
-	if (list_empty(&head_terms)) {
+	if (list_empty(&parsed_terms.terms)) {
 		evsel = __add_event(list, &parse_state->idx, &attr,
 				    /*init_attr=*/true, /*name=*/NULL,
 				    /*metric_id=*/NULL, pmu,
@@ -1431,8 +1434,8 @@ int parse_events_add_pmu(struct parse_events_state *parse_state,
 		return evsel ? 0 : -ENOMEM;
 	}
 
-	if (!parse_state->fake_pmu && perf_pmu__check_alias(pmu, &head_terms, &info, err)) {
-		parse_events_terms__purge(&head_terms);
+	if (!parse_state->fake_pmu && perf_pmu__check_alias(pmu, &parsed_terms, &info, err)) {
+		parse_events_terms__exit(&parsed_terms);
 		return -EINVAL;
 	}
 
@@ -1440,7 +1443,7 @@ int parse_events_add_pmu(struct parse_events_state *parse_state,
 		struct strbuf sb;
 
 		strbuf_init(&sb, /*hint=*/ 0);
-		parse_events_term__to_strbuf(&head_terms, &sb);
+		parse_events_terms__to_strbuf(&parsed_terms, &sb);
 		fprintf(stderr, "..after resolving event: %s/%s/\n", name, sb.buf);
 		strbuf_release(&sb);
 	}
@@ -1449,13 +1452,13 @@ int parse_events_add_pmu(struct parse_events_state *parse_state,
 	 * Configure hardcoded terms first, no need to check
 	 * return value when called with fail == 0 ;)
 	 */
-	if (config_attr(&attr, &head_terms, parse_state->error, config_term_pmu)) {
-		parse_events_terms__purge(&head_terms);
+	if (config_attr(&attr, &parsed_terms, parse_state->error, config_term_pmu)) {
+		parse_events_terms__exit(&parsed_terms);
 		return -EINVAL;
 	}
 
-	if (get_config_terms(&head_terms, &config_terms)) {
-		parse_events_terms__purge(&head_terms);
+	if (get_config_terms(&parsed_terms, &config_terms)) {
+		parse_events_terms__exit(&parsed_terms);
 		return -ENOMEM;
 	}
 
@@ -1463,24 +1466,24 @@ int parse_events_add_pmu(struct parse_events_state *parse_state,
 	 * When using default config, record which bits of attr->config were
 	 * changed by the user.
 	 */
-	if (pmu->default_config && get_config_chgs(pmu, &head_terms, &config_terms)) {
-		parse_events_terms__purge(&head_terms);
+	if (pmu->default_config && get_config_chgs(pmu, &parsed_terms, &config_terms)) {
+		parse_events_terms__exit(&parsed_terms);
 		return -ENOMEM;
 	}
 
 	if (!parse_state->fake_pmu &&
-	    perf_pmu__config(pmu, &attr, &head_terms, parse_state->error)) {
+	    perf_pmu__config(pmu, &attr, &parsed_terms, parse_state->error)) {
 		free_config_terms(&config_terms);
-		parse_events_terms__purge(&head_terms);
+		parse_events_terms__exit(&parsed_terms);
 		return -EINVAL;
 	}
 
 	evsel = __add_event(list, &parse_state->idx, &attr, /*init_attr=*/true,
-			    get_config_name(&head_terms),
-			    get_config_metric_id(&head_terms), pmu,
+			    get_config_name(&parsed_terms),
+			    get_config_metric_id(&parsed_terms), pmu,
 			    &config_terms, auto_merge_stats, /*cpu_list=*/NULL);
 	if (!evsel) {
-		parse_events_terms__purge(&head_terms);
+		parse_events_terms__exit(&parsed_terms);
 		return -ENOMEM;
 	}
 
@@ -1490,11 +1493,11 @@ int parse_events_add_pmu(struct parse_events_state *parse_state,
 	evsel->percore = config_term_percore(&evsel->config_terms);
 
 	if (parse_state->fake_pmu) {
-		parse_events_terms__purge(&head_terms);
+		parse_events_terms__exit(&parsed_terms);
 		return 0;
 	}
 
-	parse_events_terms__purge(&head_terms);
+	parse_events_terms__exit(&parsed_terms);
 	free((char *)evsel->unit);
 	evsel->unit = strdup(info.unit);
 	evsel->scale = info.scale;
@@ -1505,7 +1508,7 @@ int parse_events_add_pmu(struct parse_events_state *parse_state,
 
 int parse_events_multi_pmu_add(struct parse_events_state *parse_state,
 			       const char *event_name,
-			       const struct list_head *const_head_terms,
+			       const struct parse_events_terms *const_parsed_terms,
 			       struct list_head **listp, void *loc_)
 {
 	struct parse_events_term *term;
@@ -1514,12 +1517,13 @@ int parse_events_multi_pmu_add(struct parse_events_state *parse_state,
 	YYLTYPE *loc = loc_;
 	int ok = 0;
 	const char *config;
-	LIST_HEAD(head_terms);
+	struct parse_events_terms parsed_terms;
 
 	*listp = NULL;
 
-	if (const_head_terms) {
-		int ret = parse_events_terms__copy(const_head_terms, &head_terms);
+	parse_events_terms__init(&parsed_terms);
+	if (const_parsed_terms) {
+		int ret = parse_events_terms__copy(const_parsed_terms, &parsed_terms);
 
 		if (ret)
 			return ret;
@@ -1536,7 +1540,7 @@ int parse_events_multi_pmu_add(struct parse_events_state *parse_state,
 		zfree(&config);
 		goto out_err;
 	}
-	list_add_tail(&term->list, &head_terms);
+	list_add_tail(&term->list, &parsed_terms.terms);
 
 	/* Add it for all PMUs that support the alias */
 	list = malloc(sizeof(struct list_head));
@@ -1556,11 +1560,11 @@ int parse_events_multi_pmu_add(struct parse_events_state *parse_state,
 
 		auto_merge_stats = perf_pmu__auto_merge_stats(pmu);
 		if (!parse_events_add_pmu(parse_state, list, pmu->name,
-					  &head_terms, auto_merge_stats, loc)) {
+					  &parsed_terms, auto_merge_stats, loc)) {
 			struct strbuf sb;
 
 			strbuf_init(&sb, /*hint=*/ 0);
-			parse_events_term__to_strbuf(&head_terms, &sb);
+			parse_events_terms__to_strbuf(&parsed_terms, &sb);
 			pr_debug("%s -> %s/%s/\n", event_name, pmu->name, sb.buf);
 			strbuf_release(&sb);
 			ok++;
@@ -1568,12 +1572,12 @@ int parse_events_multi_pmu_add(struct parse_events_state *parse_state,
 	}
 
 	if (parse_state->fake_pmu) {
-		if (!parse_events_add_pmu(parse_state, list, event_name, &head_terms,
+		if (!parse_events_add_pmu(parse_state, list, event_name, &parsed_terms,
 					  /*auto_merge_stats=*/true, loc)) {
 			struct strbuf sb;
 
 			strbuf_init(&sb, /*hint=*/ 0);
-			parse_events_term__to_strbuf(&head_terms, &sb);
+			parse_events_terms__to_strbuf(&parsed_terms, &sb);
 			pr_debug("%s -> %s/%s/\n", event_name, "fake_pmu", sb.buf);
 			strbuf_release(&sb);
 			ok++;
@@ -1581,7 +1585,7 @@ int parse_events_multi_pmu_add(struct parse_events_state *parse_state,
 	}
 
 out_err:
-	parse_events_terms__purge(&head_terms);
+	parse_events_terms__exit(&parsed_terms);
 	if (ok)
 		*listp = list;
 	else
@@ -1851,7 +1855,7 @@ static int parse_events__scanner(const char *str,
 /*
  * parse event config string, return a list of event terms.
  */
-int parse_events_terms(struct list_head *terms, const char *str, FILE *input)
+int parse_events_terms(struct parse_events_terms *terms, const char *str, FILE *input)
 {
 	struct parse_events_state parse_state = {
 		.terms  = NULL,
@@ -1860,14 +1864,10 @@ int parse_events_terms(struct list_head *terms, const char *str, FILE *input)
 	int ret;
 
 	ret = parse_events__scanner(str, input, &parse_state);
+	if (!ret)
+		list_splice(&parse_state.terms->terms, &terms->terms);
 
-	if (!ret) {
-		list_splice(parse_state.terms, terms);
-		zfree(&parse_state.terms);
-		return 0;
-	}
-
-	parse_events_terms__delete(parse_state.terms);
+	zfree(&parse_state.terms);
 	return ret;
 }
 
@@ -2563,11 +2563,12 @@ void parse_events_term__delete(struct parse_events_term *term)
 	free(term);
 }
 
-static int parse_events_terms__copy(const struct list_head *src, struct list_head *dest)
+static int parse_events_terms__copy(const struct parse_events_terms *src,
+				    struct parse_events_terms *dest)
 {
 	struct parse_events_term *term;
 
-	list_for_each_entry (term, src, list) {
+	list_for_each_entry (term, &src->terms, list) {
 		struct parse_events_term *n;
 		int ret;
 
@@ -2575,38 +2576,43 @@ static int parse_events_terms__copy(const struct list_head *src, struct list_hea
 		if (ret)
 			return ret;
 
-		list_add_tail(&n->list, dest);
+		list_add_tail(&n->list, &dest->terms);
 	}
 	return 0;
 }
 
-void parse_events_terms__purge(struct list_head *terms)
+void parse_events_terms__init(struct parse_events_terms *terms)
+{
+	INIT_LIST_HEAD(&terms->terms);
+}
+
+void parse_events_terms__exit(struct parse_events_terms *terms)
 {
 	struct parse_events_term *term, *h;
 
-	list_for_each_entry_safe(term, h, terms, list) {
+	list_for_each_entry_safe(term, h, &terms->terms, list) {
 		list_del_init(&term->list);
 		parse_events_term__delete(term);
 	}
 }
 
-void parse_events_terms__delete(struct list_head *terms)
+void parse_events_terms__delete(struct parse_events_terms *terms)
 {
 	if (!terms)
 		return;
-	parse_events_terms__purge(terms);
+	parse_events_terms__exit(terms);
 	free(terms);
 }
 
-int parse_events_term__to_strbuf(struct list_head *term_list, struct strbuf *sb)
+int parse_events_terms__to_strbuf(const struct parse_events_terms *terms, struct strbuf *sb)
 {
 	struct parse_events_term *term;
 	bool first = true;
 
-	if (!term_list)
+	if (!terms)
 		return 0;
 
-	list_for_each_entry(term, term_list, list) {
+	list_for_each_entry(term, &terms->terms, list) {
 		int ret;
 
 		if (!first) {