Re: [PATCH v7 13/25] trace-cmd library: Introduce sections in trace file reading logic

[Steven Rostedt <rostedt@goodmis.org>]

On Fri, 10 Dec 2021 12:54:36 +0200
"Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote:

> Trace file version 7 is based on sections. Added an internal sections
> database and new helper functions to add, read, open and close file
> sections.
> 
> Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
> ---
>  lib/trace-cmd/trace-input.c | 69 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 69 insertions(+)
> 
> diff --git a/lib/trace-cmd/trace-input.c b/lib/trace-cmd/trace-input.c
> index 0375afba..78f9effd 100644
> --- a/lib/trace-cmd/trace-input.c
> +++ b/lib/trace-cmd/trace-input.c
> @@ -115,6 +115,14 @@ struct tsc2nsec {
>  	unsigned long long offset;
>  };
>  
> +struct file_section {
> +	unsigned long long		section_offset;
> +	unsigned long long		data_offset;
> +	int				id;
> +	int				flags;
> +	struct file_section		*next;
> +};
> +
>  struct tracecmd_input {
>  	struct tep_handle	*pevent;
>  	unsigned long		file_state;
> @@ -154,6 +162,7 @@ struct tracecmd_input {
>  	struct hook_list	*hooks;
>  	struct pid_addr_maps	*pid_maps;
>  	/* file information */
> +	struct file_section	*sections;
>  	size_t			header_files_start;
>  	size_t			ftrace_files_start;
>  	size_t			event_files_start;
> @@ -377,6 +386,58 @@ static int read8(struct tracecmd_input *handle, unsigned long long *size)
>  	return 0;
>  }
>  
> +static struct file_section *section_get(struct tracecmd_input *handle, int id)
> +{
> +	struct file_section *sec;
> +
> +	for (sec = handle->sections; sec; sec = sec->next) {
> +		if (sec->id == id)
> +			return sec;
> +	}
> +
> +	return NULL;
> +}
> +
> +static struct file_section *section_open(struct tracecmd_input *handle, int id)
> +{
> +	struct file_section *sec = section_get(handle, id);
> +
> +	if (!sec)
> +		return NULL;
> +
> +	if (lseek64(handle->fd, sec->data_offset, SEEK_SET) == (off64_t)-1)
> +		return NULL;
> +	return sec;
> +}
> +
> +static void section_close(struct tracecmd_input *handle, struct file_section *sec)
> +{
> +	/* To Do */
> +}
> +
> +static int section_add_or_update(struct tracecmd_input *handle, int id, int flags,
> +				 unsigned long long section_offset,
> +				 unsigned long long data_offset)
> +{
> +	struct file_section *sec = section_get(handle, id);
> +
> +	if (!sec) {
> +		sec = calloc(1, sizeof(struct file_section));
> +		if (!sec)
> +			return -1;
> +		sec->next = handle->sections;
> +		handle->sections = sec;
> +	}
> +	sec->id = id;

Could move the above into the previous if block. As it's only to be
updated if it wasn't already found (with the id).

> +	if (section_offset)
> +		sec->section_offset = section_offset;
> +	if (data_offset)
> +		sec->data_offset = data_offset;
> +	if (flags > 0)

Why the greater than zero check?

Is there a way to clear flags?

> +		sec->flags = flags;
> +	return 0;
> +}

-- Steve

> +
>  static int read_header_files(struct tracecmd_input *handle)
>  {
>  	struct tep_handle *pevent = handle->pevent;
> @@ -3493,6 +3554,7 @@ void tracecmd_ref(struct tracecmd_input *handle)
>   */
>  void tracecmd_close(struct tracecmd_input *handle)
>  {
> +	struct file_section *del_sec;
>  	int cpu;
>  	int i;
>  
> @@ -3532,6 +3594,12 @@ void tracecmd_close(struct tracecmd_input *handle)
>  	free(handle->version);
>  	close(handle->fd);
>  
> +	while (handle->sections) {
> +		del_sec = handle->sections;
> +		handle->sections = handle->sections->next;
> +		free(del_sec);
> +	}
> +
>  	for (i = 0; i < handle->nr_buffers; i++)
>  		free(handle->buffers[i].name);
>  	free(handle->buffers);
> @@ -3976,6 +4044,7 @@ tracecmd_buffer_instance_handle(struct tracecmd_input *handle, int indx)
>  	new_handle->nr_buffers = 0;
>  	new_handle->buffers = NULL;
>  	new_handle->version = NULL;
> +	new_handle->sections = NULL;
>  	new_handle->guest = NULL;
>  	new_handle->ref = 1;
>  	if (handle->trace_clock) {