Re: [PATCH 3/5] perf tools: Propagate error info for the tracepoint parsing

From: RaphaÃl Beamonte
Date: Tue Sep 08 2015 - 17:42:51 EST


2015-09-07 4:38 GMT-04:00 Jiri Olsa <jolsa@xxxxxxxxxx>:
> Pass 'struct parse_events_error *error' to the parse-event.c
> tracepoint adding path. It will be filled with error data
> in following patches.
>
> Link: http://lkml.kernel.org/n/tip-las1hm5zf58b0twd27h9895b@xxxxxxxxxxxxxx
> Signed-off-by: Jiri Olsa <jolsa@xxxxxxxxxx>
> ---
> tools/perf/util/parse-events.c | 27 ++++++++++++++++-----------
> tools/perf/util/parse-events.h | 3 ++-
> tools/perf/util/parse-events.y | 4 ++--
> 3 files changed, 20 insertions(+), 14 deletions(-)
>
> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> index 3840176642f8..1b284b8ad243 100644
> --- a/tools/perf/util/parse-events.c
> +++ b/tools/perf/util/parse-events.c
> @@ -387,7 +387,8 @@ int parse_events_add_cache(struct list_head *list, int *idx,
> }
>
> static int add_tracepoint(struct list_head *list, int *idx,
> - char *sys_name, char *evt_name)
> + char *sys_name, char *evt_name,
> + struct parse_events_error *error __maybe_unused)
> {
> struct perf_evsel *evsel;
>
> @@ -401,7 +402,8 @@ static int add_tracepoint(struct list_head *list, int *idx,
> }
>
> static int add_tracepoint_multi_event(struct list_head *list, int *idx,
> - char *sys_name, char *evt_name)
> + char *sys_name, char *evt_name,
> + struct parse_events_error *error)
> {
> char evt_path[MAXPATHLEN];
> struct dirent *evt_ent;
> @@ -425,7 +427,7 @@ static int add_tracepoint_multi_event(struct list_head *list, int *idx,
> if (!strglobmatch(evt_ent->d_name, evt_name))
> continue;
>
> - ret = add_tracepoint(list, idx, sys_name, evt_ent->d_name);
> + ret = add_tracepoint(list, idx, sys_name, evt_ent->d_name, error);
> }
>
> closedir(evt_dir);
> @@ -433,15 +435,17 @@ static int add_tracepoint_multi_event(struct list_head *list, int *idx,
> }
>
> static int add_tracepoint_event(struct list_head *list, int *idx,
> - char *sys_name, char *evt_name)
> + char *sys_name, char *evt_name,
> + struct parse_events_error *error)
> {
> return strpbrk(evt_name, "*?") ?
> - add_tracepoint_multi_event(list, idx, sys_name, evt_name) :
> - add_tracepoint(list, idx, sys_name, evt_name);
> + add_tracepoint_multi_event(list, idx, sys_name, evt_name, error) :
> + add_tracepoint(list, idx, sys_name, evt_name, error);
> }
>
> static int add_tracepoint_multi_sys(struct list_head *list, int *idx,
> - char *sys_name, char *evt_name)
> + char *sys_name, char *evt_name,
> + struct parse_events_error *error)
> {
> struct dirent *events_ent;
> DIR *events_dir;
> @@ -465,7 +469,7 @@ static int add_tracepoint_multi_sys(struct list_head *list, int *idx,
> continue;
>
> ret = add_tracepoint_event(list, idx, events_ent->d_name,
> - evt_name);
> + evt_name, error);
> }
>
> closedir(events_dir);
> @@ -473,12 +477,13 @@ static int add_tracepoint_multi_sys(struct list_head *list, int *idx,
> }
>
> int parse_events_add_tracepoint(struct list_head *list, int *idx,
> - char *sys, char *event)
> + char *sys, char *event,
> + struct parse_events_error *error)
> {
> if (strpbrk(sys, "*?"))
> - return add_tracepoint_multi_sys(list, idx, sys, event);
> + return add_tracepoint_multi_sys(list, idx, sys, event, error);
> else
> - return add_tracepoint_event(list, idx, sys, event);
> + return add_tracepoint_event(list, idx, sys, event, error);
> }
>
> static int
> diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
> index a09b0e210997..ffee7ece75a6 100644
> --- a/tools/perf/util/parse-events.h
> +++ b/tools/perf/util/parse-events.h
> @@ -118,7 +118,8 @@ int parse_events__modifier_event(struct list_head *list, char *str, bool add);
> int parse_events__modifier_group(struct list_head *list, char *event_mod);
> int parse_events_name(struct list_head *list, char *name);
> int parse_events_add_tracepoint(struct list_head *list, int *idx,
> - char *sys, char *event);
> + char *sys, char *event,
> + struct parse_events_error *error);
> int parse_events_add_numeric(struct parse_events_evlist *data,
> struct list_head *list,
> u32 type, u64 config,
> diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y
> index 9cd70819c795..54a3004a8192 100644
> --- a/tools/perf/util/parse-events.y
> +++ b/tools/perf/util/parse-events.y
> @@ -376,7 +376,7 @@ PE_NAME '-' PE_NAME ':' PE_NAME
> snprintf(&sys_name, 128, "%s-%s", $1, $3);
>
> ALLOC_LIST(list);
> - ABORT_ON(parse_events_add_tracepoint(list, &data->idx, &sys_name, $5));
> + ABORT_ON(parse_events_add_tracepoint(list, &data->idx, &sys_name, $5, data->error));
> $$ = list;
> }
> |
> @@ -386,7 +386,7 @@ PE_NAME ':' PE_NAME
> struct list_head *list;
>
> ALLOC_LIST(list);
> - if (parse_events_add_tracepoint(list, &data->idx, $1, $3)) {
> + if (parse_events_add_tracepoint(list, &data->idx, $1, $3, data->error)) {
> struct parse_events_error *error = data->error;
>
> if (error) {
> --
> 2.4.3
>

Works for me.
Reviewed-by: RaphaÃl Beamonte <raphael.beamonte@xxxxxxxxx>

I also made sure I could compile and run perf with that patch applied
on top of the current linux master. Should I also propose my
Tested-by: tag? I didn't do thorough tests though.
--
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/