On Thu, 2013-01-24 at 21:46 +0100, Jiri Olsa wrote:
> Fixing the dynamic array format field parsing.
>
> Currently the event_read_fields function could segfault while parsing
> dynamic array other than string type. The reason is the event->pevent
> does not need to be set and gets dereferenced unconditionaly.
>
> Also adding proper initialization of field->elementsize based on the
> parsed dynamic type.
>
> Signed-off-by: Jiri Olsa <jo...@redhat.com>
> Cc: Arnaldo Carvalho de Melo <a...@redhat.com>
> Cc: Steven Rostedt <rost...@goodmis.org>
> Cc: Corey Ashford <cjash...@linux.vnet.ibm.com>
> Cc: Frederic Weisbecker <fweis...@gmail.com>
> Cc: Ingo Molnar <mi...@elte.hu>
> Cc: Namhyung Kim <namhy...@kernel.org>
> Cc: Paul Mackerras <pau...@samba.org>
> Cc: Peter Zijlstra <a.p.zijls...@chello.nl>
> ---
> tools/lib/traceevent/event-parse.c | 39
> ++++++++++++++++++++++++++++++++++++--
> 1 file changed, 37 insertions(+), 2 deletions(-)
>
> diff --git a/tools/lib/traceevent/event-parse.c
> b/tools/lib/traceevent/event-parse.c
> index bb8b3db..2083462 100644
> --- a/tools/lib/traceevent/event-parse.c
> +++ b/tools/lib/traceevent/event-parse.c
> @@ -1223,6 +1223,34 @@ static int field_is_long(struct format_field *field)
> return 0;
> }
>
> +static unsigned int type_size(char *name)
Sorry to nitpick, but please make that const char *name.
> +{
> + /* This covers all FIELD_IS_STRING types. */
> + static struct {
> + char *type;
> + unsigned int size;
> + } table[] = {
> + { "u8", 1 },
> + { "u16", 2 },
> + { "u32", 4 },
> + { "u64", 8 },
> + { "s8", 1 },
> + { "s16", 2 },
> + { "s32", 4 },
> + { "s64", 8 },
> + { "char", 1 },
> + { },
> + };
> + int i;
> +
> + for (i = 0; table[i].type; i++) {
> + if (!strcmp(table[i].type, name))
> + return table[i].size;
> + }
> +
> + return 0;
> +}
> +
> static int event_read_fields(struct event_format *event, struct format_field
> **fields)
> {
> struct format_field *field = NULL;
> @@ -1232,6 +1260,8 @@ static int event_read_fields(struct event_format
> *event, struct format_field **f
> int count = 0;
>
> do {
> + unsigned int size_dynamic = 0;
> +
> type = read_token(&token);
> if (type == EVENT_NEWLINE) {
> free_token(token);
> @@ -1390,6 +1420,7 @@ static int event_read_fields(struct event_format
> *event, struct format_field **f
> field->type = new_type;
> strcat(field->type, " ");
> strcat(field->type, field->name);
> + size_dynamic = type_size(field->name);
> free_token(field->name);
> strcat(field->type, brackets);
> field->name = token;
> @@ -1478,10 +1509,14 @@ static int event_read_fields(struct event_format
> *event, struct format_field **f
> if (field->flags & FIELD_IS_ARRAY) {
> if (field->arraylen)
> field->elementsize = field->size /
> field->arraylen;
> + else if (field->flags & FIELD_IS_DYNAMIC)
> + field->elementsize = size_dynamic;
> else if (field->flags & FIELD_IS_STRING)
> field->elementsize = 1;
> - else
> - field->elementsize = event->pevent->long_size;
> + else if (field->flags & FIELD_IS_LONG)
> + field->elementsize = event->pevent ?
> + event->pevent->long_size :
> + sizeof(long);
> } else
The rest looks good.
-- Steve
> field->elementsize = field->size;
>
--
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/
