public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCHv2] tools lib traceevent: Handle dynamic array's element size properly
@ 2013-01-24 20:46 Jiri Olsa
  2013-01-24 21:13 ` Steven Rostedt
  2013-01-31 10:36 ` [tip:perf/core] tools lib traceevent: Handle dynamic array' s " tip-bot for Jiri Olsa
  0 siblings, 2 replies; 6+ messages in thread
From: Jiri Olsa @ 2013-01-24 20:46 UTC (permalink / raw)
  To: linux-kernel
  Cc: Jiri Olsa, Arnaldo Carvalho de Melo, Steven Rostedt,
	Corey Ashford, Frederic Weisbecker, Ingo Molnar, Namhyung Kim,
	Paul Mackerras, Peter Zijlstra

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 <jolsa@redhat.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@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)
+{
+	/* 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
 			field->elementsize = field->size;
 
-- 
1.7.11.7


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCHv2] tools lib traceevent: Handle dynamic array's element size properly
  2013-01-24 20:46 [PATCHv2] tools lib traceevent: Handle dynamic array's element size properly Jiri Olsa
@ 2013-01-24 21:13 ` Steven Rostedt
  2013-01-24 21:28   ` Arnaldo Carvalho de Melo
  2013-01-31 10:36 ` [tip:perf/core] tools lib traceevent: Handle dynamic array' s " tip-bot for Jiri Olsa
  1 sibling, 1 reply; 6+ messages in thread
From: Steven Rostedt @ 2013-01-24 21:13 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: linux-kernel, Arnaldo Carvalho de Melo, Corey Ashford,
	Frederic Weisbecker, Ingo Molnar, Namhyung Kim, Paul Mackerras,
	Peter Zijlstra

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 <jolsa@redhat.com>
> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
> Cc: Frederic Weisbecker <fweisbec@gmail.com>
> Cc: Ingo Molnar <mingo@elte.hu>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Peter Zijlstra <a.p.zijlstra@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;
>  



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCHv2] tools lib traceevent: Handle dynamic array's element size properly
  2013-01-24 21:13 ` Steven Rostedt
@ 2013-01-24 21:28   ` Arnaldo Carvalho de Melo
  2013-01-24 21:33     ` Steven Rostedt
  0 siblings, 1 reply; 6+ messages in thread
From: Arnaldo Carvalho de Melo @ 2013-01-24 21:28 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Jiri Olsa, linux-kernel, Corey Ashford, Frederic Weisbecker,
	Ingo Molnar, Namhyung Kim, Paul Mackerras, Peter Zijlstra

Em Thu, Jan 24, 2013 at 04:13:13PM -0500, Steven Rostedt escreveu:
> On Thu, 2013-01-24 at 21:46 +0100, Jiri Olsa wrote:
> >  
> > +static unsigned int type_size(char *name)
> 
> Sorry to nitpick, but please make that const char *name.

Agreed, no need to resubmit, I'll fix that up.

> The rest looks good.

Thanks, adding your Acked-by,

- Arnaldo

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCHv2] tools lib traceevent: Handle dynamic array's element size properly
  2013-01-24 21:28   ` Arnaldo Carvalho de Melo
@ 2013-01-24 21:33     ` Steven Rostedt
  2013-01-24 22:58       ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 6+ messages in thread
From: Steven Rostedt @ 2013-01-24 21:33 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Jiri Olsa, linux-kernel, Corey Ashford, Frederic Weisbecker,
	Ingo Molnar, Namhyung Kim, Paul Mackerras, Peter Zijlstra

On Thu, 2013-01-24 at 18:28 -0300, Arnaldo Carvalho de Melo wrote:
> Em Thu, Jan 24, 2013 at 04:13:13PM -0500, Steven Rostedt escreveu:
> > On Thu, 2013-01-24 at 21:46 +0100, Jiri Olsa wrote:
> > >  
> > > +static unsigned int type_size(char *name)
> > 
> > Sorry to nitpick, but please make that const char *name.
> 
> Agreed, no need to resubmit, I'll fix that up.

I'm sure Tom St Denis appreciates it.

> 
> > The rest looks good.
> 
> Thanks, adding your Acked-by,

Sure. But I probably should test it too ;-)

-- Steve



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCHv2] tools lib traceevent: Handle dynamic array's element size properly
  2013-01-24 21:33     ` Steven Rostedt
@ 2013-01-24 22:58       ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 6+ messages in thread
From: Arnaldo Carvalho de Melo @ 2013-01-24 22:58 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Jiri Olsa, linux-kernel, Corey Ashford, Frederic Weisbecker,
	Ingo Molnar, Namhyung Kim, Paul Mackerras, Peter Zijlstra

Em Thu, Jan 24, 2013 at 04:33:05PM -0500, Steven Rostedt escreveu:
> On Thu, 2013-01-24 at 18:28 -0300, Arnaldo Carvalho de Melo wrote:
> > Em Thu, Jan 24, 2013 at 04:13:13PM -0500, Steven Rostedt escreveu:
> > > On Thu, 2013-01-24 at 21:46 +0100, Jiri Olsa wrote:
> > > >  
> > > > +static unsigned int type_size(char *name)
> > > 
> > > Sorry to nitpick, but please make that const char *name.
> > 
> > Agreed, no need to resubmit, I'll fix that up.
> 
> I'm sure Tom St Denis appreciates it.
> 
> > 
> > > The rest looks good.
> > 
> > Thanks, adding your Acked-by,
> 
> Sure. But I probably should test it too ;-)

ROFL

- Arnaldo

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [tip:perf/core] tools lib traceevent: Handle dynamic array' s element size properly
  2013-01-24 20:46 [PATCHv2] tools lib traceevent: Handle dynamic array's element size properly Jiri Olsa
  2013-01-24 21:13 ` Steven Rostedt
@ 2013-01-31 10:36 ` tip-bot for Jiri Olsa
  1 sibling, 0 replies; 6+ messages in thread
From: tip-bot for Jiri Olsa @ 2013-01-31 10:36 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: acme, linux-kernel, paulus, hpa, mingo, a.p.zijlstra, namhyung,
	jolsa, fweisbec, rostedt, tglx, cjashfor, mingo

Commit-ID:  e23c1a5578cf32ed3a7ac9dde59a2de0a52ff812
Gitweb:     http://git.kernel.org/tip/e23c1a5578cf32ed3a7ac9dde59a2de0a52ff812
Author:     Jiri Olsa <jolsa@redhat.com>
AuthorDate: Thu, 24 Jan 2013 21:46:43 +0100
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Fri, 25 Jan 2013 12:49:28 -0300

tools lib traceevent: Handle dynamic array's element size properly

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 <jolsa@redhat.com>
Acked-by: Steven Rostedt <rostedt@goodmis.org>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Steven Rostedt <rostedt@goodmis.org>
Link: http://lkml.kernel.org/r/1359060403-32422-1-git-send-email-jolsa@redhat.com
[ committer note: Made a char pointer parameter const, as requested by Steven ]
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 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..82b0606 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(const char *name)
+{
+	/* This covers all FIELD_IS_STRING types. */
+	static struct {
+		const 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
 			field->elementsize = field->size;
 

^ permalink raw reply related	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2013-01-31 10:38 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-01-24 20:46 [PATCHv2] tools lib traceevent: Handle dynamic array's element size properly Jiri Olsa
2013-01-24 21:13 ` Steven Rostedt
2013-01-24 21:28   ` Arnaldo Carvalho de Melo
2013-01-24 21:33     ` Steven Rostedt
2013-01-24 22:58       ` Arnaldo Carvalho de Melo
2013-01-31 10:36 ` [tip:perf/core] tools lib traceevent: Handle dynamic array' s " tip-bot for Jiri Olsa

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox