On Thu, 9 Jun 2016 11:28:28 -0300
Arnaldo Carvalho de Melo <a...@kernel.org> wrote:
> > diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c
> > index a6d4a67..f687607 100644
> > --- a/tools/perf/util/probe-file.c
> > +++ b/tools/perf/util/probe-file.c
> > @@ -660,19 +660,39 @@ out:
> > return ret;
> > }
> >
> > +static bool probe_cache_entry__compare(struct probe_cache_entry *entry,
> > + struct strfilter *filter)
> > +{
> > + char buf[128], *ptr = entry->spev;
> > +
> > + if (entry->pev.event) {
> > + snprintf(buf, 128, "%s:%s", entry->pev.group, entry->pev.event);
> > + ptr = buf;
> > + }
> > + return strfilter__compare(filter, ptr);
> > +}
> > +
> > +int probe_cache__remove_entries(struct probe_cache *pcache,
> > + struct strfilter *filter)
> > +{
> > + struct probe_cache_entry *entry, *tmp;
> > +
> > + list_for_each_entry_safe(entry, tmp, &pcache->list, list) {
>
> so here you used the preferred idiom, i.e. using
> list_for_each_entry_safe(), once you stop doing the list removal at the
> list entry destructor, it gets the good old boring usual idiom, please
> do that.
OK, I see.
>
> Also please consider renaming perf_cache__remove_entries() to
> perf_cache__filter_purge(), as it doesn't simply remove entries, it
> purges them (that is, remove an entry and delete it), and only if the
> entry got filtered.
OK, such advice about naming helps me a lot :)
Thank you!
--
Masami Hiramatsu <mhira...@kernel.org>
