Messages in this thread | | | Subject | Re: [PATCH] lib tools traceevent: Add back pevent assignment in __pevent_parse_format() | From | Steven Rostedt <> | Date | Mon, 01 Oct 2012 10:02:13 -0400 |
| |
On Mon, 2012-10-01 at 22:42 +0900, Namhyung Kim wrote: > 2012-09-28 (금), 22:20 -0400, Steven Rostedt: > > Even though with the change of commit commit 2b29175 "tools lib traceevent: > > Carve out events format parsing routine", allowed __pevent_parse_format() to > > parse an event without the need of a pevent handler, the event still needs to > > assign the pevent handed to it. > > > > There's no problem with assigning it if the pevent is NULL, as the event->pevent > > would be NULL without the assignment. But function parsing handlers may be > > assigned to the pevent handler to help in parsing the event. If there's no > > pevent then there would not be any function handlers, but if the pevent > > isn't assigned first before parsing the event, it wont honor the function > > handlers that were assigned. > > > > Worse yet, the current code crashes if an event has a function that it tries > > to parse. For example: > > > > # perf record -e scsi:scsi_dispatch_cmd_timeout > > Segmentation fault (core dumped) > > > > This happens because the scsi_dispatch_cmd_timeout event format has the following: > > > > __print_hex(__get_dynamic_array(cmnd), REC->cmd_len) > > > > which hasn't been defined by the pevent code. > > ??? We have both of __print_hex() and __get_dynamic_array() handler, > please see process_function. In my case, the offending function was > "scsi_trace_parse_cdb". >
Ah, you're right. I'll update the commit log. I missed that function. It was late when I wrote this and should have look harder :-p
> Other than that, looks good to me. > > Reviewed-by: Namhyung Kim <namhyung@kernel.org>
Thanks,
Will submit with your tag and a changed log.
-- Steve
-- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |