Hi Namhyung, On 2020/9/17 13:19, Namhyung Kim wrote: > Hello, > > On Thu, Sep 17, 2020 at 11:45 AM Wei Li <[email protected]> wrote: >> >> Since we have introduced map_for_each_event() to walk the 'pmu_events_map', >> clean up metricgroup__print() and metricgroup__has_metric() with it. >> >> Signed-off-by: Wei Li <[email protected]> > > Acked-by: Namhyung Kim <[email protected]> > > A nit-pick below: > > >> --- >> tools/perf/util/metricgroup.c | 33 +++++++++++++-------------------- >> 1 file changed, 13 insertions(+), 20 deletions(-) >> >> diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c >> index 8831b964288f..3734cbb2c456 100644 >> --- a/tools/perf/util/metricgroup.c >> +++ b/tools/perf/util/metricgroup.c >> @@ -26,6 +26,17 @@ >> #include "util.h" >> #include <asm/bug.h> >> >> +#define map_for_each_event(__pe, __idx, __map) \ >> + for (__idx = 0, __pe = &__map->table[__idx]; \ >> + __pe->name || __pe->metric_group || __pe->metric_name; \ >> + __pe = &__map->table[++__idx]) >> + >> +#define map_for_each_metric(__pe, __idx, __map, __metric) \ >> + map_for_each_event(__pe, __idx, __map) \ >> + if (__pe->metric_expr && \ >> + (match_metric(__pe->metric_group, __metric) || \ >> + match_metric(__pe->metric_name, __metric))) >> + > > You may consider adding a declaration of match_metric() here. > Right now, all users are below the function so it's ok but > having the macro here can enable future addition above IMHO. >
Thanks for spotting that. I'd like to move them just below match_metric() for now, when we really need to use them somewhere else, we may move them to header file then. Thanks, Wei
