Re: [PATCH] lib tools traceevent: Add back pevent assignment in__pevent_parse_format()

[Steven Rostedt]

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@xxxxxxxxxx>

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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/