trace-cmd bug fixes

trace-cmd bug fixes

[Federico Vaga]

This set of patches contains some fixes found while studying
the trace-cmd code.

[PATCH 1/5] plugin:python: fix compiler warning

[Federico Vaga]

The function `load_plugin` is passed, as argument, to
`trace_util_load_plugins()` but the prototype was not exactly the same.

Signed-off-by: Federico Vaga <federico.vaga@vaga.pv.it>
---
 plugin_python.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/plugin_python.c b/plugin_python.c
index da07d27..d3da8b0 100644
--- a/plugin_python.c
+++ b/plugin_python.c
@@ -20,7 +20,7 @@ static const char pyload[] =
 "finally:\n"
 "   file.close()\n";
 
-static void load_plugin(struct pevent *pevent, const char *path,
+static int load_plugin(struct pevent *pevent, const char *path,
 			const char *name, void *data)
 {
 	PyObject *globals = data;
@@ -32,7 +32,7 @@ static void load_plugin(struct pevent *pevent, const char *path,
 	PyObject *res;
 
 	if (!full || !n)
-		return;
+		return -ENOMEM;
 
 	strcpy(full, path);
 	strcat(full, "/");
@@ -43,7 +43,7 @@ static void load_plugin(struct pevent *pevent, const char *path,
 
 	asprintf(&load, pyload, full, n);
 	if (!load)
-		return;
+		return -ENOMEM;
 
 	res = PyRun_String(load, Py_file_input, globals, globals);
 	if (!res) {
@@ -53,6 +53,8 @@ static void load_plugin(struct pevent *pevent, const char *path,
 		Py_DECREF(res);
 
 	free(load);
+
+	return 0;
 }
 
 int PEVENT_PLUGIN_LOADER(struct pevent *pevent)
-- 
2.9.3

[PATCH 2/5] plugin:python: check asprintf() errors

[Federico Vaga]

The code was validating the string but the man page for asprintf(3)
says clearly:

--------
If memory allocation wasn't possible, or some other error occurs,
these functions  will return -1, and the contents of strp are undefined.
--------

So, we cannot really rely on the returned string pointer.

Signed-off-by: Federico Vaga <federico.vaga@vaga.pv.it>
---
 plugin_python.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/plugin_python.c b/plugin_python.c
index d3da8b0..2997679 100644
--- a/plugin_python.c
+++ b/plugin_python.c
@@ -24,6 +24,7 @@ static int load_plugin(struct pevent *pevent, const char *path,
 			const char *name, void *data)
 {
 	PyObject *globals = data;
+	int err;
 	int len = strlen(path) + strlen(name) + 2;
 	int nlen = strlen(name) + 1;
 	char *full = malloc(len);
@@ -41,9 +42,9 @@ static int load_plugin(struct pevent *pevent, const char *path,
 	strcpy(n, name);
 	n[nlen - 4] = '\0';
 
-	asprintf(&load, pyload, full, n);
-	if (!load)
-		return -ENOMEM;
+	err = asprintf(&load, pyload, full, n);
+	if (err < 0)
+		return err;
 
 	res = PyRun_String(load, Py_file_input, globals, globals);
 	if (!res) {
-- 
2.9.3

[PATCH 3/5] trace-cmd:read: BUG initialize input_files item to zero

[Federico Vaga]

On allocation the data structure was not initialized. Later on some
attribute of this structure are used (e.g. tsoffset) assuming that the
default value is zero, but it is not always true.

Signed-off-by: Federico Vaga <federico.vaga@vaga.pv.it>
---
 trace-read.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/trace-read.c b/trace-read.c
index 79519bd..9773a47 100644
--- a/trace-read.c
+++ b/trace-read.c
@@ -295,6 +295,7 @@ static void add_input(const char *file)
 	item = malloc(sizeof(*item));
 	if (!item)
 		die("Failed to allocate for %s", file);
+	memset(item, 0, sizeof(*item));
 	item->file = file;
 	list_add_tail(&item->list, &input_files);
 	last_input_file = item;
-- 
2.9.3

[PATCH 4/5] trace-cmd: fix argument parsing minor BUG

[Federico Vaga]

For some reason the list command does not use anymore `getopt()`
to parse the arguments, instead it uses a custum implementation.

During this change [5da0eff trace-cmd: Add regex for listing of events]
the variable `optind` has been forgotten.

To reproduce the problem try to use invalid arguments. The application
will not report the correct invalid argument

$ ./trace-cmd list -a
list: invalid option -- 'i'

Signed-off-by: Federico Vaga <federico.vaga@vaga.pv.it>
---
 trace-cmd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/trace-cmd.c b/trace-cmd.c
index 1a62faa..a05df92 100644
--- a/trace-cmd.c
+++ b/trace-cmd.c
@@ -706,7 +706,7 @@ int main (int argc, char **argv)
 					break;
 				default:
 					fprintf(stderr, "list: invalid option -- '%c'\n",
-						argv[optind][1]);
+						argv[i][1]);
 					usage(argv);
 				}
 			}
-- 
2.9.3

[PATCH 5/5] trace-cmd: BUG fix malloc() pointer validation

[Federico Vaga]

To reproduce the bug

mkdir /sys/kernel/debug/tracing/instances/test
./trace-cmd show -B test -s -f
  Failed to allocate instance path snapshot

Signed-off-by: Federico Vaga <federico.vaga@vaga.pv.it>
---
 trace-cmd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/trace-cmd.c b/trace-cmd.c
index a05df92..320229e 100644
--- a/trace-cmd.c
+++ b/trace-cmd.c
@@ -616,7 +616,7 @@ int main (int argc, char **argv)
 		if (buffer) {
 			path = malloc(strlen(buffer) + strlen("instances//") +
 				      strlen(file) + 1);
-			if (path)
+			if (!path)
 				die("Failed to allocate instance path %s", file);
 			sprintf(path, "instances/%s/%s", buffer, file);
 			file = path;
-- 
2.9.3

Re: [PATCH 4/5] trace-cmd: fix argument parsing minor BUG

[Steven Rostedt]

On Sun, 23 Apr 2017 12:22:57 +0200
Federico Vaga <federico.vaga@vaga.pv.it> wrote:

> For some reason the list command does not use anymore `getopt()`
> to parse the arguments, instead it uses a custum implementation.
> 
> During this change [5da0eff trace-cmd: Add regex for listing of events]
> the variable `optind` has been forgotten.
> 
> To reproduce the problem try to use invalid arguments. The application
> will not report the correct invalid argument
> 
> $ ./trace-cmd list -a
> list: invalid option -- 'i'
> 
> Signed-off-by: Federico Vaga <federico.vaga@vaga.pv.it>
> ---
>  trace-cmd.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/trace-cmd.c b/trace-cmd.c
> index 1a62faa..a05df92 100644
> --- a/trace-cmd.c
> +++ b/trace-cmd.c
> @@ -706,7 +706,7 @@ int main (int argc, char **argv)
>  					break;
>  				default:
>  					fprintf(stderr, "list: invalid option -- '%c'\n",
> -						argv[optind][1]);
> +						argv[i][1]);

Hmm, I had this fixed back in September. I haven't push in a long time.

Well, I applied your other patches (Thanks!). I'm going to finally
start updating trace-cmd this week.

-- Steve
 
>  					usage(argv);
>  				}
>  			}