[PATCH 0/7] trace-cmd: Small fixes and cleanups on libparsevent

[PATCH 0/7] trace-cmd: Small fixes and cleanups on libparsevent

[Namhyung Kim]

Hi, all

This is a couple of small changes that I found during the flight back to
Korea :). While I'm not finished with reading the whole code, I send what
I have now in the hope that it'd help Frederic's unification work somewhat.
More might come later as I read the remaining parts.

The patch 01 is a workaround to build trace-cmd without installing swig
since I don't have an internet access at that time, so it's independent
to others and can go separately.

Thanks.


Namhyung Kim (7):
  trace-cmd: Add NO_PYTHON option for Makefile
  parse-events: Fix printk_cmp()
  parse-events: Introduce extend_token()
  parse-events: Handle strdup failure cases
  parse-events: Fix a possible memory leak
  parse-events: Handle realloc() failure path
  parse-events: Fix a possibly wrong memory dereference

 Makefile       |    2 +
 parse-events.c |  168 +++++++++++++++++++++++++++++++++++++++----------------
 2 files changed, 121 insertions(+), 49 deletions(-)

-- 
1.7.7.6

[PATCH 1/7] trace-cmd: Add NO_PYTHON option for Makefile

[Namhyung Kim]

If someone do not want to build python plugins, [s]he can
define the option to disable python support:

  $ make NO_PYTHON=1

Signed-off-by: Namhyung Kim <namhyung.kim@lge.com>
---
 Makefile |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/Makefile b/Makefile
index edf3402..54dbba4 100644
--- a/Makefile
+++ b/Makefile
@@ -76,6 +76,7 @@ ifndef VERBOSE
   VERBOSE = 0
 endif
 
+ifndef NO_PYTHON
 PYTHON		:= ctracecmd.so
 PYTHON_GUI	:= ctracecmd.so ctracecmdgui.so
 
@@ -86,6 +87,7 @@ ifeq ($(shell sh -c "python-config --includes > /dev/null 2>&1 && echo y"), y)
 	PYTHON_SO_INSTALL := ctracecmd.install
 	PYTHON_PY_INSTALL := event-viewer.install tracecmd.install tracecmdgui.install
 endif
+endif # NO_PYTHON
 
 # $(call test-build, snippet, ret) -> ret if snippet compiles
 #                                  -> empty otherwise
-- 
1.7.7.6

[PATCH 2/7] parse-events: Fix printk_cmp()

[Namhyung Kim]

The printk_cmp function should use printk_map instead of func_map.
Also rename the variables for consistency.

Signed-off-by: Namhyung Kim <namhyung.kim@lge.com>
---
 parse-events.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/parse-events.c b/parse-events.c
