> From: Wangnan (F) [mailto:wangn...@huawei.com]
>
> Sorry, forget to CC kernel mailing list...
>
> On 2015/9/2 14:49, Wang Nan wrote:
> > If parse_events__scanner() collects no entry, perf_evlist__last(evlist)
> > is invalid.
> >
> > Although it shouldn't happen at this point, before calling
> > perf_evlist__last(), we should ensure the list is not empty for safety
> > reason.
> >
> > There are 3 places need this checking:
> >
> > 1. Before setting cmdline_group_boundary;
> > 2. Before __perf_evlist__set_leader();
> > 3. In foreach_evsel_in_last_glob.
> >
This looks OK to me.
Reviewed-by: Masami Hiramatsu <masami.hiramatsu...@hitachi.com>
Thanks!
> > Signed-off-by: Wang Nan <wangn...@huawei.com>
> > Cc: Arnaldo Carvalho de Melo <a...@redhat.com>
> > Cc: Alexei Starovoitov <a...@plumgrid.com>
> > Cc: Jiri Olsa <jo...@kernel.org>
> > Cc: Masami Hiramatsu <masami.hiramatsu...@hitachi.com>
> > Cc: Namhyung Kim <namhy...@kernel.org>
> > Cc: Zefan Li <lize...@huawei.com>
> > Cc: pi3or...@163.com
> > ---
> >
> > Merge all 3 list_empty() test together into one patch.
> >
> > Add warning messages.
> >
> > Improve commit message.
> >
> > ---
> > tools/perf/util/parse-events.c | 22 +++++++++++++++++++---
> > 1 file changed, 19 insertions(+), 3 deletions(-)
> >
> > diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> > index d826e6f..069848d 100644
> > --- a/tools/perf/util/parse-events.c
> > +++ b/tools/perf/util/parse-events.c
> > @@ -793,6 +793,11 @@ void parse_events__set_leader(char *name, struct
> > list_head *list)
> > {
> > struct perf_evsel *leader;
> >
> > + if (list_empty(list)) {
> > + __WARN_printf("WARNING: failed to set leader: empty list");
> > + return;
> > + }
> > +
> > __perf_evlist__set_leader(list);
> > leader = list_entry(list->next, struct perf_evsel, node);
> > leader->group_name = name ? strdup(name) : NULL;
> > @@ -1143,10 +1148,15 @@ int parse_events(struct perf_evlist *evlist, const
> > char *str,
> > int entries = data.idx - evlist->nr_entries;
> > struct perf_evsel *last;
> >
> > + if (!list_empty(&data.list)) {
> > + last = list_entry(data.list.prev,
> > + struct perf_evsel, node);
> > + last->cmdline_group_boundary = true;
> > + } else
> > + __WARN_printf("WARNING: event parser found nothing");
> > +
> > perf_evlist__splice_list_tail(evlist, &data.list, entries);
> > evlist->nr_groups += data.nr_groups;
> > - last = perf_evlist__last(evlist);
> > - last->cmdline_group_boundary = true;
> >
> > return 0;
> > }
> > @@ -1252,7 +1262,13 @@ foreach_evsel_in_last_glob(struct perf_evlist
> > *evlist,
> > struct perf_evsel *last = NULL;
> > int err;
> >
> > - if (evlist->nr_entries > 0)
> > + /*
> > + * Don't return when list_empty, give func a chance to report
> > + * error when it found last == NULL.
> > + *
> > + * So no need to WARN here, let *func do this.
> > + */
> > + if (!list_empty(&evlist->entries))
> > last = perf_evlist__last(evlist);
> >
> > do {
>
