On 23/10/13 09:15, David Ahern wrote: > On 10/22/13 8:34 AM, Adrian Hunter wrote: >> +static int process_attr(struct perf_tool *tool, union perf_event *event, >> + struct perf_evlist **pevlist) >> +{ >> + struct perf_script *scr = container_of(tool, struct perf_script, tool); >> + struct perf_evlist *evlist; >> + struct perf_evsel *evsel, *pos; >> + int err; >> + >> + err = perf_event__process_attr(tool, event, pevlist); >> + if (err) >> + return err; >> + >> + evlist = *pevlist; >> + evsel = perf_evlist__last(*pevlist); > > This assumes new entries are added to the end of evlist in > perf_event__process_attr. Would be better to change it to return the newly > created evsel so you don't need to look it up after adding it.
perf_event__process_attr() must not reorder the attributes, it would misrepresent the way they were recorded. > >> + >> + if (evsel->attr.type >= PERF_TYPE_MAX) >> + return 0; >> + >> + list_for_each_entry(pos, &evlist->entries, node) { >> + if (pos->attr.type == evsel->attr.type && pos != evsel) >> + return 0; >> + } > > What's the point of this loop? Each type is checked once - see perf_session__check_output_opt() -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/