Skip to content

Commit 58d3a4c

Browse files
captain5050acmel
captain5050
authored and
acmel
committed
perf parse-events: Name the two term enums
Name the enums used by 'struct parse_events_term' to parse_events__term_val_type and parse_events__term_type. This allows greater compile time error checking. Fix -Wswitch related issues by explicitly listing all enum values prior to default. Add config_term_name to safely look up a parse_events__term_type name, bounds checking the array access first. Add documentation to 'struct parse_events_terms' and reorder to save space. Signed-off-by: Ian Rogers <irogers@google.com> Tested-by: Kan Liang <kan.liang@linux.intel.com> Cc: Adrian Hunter <adrian.hunter@intel.com> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com> Cc: Ingo Molnar <mingo@redhat.com> Cc: James Clark <james.clark@arm.com> Cc: Jiri Olsa <jolsa@kernel.org> 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/20230831071421.2201358-3-irogers@google.com Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
1 parent 478c3f5 commit 58d3a4c

File tree

4 files changed

+187
-67
lines changed

4 files changed

+187
-67
lines changed

‎tools/perf/util/parse-events.c

Lines changed: 128 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@ const char *event_type(int type)
153153
return "unknown";
154154
}
155155

156-
static char *get_config_str(struct list_head *head_terms, int type_term)
156+
static char *get_config_str(struct list_head *head_terms, enum parse_events__term_type type_term)
157157
{
158158
struct parse_events_term *term;
159159

@@ -722,7 +722,7 @@ int parse_events_add_breakpoint(struct parse_events_state *parse_state,
722722

723723
static int check_type_val(struct parse_events_term *term,
724724
struct parse_events_error *err,
725-
int type)
725+
enum parse_events__term_val_type type)
726726
{
727727
if (type == term->type_val)
728728
return 0;
@@ -737,42 +737,49 @@ static int check_type_val(struct parse_events_term *term,
737737
return -EINVAL;
738738
}
739739

740-
/*
741-
* Update according to parse-events.l
742-
*/
743-
static const char *config_term_names[__PARSE_EVENTS__TERM_TYPE_NR] = {
744-
[PARSE_EVENTS__TERM_TYPE_USER] = "<sysfs term>",
745-
[PARSE_EVENTS__TERM_TYPE_CONFIG] = "config",
746-
[PARSE_EVENTS__TERM_TYPE_CONFIG1] = "config1",
747-
[PARSE_EVENTS__TERM_TYPE_CONFIG2] = "config2",
748-
[PARSE_EVENTS__TERM_TYPE_CONFIG3] = "config3",
749-
[PARSE_EVENTS__TERM_TYPE_NAME] = "name",
750-
[PARSE_EVENTS__TERM_TYPE_SAMPLE_PERIOD] = "period",
751-
[PARSE_EVENTS__TERM_TYPE_SAMPLE_FREQ] = "freq",
752-
[PARSE_EVENTS__TERM_TYPE_BRANCH_SAMPLE_TYPE] = "branch_type",
753-
[PARSE_EVENTS__TERM_TYPE_TIME] = "time",
754-
[PARSE_EVENTS__TERM_TYPE_CALLGRAPH] = "call-graph",
755-
[PARSE_EVENTS__TERM_TYPE_STACKSIZE] = "stack-size",
756-
[PARSE_EVENTS__TERM_TYPE_NOINHERIT] = "no-inherit",
757-
[PARSE_EVENTS__TERM_TYPE_INHERIT] = "inherit",
758-
[PARSE_EVENTS__TERM_TYPE_MAX_STACK] = "max-stack",
759-
[PARSE_EVENTS__TERM_TYPE_MAX_EVENTS] = "nr",
760-
[PARSE_EVENTS__TERM_TYPE_OVERWRITE] = "overwrite",
761-
[PARSE_EVENTS__TERM_TYPE_NOOVERWRITE] = "no-overwrite",
762-
[PARSE_EVENTS__TERM_TYPE_DRV_CFG] = "driver-config",
763-
[PARSE_EVENTS__TERM_TYPE_PERCORE] = "percore",
764-
[PARSE_EVENTS__TERM_TYPE_AUX_OUTPUT] = "aux-output",
765-
[PARSE_EVENTS__TERM_TYPE_AUX_SAMPLE_SIZE] = "aux-sample-size",
766-
[PARSE_EVENTS__TERM_TYPE_METRIC_ID] = "metric-id",
767-
[PARSE_EVENTS__TERM_TYPE_RAW] = "raw",
768-
[PARSE_EVENTS__TERM_TYPE_LEGACY_CACHE] = "legacy-cache",
769-
[PARSE_EVENTS__TERM_TYPE_HARDWARE] = "hardware",
770-
};
771-
772740
static bool config_term_shrinked;
773741

742+
static const char *config_term_name(enum parse_events__term_type term_type)
743+
{
744+
/*
745+
* Update according to parse-events.l
746+
*/
747+
static const char *config_term_names[__PARSE_EVENTS__TERM_TYPE_NR] = {
748+
[PARSE_EVENTS__TERM_TYPE_USER] = "<sysfs term>",
749+
[PARSE_EVENTS__TERM_TYPE_CONFIG] = "config",
750+
[PARSE_EVENTS__TERM_TYPE_CONFIG1] = "config1",
751+
[PARSE_EVENTS__TERM_TYPE_CONFIG2] = "config2",
752+
[PARSE_EVENTS__TERM_TYPE_CONFIG3] = "config3",
753+
[PARSE_EVENTS__TERM_TYPE_NAME] = "name",
754+
[PARSE_EVENTS__TERM_TYPE_SAMPLE_PERIOD] = "period",
755+
[PARSE_EVENTS__TERM_TYPE_SAMPLE_FREQ] = "freq",
756+
[PARSE_EVENTS__TERM_TYPE_BRANCH_SAMPLE_TYPE] = "branch_type",
757+
[PARSE_EVENTS__TERM_TYPE_TIME] = "time",
758+
[PARSE_EVENTS__TERM_TYPE_CALLGRAPH] = "call-graph",
759+
[PARSE_EVENTS__TERM_TYPE_STACKSIZE] = "stack-size",
760+
[PARSE_EVENTS__TERM_TYPE_NOINHERIT] = "no-inherit",
761+
[PARSE_EVENTS__TERM_TYPE_INHERIT] = "inherit",
762+
[PARSE_EVENTS__TERM_TYPE_MAX_STACK] = "max-stack",
763+
[PARSE_EVENTS__TERM_TYPE_MAX_EVENTS] = "nr",
764+
[PARSE_EVENTS__TERM_TYPE_OVERWRITE] = "overwrite",
765+
[PARSE_EVENTS__TERM_TYPE_NOOVERWRITE] = "no-overwrite",
766+
[PARSE_EVENTS__TERM_TYPE_DRV_CFG] = "driver-config",
767+
[PARSE_EVENTS__TERM_TYPE_PERCORE] = "percore",
768+
[PARSE_EVENTS__TERM_TYPE_AUX_OUTPUT] = "aux-output",
769+
[PARSE_EVENTS__TERM_TYPE_AUX_SAMPLE_SIZE] = "aux-sample-size",
770+
[PARSE_EVENTS__TERM_TYPE_METRIC_ID] = "metric-id",
771+
[PARSE_EVENTS__TERM_TYPE_RAW] = "raw",
772+
[PARSE_EVENTS__TERM_TYPE_LEGACY_CACHE] = "legacy-cache",
773+
[PARSE_EVENTS__TERM_TYPE_HARDWARE] = "hardware",
774+
};
775+
if ((unsigned int)term_type >= __PARSE_EVENTS__TERM_TYPE_NR)
776+
return "unknown term";
777+
778+
return config_term_names[term_type];
779+
}
780+
774781
static bool
775-
config_term_avail(int term_type, struct parse_events_error *err)
782+
config_term_avail(enum parse_events__term_type term_type, struct parse_events_error *err)
776783
{
777784
char *err_str;
778785

@@ -794,13 +801,31 @@ config_term_avail(int term_type, struct parse_events_error *err)
794801
case PARSE_EVENTS__TERM_TYPE_SAMPLE_PERIOD:
795802
case PARSE_EVENTS__TERM_TYPE_PERCORE:
796803
return true;
804+
case PARSE_EVENTS__TERM_TYPE_USER:
805+
case PARSE_EVENTS__TERM_TYPE_SAMPLE_FREQ:
806+
case PARSE_EVENTS__TERM_TYPE_BRANCH_SAMPLE_TYPE:
807+
case PARSE_EVENTS__TERM_TYPE_TIME:
808+
case PARSE_EVENTS__TERM_TYPE_CALLGRAPH:
809+
case PARSE_EVENTS__TERM_TYPE_STACKSIZE:
810+
case PARSE_EVENTS__TERM_TYPE_NOINHERIT:
811+
case PARSE_EVENTS__TERM_TYPE_INHERIT:
812+
case PARSE_EVENTS__TERM_TYPE_MAX_STACK:
813+
case PARSE_EVENTS__TERM_TYPE_MAX_EVENTS:
814+
case PARSE_EVENTS__TERM_TYPE_NOOVERWRITE:
815+
case PARSE_EVENTS__TERM_TYPE_OVERWRITE:
816+
case PARSE_EVENTS__TERM_TYPE_DRV_CFG:
817+
case PARSE_EVENTS__TERM_TYPE_AUX_OUTPUT:
818+
case PARSE_EVENTS__TERM_TYPE_AUX_SAMPLE_SIZE:
819+
case PARSE_EVENTS__TERM_TYPE_RAW:
820+
case PARSE_EVENTS__TERM_TYPE_LEGACY_CACHE:
821+
case PARSE_EVENTS__TERM_TYPE_HARDWARE:
797822
default:
798823
if (!err)
799824
return false;
800825

801826
/* term_type is validated so indexing is safe */
802827
if (asprintf(&err_str, "'%s' is not usable in 'perf stat'",
803-
config_term_names[term_type]) >= 0)
828+
config_term_name(term_type)) >= 0)
804829
parse_events_error__handle(err, -1, err_str, NULL);
805830
return false;
806831
}
@@ -918,10 +943,14 @@ do { \
918943
return -EINVAL;
919944
}
920945
break;
946+
case PARSE_EVENTS__TERM_TYPE_DRV_CFG:
947+
case PARSE_EVENTS__TERM_TYPE_USER:
948+
case PARSE_EVENTS__TERM_TYPE_LEGACY_CACHE:
949+
case PARSE_EVENTS__TERM_TYPE_HARDWARE:
921950
default:
922951
parse_events_error__handle(err, term->err_term,
923-
strdup("unknown term"),
924-
parse_events_formats_error_string(NULL));
952+
strdup(config_term_name(term->type_term)),
953+
parse_events_formats_error_string(NULL));
925954
return -EINVAL;
926955
}
927956