index 5214771..f90af1a 100644
--- a/parse-events.c
+++ b/parse-events.c
@@ -507,12 +507,12 @@ struct printk_list {
 
 static int printk_cmp(const void *a, const void *b)
 {
-	const struct func_map *fa = a;
-	const struct func_map *fb = b;
+	const struct printk_map *pa = a;
+	const struct printk_map *pb = b;
 
-	if (fa->addr < fb->addr)
+	if (pa->addr < pb->addr)
 		return -1;
-	if (fa->addr > fb->addr)
+	if (pa->addr > pb->addr)
 		return 1;
 
 	return 0;
-- 
1.7.7.6

[PATCH 3/7] parse-events: Introduce extend_token()

[Namhyung Kim]

The __read_token() function has some duplicated code to handle
internal buffer overflow. Factor them out to new extend_token().

According to the man pages of realloc/free(3), they can handle
NULL pointer input so that it can be ended up to compact the code.
Also handle error path correctly.

Signed-off-by: Namhyung Kim <namhyung.kim@lge.com>
---
 parse-events.c |   48 ++++++++++++++++++++++--------------------------
 1 files changed, 22 insertions(+), 26 deletions(-)

diff --git a/parse-events.c b/parse-events.c
index f90af1a..0b1e40a 100644
--- a/parse-events.c
+++ b/parse-events.c
@@ -771,6 +771,23 @@ int pevent_peek_char(void)
 	return __peek_char();
 }
 
+static int extend_token(char **tok, char *buf, int size)
+{
+	char *newtok = realloc(*tok, size);
+	if (!newtok) {
+		free(*tok);
+		*tok = NULL;
+		return -1;
+	}
+
+	if (!*tok)
+		strcpy(newtok, buf);
+	else
+		strcat(newtok, buf);
+	*tok = newtok;
+	return 0;
+}
+
 static enum event_type force_token(const char *str, char **tok);
 
 static enum event_type __read_token(char **tok)
@@ -855,17 +872,10 @@ static enum event_type __read_token(char **tok)
 		do {
 			if (i == (BUFSIZ - 1)) {
 				buf[i] = 0;
-				if (*tok) {
-					*tok = realloc(*tok, tok_size + BUFSIZ);
-					if (!*tok)
-						return EVENT_NONE;
-					strcat(*tok, buf);
-				} else
-					*tok = strdup(buf);
+				tok_size += BUFSIZ;
 
-				if (!*tok)
+				if (extend_token(tok, buf, tok_size) < 0)
 					return EVENT_NONE;
-				tok_size += BUFSIZ;
 				i = 0;
 			}
 			last_ch = ch;
@@ -904,17 +914,10 @@ static enum event_type __read_token(char **tok)
 	while (get_type(__peek_char()) == type) {
 		if (i == (BUFSIZ - 1)) {
 			buf[i] = 0;
-			if (*tok) {
-				*tok = realloc(*tok, tok_size + BUFSIZ);
-				if (!*tok)
-					return EVENT_NONE;
-				strcat(*tok, buf);
-			} else
-				*tok = strdup(buf);
+			tok_size += BUFSIZ;
 
-			if (!*tok)
+			if (extend_token(tok, buf, tok_size) < 0)
 				return EVENT_NONE;
-			tok_size += BUFSIZ;
 			i = 0;
 		}
 		ch = __read_char();
@@ -923,14 +926,7 @@ static enum event_type __read_token(char **tok)
 
  out:
 	buf[i] = 0;
-	if (*tok) {
-		*tok = realloc(*tok, tok_size + i);
-		if (!*tok)
-			return EVENT_NONE;
-		strcat(*tok, buf);
-	} else
-		*tok = strdup(buf);
-	if (!*tok)
+	if (extend_token(tok, buf, tok_size + i + 1) < 0)
 		return EVENT_NONE;
 
 	if (type == EVENT_ITEM) {
-- 
1.7.7.6

[PATCH 4/7] parse-events: Handle strdup failure cases

[Namhyung Kim]

There were some places didn't check return value of the strdup
and had unneeded/duplicated checks. Fix it.

Signed-off-by: Namhyung Kim <namhyung.kim@lge.com>
---
 parse-events.c |   29 +++++++++++++++++++++++++++--
 1 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/parse-events.c b/parse-events.c
index 0b1e40a..773c928 100644
--- a/parse-events.c
+++ b/parse-events.c
@@ -463,8 +463,10 @@ int pevent_register_function(struct pevent *pevent, char *func,
 		item->mod = NULL;
 	item->addr = addr;
 
-	pevent->funclist = item;
+	if (!item->func || (mod && !item->mod))
+		die("malloc func");
 
+	pevent->funclist = item;
 	pevent->func_count++;
 
 	return 0;
@@ -579,10 +581,13 @@ int pevent_register_print_string(struct pevent *pevent, char *fmt,
 	item = malloc_or_die(sizeof(*item));
 
 	item->next = pevent->printklist;
-	pevent->printklist = item;
 	item->printk = strdup(fmt);
 	item->addr = addr;
 
+	if (!item->printk)
+		die("malloc fmt");
+
+	pevent->printklist = item;
 	pevent->printk_count++;
 
 	return 0;
@@ -2123,6 +2128,8 @@ process_fields(struct event_format *event, struct print_flag_sym **list, char **
 		if (value == NULL)
 			goto out_free;
 		field->value = strdup(value);
+		if (field->value == NULL)
+			goto out_free;
 
 		free_arg(arg);
 		arg = alloc_arg();
@@ -2136,6 +2143,8 @@ process_fields(struct event_format *event, struct print_flag_sym **list, char **
 		if (value == NULL)
 			goto out_free;
 		field->str = strdup(value);
+		if (field->str == NULL)
+			goto out_free;
 		free_arg(arg);
 		arg = NULL;
 
@@ -3338,6 +3347,9 @@ process_defined_func(struct trace_seq *s, void *data, int size,
 			string = malloc_or_die(sizeof(*string));
 			string->next = strings;
 			string->str = strdup(str.buffer);
+			if (!string->str)
+				die("malloc str");
+
 			strings = string;
 			trace_seq_destroy(&str);
 			break;
@@ -3475,6 +3487,8 @@ static struct print_arg *make_bprint_args(char *fmt, void *data, int size, struc
 				arg->next = NULL;
 				arg->type = PRINT_BSTRING;
 				arg->string.string = strdup(bptr);
+				if (!arg->string.string)
+					break;
 				bptr += strlen(bptr) + 1;
 				*next = arg;
 				next = &arg->next;
@@ -4542,6 +4556,8 @@ int pevent_parse_event(struct pevent *pevent,
 		die("failed to read event id");
 
 	event->system = strdup(sys);
+	if (!event->system)
+		die("failed to allocate system");
 
 	/* Add pevent to event so that it can be referenced */
 	event->pevent = pevent;
@@ -4583,6 +4599,10 @@ int pevent_parse_event(struct pevent *pevent,
 			list = &arg->next;
 			arg->type = PRINT_FIELD;
 			arg->field.name = strdup(field->name);
+			if (!arg->field.name) {
+				do_warning("failed to allocate field name");
+				goto event_failed;
+			}
 			arg->field.field = field;
 		}
 		return 0;
@@ -4926,6 +4946,11 @@ int pevent_register_event_handler(struct pevent *pevent,
 	if (sys_name)
 		handle->sys_name = strdup(sys_name);
 
+	if ((event_name && !handle->event_name) ||
+	    (sys_name && !handle->sys_name)) {
+		die("Failed to allocate event/sys name");
+	}
+
 	handle->func = func;
 	handle->next = pevent->handlers;
 	pevent->handlers = handle;
-- 
1.7.7.6

[PATCH 5/7] parse-events: Fix a possible memory leak

[Namhyung Kim]

If event_read_fields failed in the middle, each member of
struct format_field should be freed also.

Signed-off-by: Namhyung Kim <namhyung.kim@lge.com>
---
 parse-events.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/parse-events.c b/parse-events.c
index 773c928..710a9e8 100644
--- a/parse-events.c
+++ b/parse-events.c
@@ -1431,8 +1431,11 @@ static int event_read_fields(struct event_format *event, struct format_field **f
 fail:
 	free_token(token);
 fail_expect:
-	if (field)
+	if (field) {
+		free(field->type);
+		free(field->name);
 		free(field);
+	}
 	return -1;
 }
 
-- 
1.7.7.6

[PATCH 6/7] parse-events: Handle realloc() failure path

[Namhyung Kim]

The realloc can fail so that we should handle it properly.

Signed-off-by: Namhyung Kim <namhyung.kim@lge.com>
---
 parse-events.c |   76 ++++++++++++++++++++++++++++++++++++++++++++------------
 1 files changed, 60 insertions(+), 16 deletions(-)

diff --git a/parse-events.c b/parse-events.c
index 710a9e8..e789275 100644
--- a/parse-events.c
+++ b/parse-events.c
@@ -1252,9 +1252,15 @@ static int event_read_fields(struct event_format *event, struct format_field **f
 					field->flags |= FIELD_IS_POINTER;
 
 				if (field->type) {
-					field->type = realloc(field->type,
-							      strlen(field->type) +
-							      strlen(last_token) + 2);
+					char *new_type;
+					new_type = realloc(field->type,
+							   strlen(field->type) +
+							   strlen(last_token) + 2);
+					if (!new_type) {
+						free(last_token);
+						goto fail;
+					}
+					field->type = new_type;
 					strcat(field->type, " ");
 					strcat(field->type, last_token);
 					free(last_token);
@@ -1279,6 +1285,7 @@ static int event_read_fields(struct event_format *event, struct format_field **f
 		if (strcmp(token, "[") == 0) {
 			enum event_type last_type = type;
 			char *brackets = token;
+			char *new_brackets;
 			int len;
 
 			field->flags |= FIELD_IS_ARRAY;
@@ -1298,9 +1305,14 @@ static int event_read_fields(struct event_format *event, struct format_field **f
 					len = 1;
 				last_type = type;
 
-				brackets = realloc(brackets,
-						   strlen(brackets) +
-						   strlen(token) + len);
+				new_brackets = realloc(brackets,
+						       strlen(brackets) +
+						       strlen(token) + len);
+				if (!new_brackets) {
+					free(brackets);
+					goto fail;
+				}
+				brackets = new_brackets;
 				if (len == 2)
 					strcat(brackets, " ");
 				strcat(brackets, token);
@@ -1316,7 +1328,12 @@ static int event_read_fields(struct event_format *event, struct format_field **f
 
 			free_token(token);
 
-			brackets = realloc(brackets, strlen(brackets) + 2);
+			new_brackets = realloc(brackets, strlen(brackets) + 2);
+			if (!new_brackets) {
+				free(brackets);
+				goto fail;
+			}
+			brackets = new_brackets;
 			strcat(brackets, "]");
 
 			/* add brackets to type */
@@ -1327,10 +1344,16 @@ static int event_read_fields(struct event_format *event, struct format_field **f
 			 * the format: type [] item;
 			 */
 			if (type == EVENT_ITEM) {
-				field->type = realloc(field->type,
-						      strlen(field->type) +
-						      strlen(field->name) +
-						      strlen(brackets) + 2);
+				char *new_type;
+				new_type = realloc(field->type,
+						   strlen(field->type) +
+						   strlen(field->name) +
+						   strlen(brackets) + 2);
+				if (!new_type) {
+					free(brackets);
+					goto fail;
+				}
+				field->type = new_type;
 				strcat(field->type, " ");
 				strcat(field->type, field->name);
 				free_token(field->name);
@@ -1338,9 +1361,15 @@ static int event_read_fields(struct event_format *event, struct format_field **f
 				field->name = token;
 				type = read_token(&token);
 			} else {
-				field->type = realloc(field->type,
-						      strlen(field->type) +
-						      strlen(brackets) + 1);
+				char *new_type;
+				new_type = realloc(field->type,
+						   strlen(field->type) +
+						   strlen(brackets) + 1);
+				if (!new_type) {
+					free(brackets);
+					goto fail;
+				}
+				field->type = new_type;
 				strcat(field->type, brackets);
 			}
 			free(brackets);
@@ -1721,10 +1750,16 @@ process_op(struct event_format *event, struct print_arg *arg, char **tok)
 		/* could just be a type pointer */
 		if ((strcmp(arg->op.op, "*") == 0) &&
 		    type == EVENT_DELIM && (strcmp(token, ")") == 0)) {
+			char *new_atom;
+
 			if (left->type != PRINT_ATOM)
 				die("bad pointer type");
-			left->atom.atom = realloc(left->atom.atom,
+			new_atom = realloc(left->atom.atom,
 					    strlen(left->atom.atom) + 3);
+			if (!new_atom)
+				goto out_free;
+
+			left->atom.atom = new_atom;
 			strcat(left->atom.atom, " *");
 			free(arg->op.op);
 			*arg = *left;
@@ -2527,7 +2562,16 @@ process_arg_token(struct event_format *event, struct print_arg *arg,
 		}
 		/* atoms can be more than one token long */
 		while (type == EVENT_ITEM) {
-			atom = realloc(atom, strlen(atom) + strlen(token) + 2);
+			char *new_atom;
+			new_atom = realloc(atom,
+					   strlen(atom) + strlen(token) + 2);
+			if (!new_atom) {
+				free(atom);
+				*tok = NULL;
+				free_token(token);
+				return EVENT_ERROR;
+			}
+			atom = new_atom;
 			strcat(atom, " ");
 			strcat(atom, token);
 			free_token(token);
-- 
1.7.7.6

[PATCH 7/7] parse-events: Fix a possibly wrong memory dereference

[Namhyung Kim]

If set_op_prio() failed, the token will be freed at out_free,
then arg->op.op would turn out to be a dangle pointer. After
returning EVENT_ERROR from process_op(), free_arg() will be
called and then it will finally see the dangling pointer.

Signed-off-by: Namhyung Kim <namhyung.kim@lge.com>
---
 parse-events.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/parse-events.c b/parse-events.c
index e789275..bd078ba 100644
--- a/parse-events.c
+++ b/parse-events.c
@@ -1741,6 +1741,8 @@ process_op(struct event_format *event, struct print_arg *arg, char **tok)
 
 		if (set_op_prio(arg) == -1) {
 			event->flags |= EVENT_FL_FAILED;
+			/* arg->op.op (= token) will be freed at out_free */
+			arg->op.op = NULL;
 			goto out_free;
 		}
 
-- 
1.7.7.6

Re: [PATCH 0/7] trace-cmd: Small fixes and cleanups on libparsevent

[Frederic Weisbecker]

2012/4/9 Namhyung Kim <namhyung.kim@lge.com>:
> Hi, all
>
> This is a couple of small changes that I found during the flight back to
> Korea :). While I'm not finished with reading the whole code, I send what
> I have now in the hope that it'd help Frederic's unification work somewhat.
> More might come later as I read the remaining parts.
>
> The patch 01 is a workaround to build trace-cmd without installing swig
> since I don't have an internet access at that time, so it's independent
> to others and can go separately.
>
> Thanks.
>
>
> Namhyung Kim (7):
>  trace-cmd: Add NO_PYTHON option for Makefile
>  parse-events: Fix printk_cmp()
>  parse-events: Introduce extend_token()
>  parse-events: Handle strdup failure cases
>  parse-events: Fix a possible memory leak
>  parse-events: Handle realloc() failure path
>  parse-events: Fix a possibly wrong memory dereference
>
>  Makefile       |    2 +
>  parse-events.c |  168 +++++++++++++++++++++++++++++++++++++++----------------
>  2 files changed, 121 insertions(+), 49 deletions(-)

Nice! I guess this should probably go into trace-cmd tree and then we
backport later to the lib?
Or I can carry them myself in the lib.

> --
> 1.7.7.6
>

Re: [PATCH 0/7] trace-cmd: Small fixes and cleanups on libparsevent

[Namhyung Kim]

Hi, Frederic

On Mon, 9 Apr 2012 19:22:46 +0200, Frederic Weisbecker wrote:
> 2012/4/9 Namhyung Kim <namhyung.kim@lge.com>:
>> Hi, all
>>
>> This is a couple of small changes that I found during the flight back to
>> Korea :). While I'm not finished with reading the whole code, I send what
>> I have now in the hope that it'd help Frederic's unification work somewhat.
>> More might come later as I read the remaining parts.
>>
>> The patch 01 is a workaround to build trace-cmd without installing swig
>> since I don't have an internet access at that time, so it's independent
>> to others and can go separately.
>>
> Nice! I guess this should probably go into trace-cmd tree and then we
> backport later to the lib?
> Or I can carry them myself in the lib.
>

Either way is fine to me.

Thanks,
Namhyung

Re: [PATCH 1/7] trace-cmd: Add NO_PYTHON option for Makefile

[Steven Rostedt]

On Mon, 2012-04-09 at 11:54 +0900, Namhyung Kim wrote:
> If someone do not want to build python plugins, [s]he can
> define the option to disable python support:
> 
>   $ make NO_PYTHON=1

Thanks, I should also add a way to check if this will compile, or if it
fails to just print a message to build with this.

-- Steve

> 
> Signed-off-by: Namhyung Kim <namhyung.kim@lge.com>
> ---
>  Makefile |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index edf3402..54dbba4 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -76,6 +76,7 @@ ifndef VERBOSE
>    VERBOSE = 0
>  endif
>  
> +ifndef NO_PYTHON
>  PYTHON		:= ctracecmd.so
>  PYTHON_GUI	:= ctracecmd.so ctracecmdgui.so
>  
> @@ -86,6 +87,7 @@ ifeq ($(shell sh -c "python-config --includes > /dev/null 2>&1 && echo y"), y)
>  	PYTHON_SO_INSTALL := ctracecmd.install
>  	PYTHON_PY_INSTALL := event-viewer.install tracecmd.install tracecmdgui.install
>  endif
> +endif # NO_PYTHON
>  
>  # $(call test-build, snippet, ret) -> ret if snippet compiles
>  #                                  -> empty otherwise

Re: [PATCH 2/7] parse-events: Fix printk_cmp()

[Steven Rostedt]

On Mon, 2012-04-09 at 11:54 +0900, Namhyung Kim wrote:
> The printk_cmp function should use printk_map instead of func_map.
> Also rename the variables for consistency.
> 
> Signed-off-by: Namhyung Kim <namhyung.kim@lge.com>
> ---
>  parse-events.c |    8 ++++----
>  1 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/parse-events.c b/parse-events.c
> index 5214771..f90af1a 100644
> --- a/parse-events.c
> +++ b/parse-events.c
> @@ -507,12 +507,12 @@ struct printk_list {
>  
>  static int printk_cmp(const void *a, const void *b)
>  {
> -	const struct func_map *fa = a;
> -	const struct func_map *fb = b;
> +	const struct printk_map *pa = a;
> +	const struct printk_map *pb = b;
>  
> -	if (fa->addr < fb->addr)
> +	if (pa->addr < pb->addr)
>  		return -1;
> -	if (fa->addr > fb->addr)
> +	if (pa->addr > pb->addr)

Ouch! This worked only because the first item in both func_map and
printk_map happened to be addr (of the same size).

-- Steve

>  		return 1;
>  
>  	return 0;

Re: [PATCH 4/7] parse-events: Handle strdup failure cases

[Steven Rostedt]

On Mon, 2012-04-09 at 11:54 +0900, Namhyung Kim wrote:
> There were some places didn't check return value of the strdup
> and had unneeded/duplicated checks. Fix it.
> 
> Signed-off-by: Namhyung Kim <namhyung.kim@lge.com>
> ---
>  parse-events.c |   29 +++++++++++++++++++++++++++--
>  1 files changed, 27 insertions(+), 2 deletions(-)
> 
> diff --git a/parse-events.c b/parse-events.c
> index 0b1e40a..773c928 100644
> --- a/parse-events.c
> +++ b/parse-events.c
> @@ -463,8 +463,10 @@ int pevent_register_function(struct pevent *pevent, char *func,
>  		item->mod = NULL;
>  	item->addr = addr;
>  
> -	pevent->funclist = item;
> +	if (!item->func || (mod && !item->mod))
> +		die("malloc func");
>  

I just added this patch, but we need to get rid of all the "die"
functions and do proper freeing and error notifications to make this a
real library.

Thanks,

-- Steve

Re: [PATCH 4/7] parse-events: Handle strdup failure cases

[Namhyung Kim]

Hi,

On Mon, 23 Apr 2012 11:51:14 -0400, Steven Rostedt wrote:
> On Mon, 2012-04-09 at 11:54 +0900, Namhyung Kim wrote:
>> There were some places didn't check return value of the strdup
>> and had unneeded/duplicated checks. Fix it.
>> 
>> Signed-off-by: Namhyung Kim <namhyung.kim@lge.com>
>> ---
>>  parse-events.c |   29 +++++++++++++++++++++++++++--
>>  1 files changed, 27 insertions(+), 2 deletions(-)
>> 
>> diff --git a/parse-events.c b/parse-events.c
>> index 0b1e40a..773c928 100644
>> --- a/parse-events.c
>> +++ b/parse-events.c
>> @@ -463,8 +463,10 @@ int pevent_register_function(struct pevent *pevent, char *func,
>>  		item->mod = NULL;
>>  	item->addr = addr;
>>  
>> -	pevent->funclist = item;
>> +	if (!item->func || (mod && !item->mod))
>> +		die("malloc func");
>>  
>
> I just added this patch, but we need to get rid of all the "die"
> functions and do proper freeing and error notifications to make this a
> real library.
>

Agreed. I'm planning to dive into it soonish..

Thanks,
Namhyung

Re: [PATCH 4/7] parse-events: Handle strdup failure cases

[Steven Rostedt]

On Tue, 2012-04-24 at 09:35 +0900, Namhyung Kim wrote:

> > I just added this patch, but we need to get rid of all the "die"
> > functions and do proper freeing and error notifications to make this a
> > real library.
> >
> 
> Agreed. I'm planning to dive into it soonish..

Great! One more thing for me to take of my TODO list :-)

-- Steve

[tip:perf/core] tools lib traceevent: Fix printk_cmp()

[tip-bot for Namhyung Kim]

Commit-ID:  0fc45ef5202a34b5862ca246740e6ab50bc3e3e1
Gitweb:     http://git.kernel.org/tip/0fc45ef5202a34b5862ca246740e6ab50bc3e3e1
Author:     Namhyung Kim <namhyung.kim@lge.com>
AuthorDate: Mon, 9 Apr 2012 11:54:29 +0900
Committer:  Namhyung Kim <namhyung@kernel.org>
CommitDate: Wed, 4 Jul 2012 13:40:30 +0900

tools lib traceevent: Fix printk_cmp()

The printk_cmp function should use printk_map instead of func_map.
Also rename the variables for consistency.

Signed-off-by: Namhyung Kim <namhyung.kim@lge.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Arnaldo Carvalho de Melo <acme@infradead.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: David Ahern <dsahern@gmail.com>
Link: http://lkml.kernel.org/r/1333940074-19052-3-git-send-email-namhyung.kim@lge.com
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/lib/traceevent/event-parse.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
index ddee5a8..7815b8d 100644
--- a/tools/lib/traceevent/event-parse.c
+++ b/tools/lib/traceevent/event-parse.c
@@ -511,12 +511,12 @@ struct printk_list {
 
 static int printk_cmp(const void *a, const void *b)
 {
-	const struct func_map *fa = a;
-	const struct func_map *fb = b;
+	const struct printk_map *pa = a;
+	const struct printk_map *pb = b;
 
-	if (fa->addr < fb->addr)
+	if (pa->addr < pb->addr)
 		return -1;
-	if (fa->addr > fb->addr)
+	if (pa->addr > pb->addr)
 		return 1;
 
 	return 0;

[tip:perf/core] tools lib traceevent: Introduce extend_token()

[tip-bot for Namhyung Kim]

Commit-ID:  deba3fb26fd1ed3235b00dccced9784a7f76ec3c
Gitweb:     http://git.kernel.org/tip/deba3fb26fd1ed3235b00dccced9784a7f76ec3c
Author:     Namhyung Kim <namhyung.kim@lge.com>
AuthorDate: Mon, 9 Apr 2012 11:54:30 +0900
Committer:  Namhyung Kim <namhyung@kernel.org>
CommitDate: Wed, 4 Jul 2012 13:40:31 +0900

tools lib traceevent: Introduce extend_token()

The __read_token() function has some duplicated code to handle
internal buffer overflow. Factor them out to new extend_token().

According to the man pages of realloc/free(3), they can handle NULL
pointer input so that it can be ended up to compact the code.  Also
handle error path correctly.

Signed-off-by: Namhyung Kim <namhyung.kim@lge.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Arnaldo Carvalho de Melo <acme@infradead.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: David Ahern <dsahern@gmail.com>
Link: http://lkml.kernel.org/r/1333940074-19052-4-git-send-email-namhyung.kim@lge.com
[rostedt@goodmis.org: added some extra whitespace]
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/lib/traceevent/event-parse.c |   52 +++++++++++++++++------------------
 1 files changed, 25 insertions(+), 27 deletions(-)

diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
index 7815b8d..768fab5 100644
--- a/tools/lib/traceevent/event-parse.c
+++ b/tools/lib/traceevent/event-parse.c
@@ -781,6 +781,25 @@ int pevent_peek_char(void)
 	return __peek_char();
 }
 
+static int extend_token(char **tok, char *buf, int size)
+{
+	char *newtok = realloc(*tok, size);
+
+	if (!newtok) {
+		free(*tok);
+		*tok = NULL;
+		return -1;
+	}
+
+	if (!*tok)
+		strcpy(newtok, buf);
+	else
+		strcat(newtok, buf);
+	*tok = newtok;
+
+	return 0;
+}
+
 static enum event_type force_token(const char *str, char **tok);
 
 static enum event_type __read_token(char **tok)
@@ -865,17 +884,10 @@ static enum event_type __read_token(char **tok)
 		do {
 			if (i == (BUFSIZ - 1)) {
 				buf[i] = 0;
-				if (*tok) {
-					*tok = realloc(*tok, tok_size + BUFSIZ);
-					if (!*tok)
-						return EVENT_NONE;
-					strcat(*tok, buf);
-				} else
-					*tok = strdup(buf);
+				tok_size += BUFSIZ;
 
-				if (!*tok)
+				if (extend_token(tok, buf, tok_size) < 0)
 					return EVENT_NONE;
-				tok_size += BUFSIZ;
 				i = 0;
 			}
 			last_ch = ch;
@@ -914,17 +926,10 @@ static enum event_type __read_token(char **tok)
 	while (get_type(__peek_char()) == type) {
 		if (i == (BUFSIZ - 1)) {
 			buf[i] = 0;
-			if (*tok) {
-				*tok = realloc(*tok, tok_size + BUFSIZ);
-				if (!*tok)
-					return EVENT_NONE;
-				strcat(*tok, buf);
-			} else
-				*tok = strdup(buf);
+			tok_size += BUFSIZ;
 
-			if (!*tok)
+			if (extend_token(tok, buf, tok_size) < 0)
 				return EVENT_NONE;
-			tok_size += BUFSIZ;
 			i = 0;
 		}
 		ch = __read_char();
@@ -933,14 +938,7 @@ static enum event_type __read_token(char **tok)
 
  out:
 	buf[i] = 0;
-	if (*tok) {
-		*tok = realloc(*tok, tok_size + i);
-		if (!*tok)
-			return EVENT_NONE;
-		strcat(*tok, buf);
-	} else
-		*tok = strdup(buf);
-	if (!*tok)
+	if (extend_token(tok, buf, tok_size + i + 1) < 0)
 		return EVENT_NONE;
 
 	if (type == EVENT_ITEM) {

[tip:perf/core] tools lib traceevent: Handle strdup failure cases

[tip-bot for Namhyung Kim]

Commit-ID:  ca63858e9eb0ce495031c4ab5291874835cb43cf
Gitweb:     http://git.kernel.org/tip/ca63858e9eb0ce495031c4ab5291874835cb43cf
Author:     Namhyung Kim <namhyung.kim@lge.com>
AuthorDate: Mon, 9 Apr 2012 11:54:31 +0900
Committer:  Namhyung Kim <namhyung@kernel.org>
CommitDate: Wed, 4 Jul 2012 13:40:31 +0900

tools lib traceevent: Handle strdup failure cases

There were some places didn't check return value of the strdup and had
unneeded/duplicated checks.  Fix it.

Signed-off-by: Namhyung Kim <namhyung.kim@lge.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Arnaldo Carvalho de Melo <acme@infradead.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: David Ahern <dsahern@gmail.com>
Link: http://lkml.kernel.org/r/1333940074-19052-5-git-send-email-namhyung.kim@lge.com
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/lib/traceevent/event-parse.c |   29 +++++++++++++++++++++++++++--
 1 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
index 768fab5..cd334d5 100644
--- a/tools/lib/traceevent/event-parse.c
+++ b/tools/lib/traceevent/event-parse.c
@@ -467,8 +467,10 @@ int pevent_register_function(struct pevent *pevent, char *func,
 		item->mod = NULL;
 	item->addr = addr;
 
-	pevent->funclist = item;
+	if (!item->func || (mod && !item->mod))
+		die("malloc func");
 
+	pevent->funclist = item;
 	pevent->func_count++;
 
 	return 0;
@@ -583,10 +585,13 @@ int pevent_register_print_string(struct pevent *pevent, char *fmt,
 	item = malloc_or_die(sizeof(*item));
 
 	item->next = pevent->printklist;
-	pevent->printklist = item;
 	item->printk = strdup(fmt);
 	item->addr = addr;
 
+	if (!item->printk)
+		die("malloc fmt");
+
+	pevent->printklist = item;
 	pevent->printk_count++;
 
 	return 0;
@@ -2150,6 +2155,8 @@ process_fields(struct event_format *event, struct print_flag_sym **list, char **
 		if (value == NULL)
 			goto out_free;
 		field->value = strdup(value);
+		if (field->value == NULL)
+			goto out_free;
 
 		free_arg(arg);
 		arg = alloc_arg();
@@ -2163,6 +2170,8 @@ process_fields(struct event_format *event, struct print_flag_sym **list, char **
 		if (value == NULL)
 			goto out_free;
 		field->str = strdup(value);
+		if (field->str == NULL)
+			goto out_free;
 		free_arg(arg);
 		arg = NULL;
 
@@ -3433,6 +3442,9 @@ process_defined_func(struct trace_seq *s, void *data, int size,
 			string = malloc_or_die(sizeof(*string));
 			string->next = strings;
 			string->str = strdup(str.buffer);
+			if (!string->str)
+				die("malloc str");
+
 			strings = string;
 			trace_seq_destroy(&str);
 			break;
@@ -3571,6 +3583,8 @@ static struct print_arg *make_bprint_args(char *fmt, void *data, int size, struc
 				arg->next = NULL;
 				arg->type = PRINT_BSTRING;
 				arg->string.string = strdup(bptr);
+				if (!arg->string.string)
+					break;
 				bptr += strlen(bptr) + 1;
 				*next = arg;
 				next = &arg->next;
@@ -4666,6 +4680,8 @@ int pevent_parse_event(struct pevent *pevent,
 		die("failed to read event id");
 
 	event->system = strdup(sys);
+	if (!event->system)
+		die("failed to allocate system");
 
 	/* Add pevent to event so that it can be referenced */
 	event->pevent = pevent;
@@ -4707,6 +4723,10 @@ int pevent_parse_event(struct pevent *pevent,
 			list = &arg->next;
 			arg->type = PRINT_FIELD;
 			arg->field.name = strdup(field->name);
+			if (!arg->field.name) {
+				do_warning("failed to allocate field name");
+				goto event_failed;
+			}
 			arg->field.field = field;
 		}
 		return 0;
@@ -5050,6 +5070,11 @@ int pevent_register_event_handler(struct pevent *pevent,
 	if (sys_name)
 		handle->sys_name = strdup(sys_name);
 
+	if ((event_name && !handle->event_name) ||
+	    (sys_name && !handle->sys_name)) {
+		die("Failed to allocate event/sys name");
+	}
+
 	handle->func = func;
 	handle->next = pevent->handlers;
 	pevent->handlers = handle;

[tip:perf/core] tools lib traceevent: Handle realloc() failure path

[tip-bot for Namhyung Kim]

Commit-ID:  d286447f23cdb0337a5429e10b39761f6b1d5c18
Gitweb:     http://git.kernel.org/tip/d286447f23cdb0337a5429e10b39761f6b1d5c18
Author:     Namhyung Kim <namhyung.kim@lge.com>
AuthorDate: Mon, 9 Apr 2012 11:54:33 +0900
Committer:  Namhyung Kim <namhyung@kernel.org>
CommitDate: Wed, 4 Jul 2012 13:40:31 +0900

tools lib traceevent: Handle realloc() failure path

The realloc can fail so that we should handle it properly.

Signed-off-by: Namhyung Kim <namhyung.kim@lge.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Arnaldo Carvalho de Melo <acme@infradead.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: David Ahern <dsahern@gmail.com>
Link: http://lkml.kernel.org/r/1333940074-19052-7-git-send-email-namhyung.kim@lge.com
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/lib/traceevent/event-parse.c |   76 ++++++++++++++++++++++++++++--------
 1 files changed, 60 insertions(+), 16 deletions(-)

diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
index cd334d5..05eb6b4 100644
--- a/tools/lib/traceevent/event-parse.c
+++ b/tools/lib/traceevent/event-parse.c
@@ -1264,9 +1264,15 @@ static int event_read_fields(struct event_format *event, struct format_field **f
 					field->flags |= FIELD_IS_POINTER;
 
 				if (field->type) {
-					field->type = realloc(field->type,
-							      strlen(field->type) +
-							      strlen(last_token) + 2);
+					char *new_type;
+					new_type = realloc(field->type,
+							   strlen(field->type) +
+							   strlen(last_token) + 2);
+					if (!new_type) {
+						free(last_token);
+						goto fail;
+					}
+					field->type = new_type;
 					strcat(field->type, " ");
 					strcat(field->type, last_token);
 					free(last_token);
@@ -1291,6 +1297,7 @@ static int event_read_fields(struct event_format *event, struct format_field **f
 		if (strcmp(token, "[") == 0) {
 			enum event_type last_type = type;
 			char *brackets = token;
+			char *new_brackets;
 			int len;
 
 			field->flags |= FIELD_IS_ARRAY;
@@ -1310,9 +1317,14 @@ static int event_read_fields(struct event_format *event, struct format_field **f
 					len = 1;
 				last_type = type;
 
-				brackets = realloc(brackets,
-						   strlen(brackets) +
-						   strlen(token) + len);
+				new_brackets = realloc(brackets,
+						       strlen(brackets) +
+						       strlen(token) + len);
+				if (!new_brackets) {
+					free(brackets);
+					goto fail;
+				}
+				brackets = new_brackets;
 				if (len == 2)
 					strcat(brackets, " ");
 				strcat(brackets, token);
@@ -1328,7 +1340,12 @@ static int event_read_fields(struct event_format *event, struct format_field **f
 
 			free_token(token);
 
-			brackets = realloc(brackets, strlen(brackets) + 2);
+			new_brackets = realloc(brackets, strlen(brackets) + 2);
+			if (!new_brackets) {
+				free(brackets);
+				goto fail;
+			}
+			brackets = new_brackets;
 			strcat(brackets, "]");
 
 			/* add brackets to type */
@@ -1339,10 +1356,16 @@ static int event_read_fields(struct event_format *event, struct format_field **f
 			 * the format: type [] item;
 			 */
 			if (type == EVENT_ITEM) {
-				field->type = realloc(field->type,
-						      strlen(field->type) +
-						      strlen(field->name) +
-						      strlen(brackets) + 2);
+				char *new_type;
+				new_type = realloc(field->type,
+						   strlen(field->type) +
+						   strlen(field->name) +
+						   strlen(brackets) + 2);
+				if (!new_type) {
+					free(brackets);
+					goto fail;
+				}
+				field->type = new_type;
 				strcat(field->type, " ");
 				strcat(field->type, field->name);
 				free_token(field->name);
@@ -1350,9 +1373,15 @@ static int event_read_fields(struct event_format *event, struct format_field **f
 				field->name = token;
 				type = read_token(&token);
 			} else {
-				field->type = realloc(field->type,
-						      strlen(field->type) +
-						      strlen(brackets) + 1);
+				char *new_type;
+				new_type = realloc(field->type,
+						   strlen(field->type) +
+						   strlen(brackets) + 1);
+				if (!new_type) {
+					free(brackets);
+					goto fail;
+				}
+				field->type = new_type;
 				strcat(field->type, brackets);
 			}
 			free(brackets);
@@ -1735,10 +1764,16 @@ process_op(struct event_format *event, struct print_arg *arg, char **tok)
 		/* could just be a type pointer */
 		if ((strcmp(arg->op.op, "*") == 0) &&
 		    type == EVENT_DELIM && (strcmp(token, ")") == 0)) {
+			char *new_atom;
+
 			if (left->type != PRINT_ATOM)
 				die("bad pointer type");
-			left->atom.atom = realloc(left->atom.atom,
+			new_atom = realloc(left->atom.atom,
 					    strlen(left->atom.atom) + 3);
+			if (!new_atom)
+				goto out_free;
+
+			left->atom.atom = new_atom;
 			strcat(left->atom.atom, " *");
 			free(arg->op.op);
 			*arg = *left;
@@ -2597,7 +2632,16 @@ process_arg_token(struct event_format *event, struct print_arg *arg,
 		}
 		/* atoms can be more than one token long */
 		while (type == EVENT_ITEM) {
-			atom = realloc(atom, strlen(atom) + strlen(token) + 2);
+			char *new_atom;
+			new_atom = realloc(atom,
+					   strlen(atom) + strlen(token) + 2);
+			if (!new_atom) {
+				free(atom);
+				*tok = NULL;
+				free_token(token);
+				return EVENT_ERROR;
+			}
+			atom = new_atom;
 			strcat(atom, " ");
 			strcat(atom, token);
 			free_token(token);