| Message ID | 20191018145759.15335-2-tz.stoyanov@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 113b0664003c8287793875acd6199e5b4d7aa80d |
| Headers | show |
| Series | [1/2] trace-cmd: Fix a crash on "trace-cmd reset" command | expand |
On Fri, 18 Oct 2019 17:57:59 +0300 "Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote: > --- > tracecmd/trace-record.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c > index 81aca1f..c65731f 100644 > --- a/tracecmd/trace-record.c > +++ b/tracecmd/trace-record.c > @@ -813,11 +813,14 @@ static int > write_instance_file(struct buffer_instance *instance, > const char *file, const char *str, const char *type) > { > + struct stat st; > char *path; > int ret; > > path = get_instance_file(instance, file); > - ret = write_file(path, str, type); > + ret = stat(path, &st); This is fine for now, but perhaps in the future we should check if it is writable by the user. Hmm, we could move that check to the write_file() itself. But that will require changing other locations that expect write_file() to die. Which in the long run, we want to remove that assumption. Thanks for the patch, I just applied it. -- Steve > + if (ret == 0) > + ret = write_file(path, str, type); > tracecmd_put_tracing_file(path); > > return ret;
Hi Tzvetomir, On 18/10/2019 15:57, Tzvetomir Stoyanov (VMware) wrote: > Depending of the kernel configuration, some ftrace files are optional > and may not exist. This should not confuse trace-cmd, as it is a valid > case. One such case is tracing_max_latency file, when CONFIG_TRACER_MAX_TRACE > or CONFIG_HWLAT_TRACER are not set. > A check is added in write_instance_file() to ensure the file exist before trying > to write in it. > > https://bugzilla.kernel.org/show_bug.cgi?id=205241 > > Reported-by: Valentin Schneider <valentin.schneider@arm.com> > Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com> Ran through my test just fine, so: Tested-by: Valentin Schneider <valentin.schneider@arm.com> Thanks for looking at this! > --- > tracecmd/trace-record.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c > index 81aca1f..c65731f 100644 > --- a/tracecmd/trace-record.c > +++ b/tracecmd/trace-record.c > @@ -813,11 +813,14 @@ static int > write_instance_file(struct buffer_instance *instance, > const char *file, const char *str, const char *type) > { > + struct stat st; > char *path; > int ret; > > path = get_instance_file(instance, file); > - ret = write_file(path, str, type); > + ret = stat(path, &st); > + if (ret == 0) > + ret = write_file(path, str, type); > tracecmd_put_tracing_file(path); > > return ret; >
diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c index 81aca1f..c65731f 100644 --- a/tracecmd/trace-record.c +++ b/tracecmd/trace-record.c @@ -813,11 +813,14 @@ static int write_instance_file(struct buffer_instance *instance, const char *file, const char *str, const char *type) { + struct stat st; char *path; int ret; path = get_instance_file(instance, file); - ret = write_file(path, str, type); + ret = stat(path, &st); + if (ret == 0) + ret = write_file(path, str, type); tracecmd_put_tracing_file(path); return ret;
Depending of the kernel configuration, some ftrace files are optional and may not exist. This should not confuse trace-cmd, as it is a valid case. One such case is tracing_max_latency file, when CONFIG_TRACER_MAX_TRACE or CONFIG_HWLAT_TRACER are not set. A check is added in write_instance_file() to ensure the file exist before trying to write in it. https://bugzilla.kernel.org/show_bug.cgi?id=205241 Reported-by: Valentin Schneider <valentin.schneider@arm.com> Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com> --- tracecmd/trace-record.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)