Messages in this thread | | | From | Namhyung Kim <> | Subject | Re: [PATCH 5/9] perf util: Handle failure case in trace_report() | Date | Wed, 20 Mar 2013 10:12:41 +0900 |
| |
On Tue, 19 Mar 2013 10:47:53 -0400, Steven Rostedt wrote: > On Tue, 2013-03-19 at 17:53 +0900, Namhyung Kim wrote: > >> @@ -3100,7 +3105,7 @@ int perf_event__process_tracing_data(union perf_event *event, >> } >> } >> >> - if (size_read + padding != size) { >> + if (size_read + padding != size || session->pevent == NULL) { >> pr_err("%s: tracing data size mismatch", __func__); > > This is a strange error to give when pevent is NULL. Could have been a > malloc failure.
Hmm.. right. I was just too lazy. ;)
Will break NULL check and print "failed to process tracing data" as we set it to NULL if something broken.
> > >> return -1; >> } >> diff --git a/tools/perf/util/trace-event-read.c b/tools/perf/util/trace-event-read.c >> index 7cb24635adf2..129362b97ca5 100644 >> --- a/tools/perf/util/trace-event-read.c >> +++ b/tools/perf/util/trace-event-read.c >> @@ -293,7 +293,10 @@ ssize_t trace_report(int fd, struct pevent **ppevent, bool __repipe) >> int show_version = 0; >> int show_funcs = 0; >> int show_printk = 0; >> - ssize_t size; >> + ssize_t size = -1; >> + struct pevent *pevent; >> + >> + *ppevent = NULL; >> >> calc_data_size = 1; >> repipe = __repipe; >> @@ -317,34 +320,38 @@ ssize_t trace_report(int fd, struct pevent **ppevent, bool __repipe) >> file_bigendian = buf[0]; >> host_bigendian = bigendian(); >> >> - *ppevent = read_trace_init(file_bigendian, host_bigendian); >> - if (*ppevent == NULL) >> - die("read_trace_init failed"); >> + pevent = read_trace_init(file_bigendian, host_bigendian); >> + if (pevent == NULL) { > > Shouldn't we still set *ppevent to NULL? Or will the user now need to > make sure that its pevent is NULL and always check for error?
I thought it's impossible to check return value for error since we don't know how large a tracing data is, so I decided to init *ppevent to NULL at the beginning and set it to a valid pevent right before return. Thus user need to check NULL before using it.
In the above case of perf_event__process_tracing_data(), it's a pipe mode and we can know its size since it's saved in a temp file.
Thanks, Namhyung
| |