This set of patches contains some fixes found while studying
the trace-cmd code.
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 <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
On Sun, 23 Apr 2017 12:22:57 +0200
Federico Vaga <[email protected]> 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 <[email protected]>
> ---
> 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);
> }
> }