On Tue, May 23, 2017 at 12:48:53AM -0700, David Carrillo-Cisneros wrote:
SNIP
> +int perf_event__process_feature(struct perf_tool *tool,
> + union perf_event *event,
> + struct perf_session *session __maybe_unused)
> +{
> + struct feat_fd fd = { .fd = 0 };
> + struct feature_event *fe = (struct feature_event *)event;
> + int type = fe->header.type;
> + u64 feat = fe->header_id;
> +
> + if (type < 0 || type >= PERF_RECORD_HEADER_MAX) {
> + pr_warning("invalid record type %d\n", type);
> + return 0;
> + }
> + if (feat == HEADER_RESERVED)
> + return -1;
> +
> + if (feat > HEADER_LAST_FEATURE)
> + return 0;
I think we should warn in here
> +
> + if (!feat_ops[feat].process)
> + return 0;
> +
> + /*
> + * no print routine
> + */
superfluous comment
> + if (!feat_ops[feat].print)
> + return 0;
> +
> + fd.buf = (void *)fe->data;
> + fd.size = event->header.size - sizeof(event->header);
> + fd.ph = &session->header;
> +
> + if (!tool->show_feat_hdr)
> + return 0;
some of the features could provide data for processing,
should we call process unconditionaly and check this
just before calling print?
thanks,
jirka
