Messages in this thread | | | Subject | Re: [PATCH v3] tools/perf/metricgroup: Fix printing event names of metric group with multiple events incase of overlapping events | From | kajoljain <> | Date | Tue, 11 Feb 2020 16:50:41 +0530 |
| |
On 2/10/20 5:41 PM, Jiri Olsa wrote: > On Thu, Feb 06, 2020 at 10:58:12AM -0800, Joe Perches wrote: >> On Thu, 2020-02-06 at 19:45 +0100, Jiri Olsa wrote: >>> On Fri, Jan 31, 2020 at 10:55:22AM +0530, Kajol Jain wrote: >>> >>> SNIP >>> >>>> ev->metric_leader = metric_events[i]; >>>> } >>>> + j++; >>>> } >>>> + ev = metric_events[i]; >>>> + evlist_used[ev->idx] = true; >>>> } >>>> >>>> return metric_events[0]; >>>> @@ -160,6 +161,9 @@ static int metricgroup__setup_events(struct list_head *groups, >>>> int ret = 0; >>>> struct egroup *eg; >>>> struct evsel *evsel; >>>> + bool evlist_used[perf_evlist->core.nr_entries]; >>>> + >>>> + memset(evlist_used, 0, perf_evlist->core.nr_entries); >>> >>> I know I posted this in the previous email, but are we sure bool >>> is always 1 byte? would sizeod(evlist_used) be safer?
Hi jiri, Yes you are right. We should use 'evlist_used' size itself.
>>> >>> other than that it looks ok >>> >>> Andi, you're ok with this? >> >> stack declarations of variable length arrays are not >> a good thing. >> >> https://lwn.net/Articles/749089/ >> >> and >> >> bool evlist_used[perf_evlist->core.nr_entries] = {};
I am planning to use calloc and free that memory later in function 'metricgroup__setup_events'. Something like this.
+ bool *evlist_used; + + evlist_used = (bool *)calloc(perf_evlist->core.nr_entries, + sizeof(bool)); + if (!evlist_used) { + ret = -ENOMEM; + break; + }
Please let me know if its looking fine.
Thanks, Kajol
> > hum, I think we already have few of them in perf ;-) > thanks for the link > > right, that initialization is of course much better, thanks > > jirka >
| |