@@ -1007,10 +1036,26 @@ static int config_term_tracepoint(struct perf_event_attr *attr,
10071036
case PARSE_EVENTS__TERM_TYPE_AUX_OUTPUT:
10081037
case PARSE_EVENTS__TERM_TYPE_AUX_SAMPLE_SIZE:
10091038
return config_term_common(attr, term, err);
1039+
case PARSE_EVENTS__TERM_TYPE_USER:
1040+
case PARSE_EVENTS__TERM_TYPE_CONFIG:
1041+
case PARSE_EVENTS__TERM_TYPE_CONFIG1:
1042+
case PARSE_EVENTS__TERM_TYPE_CONFIG2:
1043+
case PARSE_EVENTS__TERM_TYPE_CONFIG3:
1044+
case PARSE_EVENTS__TERM_TYPE_NAME:
1045+
case PARSE_EVENTS__TERM_TYPE_SAMPLE_PERIOD:
1046+
case PARSE_EVENTS__TERM_TYPE_SAMPLE_FREQ:
1047+
case PARSE_EVENTS__TERM_TYPE_BRANCH_SAMPLE_TYPE:
1048+
case PARSE_EVENTS__TERM_TYPE_TIME:
1049+
case PARSE_EVENTS__TERM_TYPE_DRV_CFG:
1050+
case PARSE_EVENTS__TERM_TYPE_PERCORE:
1051+
case PARSE_EVENTS__TERM_TYPE_METRIC_ID:
1052+
case PARSE_EVENTS__TERM_TYPE_RAW:
1053+
case PARSE_EVENTS__TERM_TYPE_LEGACY_CACHE:
1054+
case PARSE_EVENTS__TERM_TYPE_HARDWARE:
10101055
default:
10111056
if (err) {
10121057
parse_events_error__handle(err, term->err_term,
1013-
strdup("unknown term"),
1058+
strdup(config_term_name(term->type_term)),
10141059
strdup("valid terms: call-graph,stack-size\n"));
10151060
}
10161061
return -EINVAL;
@@ -1128,6 +1173,16 @@ do { \
11281173
ADD_CONFIG_TERM_VAL(AUX_SAMPLE_SIZE, aux_sample_size,
11291174
term->val.num, term->weak);
11301175
break;
1176+
case PARSE_EVENTS__TERM_TYPE_USER:
1177+
case PARSE_EVENTS__TERM_TYPE_CONFIG:
1178+
case PARSE_EVENTS__TERM_TYPE_CONFIG1:
1179+
case PARSE_EVENTS__TERM_TYPE_CONFIG2:
1180+
case PARSE_EVENTS__TERM_TYPE_CONFIG3:
1181+
case PARSE_EVENTS__TERM_TYPE_NAME:
1182+
case PARSE_EVENTS__TERM_TYPE_METRIC_ID:
1183+
case PARSE_EVENTS__TERM_TYPE_RAW:
1184+
case PARSE_EVENTS__TERM_TYPE_LEGACY_CACHE:
1185+
case PARSE_EVENTS__TERM_TYPE_HARDWARE:
11311186
default:
11321187
break;
11331188
}
@@ -1157,6 +1212,30 @@ static int get_config_chgs(struct perf_pmu *pmu, struct list_head *head_config,
11571212
case PARSE_EVENTS__TERM_TYPE_CONFIG:
11581213
bits = ~(u64)0;
11591214
break;
1215+
case PARSE_EVENTS__TERM_TYPE_CONFIG1:
1216+
case PARSE_EVENTS__TERM_TYPE_CONFIG2:
1217+
case PARSE_EVENTS__TERM_TYPE_CONFIG3:
1218+
case PARSE_EVENTS__TERM_TYPE_NAME:
1219+
case PARSE_EVENTS__TERM_TYPE_SAMPLE_PERIOD:
1220+
case PARSE_EVENTS__TERM_TYPE_SAMPLE_FREQ:
1221+
case PARSE_EVENTS__TERM_TYPE_BRANCH_SAMPLE_TYPE:
1222+
case PARSE_EVENTS__TERM_TYPE_TIME:
1223+
case PARSE_EVENTS__TERM_TYPE_CALLGRAPH:
1224+
case PARSE_EVENTS__TERM_TYPE_STACKSIZE:
1225+
case PARSE_EVENTS__TERM_TYPE_NOINHERIT:
1226+
case PARSE_EVENTS__TERM_TYPE_INHERIT:
1227+
case PARSE_EVENTS__TERM_TYPE_MAX_STACK:
1228+
case PARSE_EVENTS__TERM_TYPE_MAX_EVENTS:
1229+
case PARSE_EVENTS__TERM_TYPE_NOOVERWRITE:
1230+
case PARSE_EVENTS__TERM_TYPE_OVERWRITE:
1231+
case PARSE_EVENTS__TERM_TYPE_DRV_CFG:
1232+
case PARSE_EVENTS__TERM_TYPE_PERCORE:
1233+
case PARSE_EVENTS__TERM_TYPE_AUX_OUTPUT:
1234+
case PARSE_EVENTS__TERM_TYPE_AUX_SAMPLE_SIZE:
1235+
case PARSE_EVENTS__TERM_TYPE_METRIC_ID:
1236+
case PARSE_EVENTS__TERM_TYPE_RAW:
1237+
case PARSE_EVENTS__TERM_TYPE_LEGACY_CACHE:
1238+
case PARSE_EVENTS__TERM_TYPE_HARDWARE:
11601239
default:
11611240
break;
11621241
}
@@ -2386,7 +2465,8 @@ static int new_term(struct parse_events_term **_term,
23862465
}
23872466

23882467
int parse_events_term__num(struct parse_events_term **term,
2389-
int type_term, const char *config, u64 num,
2468+
enum parse_events__term_type type_term,
2469+
const char *config, u64 num,
23902470
bool no_value,
23912471
void *loc_term_, void *loc_val_)
23922472
{
@@ -2396,7 +2476,7 @@ int parse_events_term__num(struct parse_events_term **term,
23962476
struct parse_events_term temp = {
23972477
.type_val = PARSE_EVENTS__TERM_TYPE_NUM,
23982478
.type_term = type_term,
2399-
.config = config ? : strdup(config_term_names[type_term]),
2479+
.config = config ? : strdup(config_term_name(type_term)),
24002480
.no_value = no_value,
24012481
.err_term = loc_term ? loc_term->first_column : 0,
24022482
.err_val = loc_val ? loc_val->first_column : 0,
@@ -2406,7 +2486,8 @@ int parse_events_term__num(struct parse_events_term **term,
24062486
}
24072487

24082488
int parse_events_term__str(struct parse_events_term **term,
2409-
int type_term, char *config, char *str,
2489+
enum parse_events__term_type type_term,
2490+
char *config, char *str,
24102491
void *loc_term_, void *loc_val_)
24112492
{
24122493
YYLTYPE *loc_term = loc_term_;
@@ -2424,11 +2505,12 @@ int parse_events_term__str(struct parse_events_term **term,
24242505
}
24252506

24262507
int parse_events_term__term(struct parse_events_term **term,
2427-
int term_lhs, int term_rhs,
2508+
enum parse_events__term_type term_lhs,
2509+
enum parse_events__term_type term_rhs,
24282510
void *loc_term, void *loc_val)
24292511
{
24302512
return parse_events_term__str(term, term_lhs, NULL,
2431-
strdup(config_term_names[term_rhs]),
2513+
strdup(config_term_name(term_rhs)),
24322514
loc_term, loc_val);
24332515
}
24342516

@@ -2539,7 +2621,7 @@ int parse_events_term__to_strbuf(struct list_head *term_list, struct strbuf *sb)
25392621
if (ret < 0)
25402622
return ret;
25412623
} else if (term->type_term < __PARSE_EVENTS__TERM_TYPE_NR) {
2542-
ret = strbuf_addf(sb, "%s=", config_term_names[term->type_term]);
2624+
ret = strbuf_addf(sb, "%s=", config_term_name(term->type_term));
25432625
if (ret < 0)
25442626
return ret;
25452627
}
@@ -2567,7 +2649,7 @@ static void config_terms_list(char *buf, size_t buf_sz)
25672649

25682650
buf[0] = '\0';
25692651
for (i = 0; i < __PARSE_EVENTS__TERM_TYPE_NR; i++) {
2570-
const char *name = config_term_names[i];
2652+
const char *name = config_term_name(i);
25712653

25722654
if (!config_term_avail(i, NULL))
25732655
continue;

‎tools/perf/util/parse-events.h

Lines changed: 45 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -48,12 +48,12 @@ int parse_events_terms(struct list_head *terms, const char *str, FILE *input);
4848
int parse_filter(const struct option *opt, const char *str, int unset);
4949
int exclude_perf(const struct option *opt, const char *arg, int unset);
5050

51-
enum {
51+
enum parse_events__term_val_type {
5252
PARSE_EVENTS__TERM_TYPE_NUM,
5353
PARSE_EVENTS__TERM_TYPE_STR,
5454
};
5555

56-
enum {
56+
enum parse_events__term_type {
5757
PARSE_EVENTS__TERM_TYPE_USER,
5858
PARSE_EVENTS__TERM_TYPE_CONFIG,
5959
PARSE_EVENTS__TERM_TYPE_CONFIG1,
@@ -80,27 +80,54 @@ enum {
8080
PARSE_EVENTS__TERM_TYPE_RAW,
8181
PARSE_EVENTS__TERM_TYPE_LEGACY_CACHE,
8282
PARSE_EVENTS__TERM_TYPE_HARDWARE,
83-
__PARSE_EVENTS__TERM_TYPE_NR,
83+
#define __PARSE_EVENTS__TERM_TYPE_NR (PARSE_EVENTS__TERM_TYPE_HARDWARE + 1)
8484
};
8585

8686
struct parse_events_term {
87+
/** @list: The term list the term is a part of. */
88+
struct list_head list;
89+
/**
90+
* @config: The left-hand side of a term assignment, so the term
91+
* "event=8" would have the config be "event"
92+
*/
8793
const char *config;
94+
/**
95+
* @val: The right-hand side of a term assignment that can either be a
96+
* string or a number depending on type_val.
97+
*/
8898
union {
8999
char *str;
90100
u64 num;
91101
} val;
92-
int type_val;
93-
int type_term;
94-
struct list_head list;
95-
bool used;
96-
bool no_value;
97-
98-
/* error string indexes for within parsed string */
102+
/** @type_val: The union variable in val to be used for the term. */
103+
enum parse_events__term_val_type type_val;
104+
/**
105+
* @type_term: A predefined term type or PARSE_EVENTS__TERM_TYPE_USER
106+
* when not inbuilt.
107+
*/
108+
enum parse_events__term_type type_term;
109+
/**
110+
* @err_term: The column index of the term from parsing, used during
111+
* error output.
112+
*/
99113
int err_term;
114+
/**
115+
* @err_val: The column index of the val from parsing, used during error
116+
* output.
117+
*/
100118
int err_val;
101-
102-
/* Coming from implicit alias */
119+
/** @used: Was the term used during parameterized-eval. */
120+
bool used;
121+
/**
122+
* @weak: A term from the sysfs or json encoding of an event that
123+
* shouldn't override terms coming from the command line.
124+
*/
103125
bool weak;
126+
/**
127+
* @no_value: Is there no value. TODO: this should really be part of
128+
* type_val.
129+
*/
130+
bool no_value;
104131
};
105132

106133
struct parse_events_error {
@@ -139,14 +166,17 @@ bool parse_events__filter_pmu(const struct parse_events_state *parse_state,
139166
void parse_events__shrink_config_terms(void);
140167
int parse_events__is_hardcoded_term(struct parse_events_term *term);
141168
int parse_events_term__num(struct parse_events_term **term,
142-
int type_term, const char *config, u64 num,
169+
enum parse_events__term_type type_term,
170+
const char *config, u64 num,
143171
bool novalue,
144172
void *loc_term, void *loc_val);
145173
int parse_events_term__str(struct parse_events_term **term,
146-
int type_term, char *config, char *str,
174+
enum parse_events__term_type type_term,
175+
char *config, char *str,
147176
void *loc_term, void *loc_val);
148177
int parse_events_term__term(struct parse_events_term **term,
149-
int term_lhs, int term_rhs,
178+
enum parse_events__term_type term_lhs,
179+
enum parse_events__term_type term_rhs,
150180
void *loc_term, void *loc_val);
151181
int parse_events_term__clone(struct parse_events_term **new,
152182
struct parse_events_term *term);

‎tools/perf/util/parse-events.l

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ static int tool(yyscan_t scanner, enum perf_tool_event event)
116116
return PE_VALUE_SYM_TOOL;
117117
}
118118

119-
static int term(yyscan_t scanner, int type)
119+
static int term(yyscan_t scanner, enum parse_events__term_type type)
120120
{
121121
YYSTYPE *yylval = parse_events_get_lval(scanner);
122122

0 commit comments

Comments
 (0)