* Re: [PATCH 2/2] trace-cmd: Check if ftrace file exists, before writing in it.
2019-10-18 14:57 ` [PATCH 2/2] trace-cmd: Check if ftrace file exists, before writing in it Tzvetomir Stoyanov (VMware)
@ 2019-10-18 15:18 ` Steven Rostedt
2019-10-18 15:20 ` Valentin Schneider
1 sibling, 0 replies; 4+ messages in thread
From: Steven Rostedt @ 2019-10-18 15:18 UTC (permalink / raw)
To: Tzvetomir Stoyanov (VMware); +Cc: linux-trace-devel
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;
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH 2/2] trace-cmd: Check if ftrace file exists, before writing in it.
2019-10-18 14:57 ` [PATCH 2/2] trace-cmd: Check if ftrace file exists, before writing in it Tzvetomir Stoyanov (VMware)
2019-10-18 15:18 ` Steven Rostedt
@ 2019-10-18 15:20 ` Valentin Schneider
1 sibling, 0 replies; 4+ messages in thread
From: Valentin Schneider @ 2019-10-18 15:20 UTC (permalink / raw)
To: Tzvetomir Stoyanov (VMware), rostedt; +Cc: linux-trace-devel
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;
>
^ permalink raw reply [flat|nested] 4+ messages in thread