Re: [PATCH] trace-cmd split: Copy trace_clock from input handler to output handler

[Julia Lawall <julia.lawall@inria.fr>]

On Tue, 22 Jun 2021, Steven Rostedt wrote:

> From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
>
> When splitting a trace file, the clock is needed to create a output file
> from the input file. But the output descriptor clock is never set, and the
> clock returned from retrieving the descriptor is NULL (unless you are
> root, in which case the code will read the machine trace clock instead,

Thanks for the fix.  I should have mentioned another difference between
the machine where it worked and the machine where it didn't, which is that
I was root on the former and not root on the latter.

> and continue as if nothing was wrong). This caused a failure, and once
> again, trace-cmd split, errored out without failure, but just did not
> append all the data.
>
> Although this fixes the commit listed below, the problem was there before,
> as the code before that commit tried to read the clock from the file
> system, but just wouldn't error out if it couldn't read it.
>
> Add a new function to retrieve the trace_clock from the input handle, to
> be used to pass it to the output handle before appending the CPU data.

Just to confirm, is it still possible to run trace-cmd split without being
root?

thanks,
julia

> Reported-by: Julia Lawall <julia.lawall@inria.fr>
> Fixes: 72670886 ("trace-cmd: Save only the selected clock in the trace.dat file")
> Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
> ---
>  lib/trace-cmd/include/private/trace-cmd-private.h |  3 ++-
>  lib/trace-cmd/trace-input.c                       | 13 +++++++++++++
>  lib/trace-cmd/trace-output.c                      |  2 +-
>  tracecmd/trace-split.c                            |  1 +
>  4 files changed, 17 insertions(+), 2 deletions(-)
>
> diff --git a/lib/trace-cmd/include/private/trace-cmd-private.h b/lib/trace-cmd/include/private/trace-cmd-private.h
> index 7194cb30..6440084d 100644
> --- a/lib/trace-cmd/include/private/trace-cmd-private.h
> +++ b/lib/trace-cmd/include/private/trace-cmd-private.h
> @@ -87,7 +87,8 @@ tracecmd_plugin_context_output(struct trace_plugin_context *trace_context);
>
>  void tracecmd_set_quiet(struct tracecmd_output *handle, bool set_quiet);
>  bool tracecmd_get_quiet(struct tracecmd_output *handle);
> -void tracecmd_set_out_clock(struct tracecmd_output *handle, char *clock);
> +void tracecmd_set_out_clock(struct tracecmd_output *handle, const char *clock);
> +const char *tracecmd_get_trace_clock(struct tracecmd_input *handle);
>
>  static inline int tracecmd_host_bigendian(void)
>  {
> diff --git a/lib/trace-cmd/trace-input.c b/lib/trace-cmd/trace-input.c
> index 655bd654..ac57bc4f 100644
> --- a/lib/trace-cmd/trace-input.c
> +++ b/lib/trace-cmd/trace-input.c
> @@ -4075,6 +4075,19 @@ size_t tracecmd_get_options_offset(struct tracecmd_input *handle)
>  	return handle->options_start;
>  }
>
> +/**
> + * tracecmd_get_trace_clock - return the saved trace clock
> + * @handle: input handle for the trace.dat file
> + *
> + * Returns a string of the clock that was saved in the trace.dat file.
> + * The string should not be freed, as it points to the internal
> + * structure data.
> + */
> +const char *tracecmd_get_trace_clock(struct tracecmd_input *handle)
> +{
> +	return handle->trace_clock;
> +}
> +
>  /**
>   * tracecmd_get_show_data_func - return the show data func
>   * @handle: input handle for the trace.dat file
> diff --git a/lib/trace-cmd/trace-output.c b/lib/trace-cmd/trace-output.c
> index 78a25350..a8de107c 100644
> --- a/lib/trace-cmd/trace-output.c
> +++ b/lib/trace-cmd/trace-output.c
> @@ -122,7 +122,7 @@ void tracecmd_set_quiet(struct tracecmd_output *handle, bool set_quiet)
>  		handle->quiet = set_quiet;
>  }
>
> -void tracecmd_set_out_clock(struct tracecmd_output *handle, char *clock)
> +void tracecmd_set_out_clock(struct tracecmd_output *handle, const char *clock)
>  {
>  	if (handle && clock)
>  		handle->trace_clock = strdup(clock);
> diff --git a/tracecmd/trace-split.c b/tracecmd/trace-split.c
> index 8366d128..10d0943d 100644
> --- a/tracecmd/trace-split.c
> +++ b/tracecmd/trace-split.c
> @@ -384,6 +384,7 @@ static double parse_file(struct tracecmd_input *handle,
>  	for (cpu = 0; cpu < cpus; cpu ++)
>  		cpu_list[cpu] = cpu_data[cpu].file;
>
> +	tracecmd_set_out_clock(ohandle, tracecmd_get_trace_clock(handle));
>  	tracecmd_append_cpu_data(ohandle, cpus, cpu_list);
>
>  	current = end;
> --
> 2.29.2
>
>