Re: [PATCH perf/core v10 10/23] perf probe: Remove caches when --cache is given

[Masami Hiramatsu <mhiramat@kernel.org>]

On Thu, 9 Jun 2016 11:28:28 -0300
Arnaldo Carvalho de Melo <acme@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 <mhiramat@kernel.org>