[PATCH 0/8] trace-cmd: Fixes for trace-cmd restore

[PATCH 0/8] trace-cmd: Fixes for trace-cmd restore

[Steven Rostedt]

The trace-cmd restore has been broken for some time. This series
fixes it.

Steven Rostedt (VMware) (8):
      trace-cmd restore: Fix to add saved cmdlines after calling tracecmd_create_init_file_override()
      trace-cmd: Create API tracecmd_read_pre_headers()
      trace-cmd: Move tracecmd_write_cmdlines() out of tracecmd_append_cpu_data()
      trace-cmd: Move the output state updates into the functions that change the state
      trace-cmd: Move the input state updates into the functions that change the state
      trace-cmd output: Set file_state of output handle after copy of headers
      trace-cmd input: Validate the input handle when copying from it
      trace-cmd input: Add validation updates to the copy of a handle

----
 lib/trace-cmd/include/private/trace-cmd-private.h |   1 +
 lib/trace-cmd/trace-input.c                       | 100 ++++++++++++++++----
 lib/trace-cmd/trace-output.c                      | 109 ++++++++++++++--------
 tracecmd/trace-record.c                           |   3 +
 tracecmd/trace-restore.c                          |   4 +-
 tracecmd/trace-split.c                            |   4 +
 6 files changed, 164 insertions(+), 57 deletions(-)

[PATCH 1/8] trace-cmd restore: Fix to add saved cmdlines after calling tracecmd_create_init_file_override()

[Steven Rostedt]

From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>

The saving of command lines was moved out of the create_file() logic to
capture them after the tracing has finished. But this broke trace-cmd
restore as it expected them to be saved by the
tracecmd_create_init_file_override() function.

Fixes: 1eea02a4b ("trace-cmd: Write saved cmdlines in the trace file at the end of the trace.")
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 tracecmd/trace-restore.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tracecmd/trace-restore.c b/tracecmd/trace-restore.c
index 98e757337a03..13f803053582 100644
--- a/tracecmd/trace-restore.c
+++ b/tracecmd/trace-restore.c
@@ -94,6 +94,8 @@ void trace_restore (int argc, char **argv)
 							    kallsyms);
 		if (!handle)
 			die("Unabled to create output file %s", output);
+		if (tracecmd_write_cmdlines(handle) < 0)
+			die("Failed to write command lines");
 		tracecmd_output_close(handle);
 		exit(0);
 	}
-- 
2.30.0

[PATCH 2/8] trace-cmd: Create API tracecmd_read_pre_headers()

[Steven Rostedt]

From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>

The trace-cmd restore operation can create a partial header to read the
trace event formats and kallsyms and other data into a stand alone header
before it has access to the cpu data. Then it will also read this header to
put together a broken trace, and it reads the header that does not have the
cpu data attached to it.

In order to handle this case, it needs a way to read the headers but stop
short of reading the CPU information. That requires breaking up
tracecmd_read_headers() with just stopping short of adding the cpu data.

A new API is added called tracecmd_read_pre_headers() that does exactly that.

Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 .../include/private/trace-cmd-private.h       |  1 +
 lib/trace-cmd/trace-input.c                   | 25 ++++++++++++++++---
 tracecmd/trace-restore.c                      |  2 +-
 3 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/lib/trace-cmd/include/private/trace-cmd-private.h b/lib/trace-cmd/include/private/trace-cmd-private.h
index fc968cc9efe1..c7ef3af7c8f7 100644
--- a/lib/trace-cmd/include/private/trace-cmd-private.h
+++ b/lib/trace-cmd/include/private/trace-cmd-private.h
@@ -153,6 +153,7 @@ typedef void (*tracecmd_handle_init_func)(struct tracecmd_input *handle,
 struct tracecmd_input *tracecmd_alloc(const char *file, int flags);
 struct tracecmd_input *tracecmd_alloc_fd(int fd, int flags);
 void tracecmd_ref(struct tracecmd_input *handle);
+int tracecmd_read_pre_headers(struct tracecmd_input *handle);
 int tracecmd_read_headers(struct tracecmd_input *handle);
 int tracecmd_get_parsing_failures(struct tracecmd_input *handle);
 int tracecmd_long_size(struct tracecmd_input *handle);
diff --git a/lib/trace-cmd/trace-input.c b/lib/trace-cmd/trace-input.c
index 9ef7b9f16951..9e1a44540201 100644
--- a/lib/trace-cmd/trace-input.c
+++ b/lib/trace-cmd/trace-input.c
@@ -772,14 +772,17 @@ static int read_cpus(struct tracecmd_input *handle)
 }
 
 /**
- * tracecmd_read_headers - read the header information from trace.dat
+ * tracecmd_read_pre_headers - read the header information from trace.dat
  * @handle: input handle for the trace.dat file
  *
  * This reads the trace.dat file for various information. Like the
  * format of the ring buffer, event formats, ftrace formats, kallsyms
- * and printk.
+ * and printk, but stops before reading cpu and options.
+ *
+ * This is needed by the restore operation where the header does not
+ * have the CPU information yet.
  */
-int tracecmd_read_headers(struct tracecmd_input *handle)
+int tracecmd_read_pre_headers(struct tracecmd_input *handle)
 {
 	int ret;
 
@@ -815,6 +818,22 @@ int tracecmd_read_headers(struct tracecmd_input *handle)
 		return -1;
 	handle->file_state = TRACECMD_FILE_CMD_LINES;
 
+	return 0;
+}
+
+/**
+ * tracecmd_read_headers - read the header information from trace.dat
+ * @handle: input handle for the trace.dat file
+ *
+ * This reads the trace.dat file for various information. Like the
+ * format of the ring buffer, event formats, ftrace formats, kallsyms
+ * and printk.
+ */
+int tracecmd_read_headers(struct tracecmd_input *handle)
+{
+	if (tracecmd_read_pre_headers(handle))
+		return -1;
+
 	if (read_cpus(handle) < 0)
 		return -1;
 	handle->file_state = TRACECMD_FILE_CPU_COUNT;
diff --git a/tracecmd/trace-restore.c b/tracecmd/trace-restore.c
index 13f803053582..bf6940991178 100644
--- a/tracecmd/trace-restore.c
+++ b/tracecmd/trace-restore.c
@@ -122,7 +122,7 @@ void trace_restore (int argc, char **argv)
 		if (!ihandle)
 			die("error reading file %s", input);
 		/* make sure headers are ok */
-		if (tracecmd_read_headers(ihandle) < 0)
+		if (tracecmd_read_pre_headers(ihandle) < 0)
 			die("error reading file %s headers", input);
 
 		handle = tracecmd_copy(ihandle, output);
-- 
2.30.0

Re: [PATCH 2/8] trace-cmd: Create API tracecmd_read_pre_headers()

[Tzvetomir Stoyanov]

Hi Steven,

On Mon, Mar 1, 2021 at 4:38 PM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
>
> The trace-cmd restore operation can create a partial header to read the
> trace event formats and kallsyms and other data into a stand alone header
> before it has access to the cpu data. Then it will also read this header to
> put together a broken trace, and it reads the header that does not have the
> cpu data attached to it.
>
> In order to handle this case, it needs a way to read the headers but stop
> short of reading the CPU information. That requires breaking up
> tracecmd_read_headers() with just stopping short of adding the cpu data.

While looking at these changes, I think it could be more flexible instead of
introducing a new API, to enhance the existing tracecmd_read_headers().
We can add an additional (optional) parameter to point out what headers
to be read, i.e. the desired new state of reading the file. Something like that:
   int tracecmd_read_headers(struct tracecmd_input *handle, int state);
It will read the file from its current file_state to the desired state.
The API could be called multiple times, to progress the reading of the
file, header by header if needed, or a bunch of headers.



>
> A new API is added called tracecmd_read_pre_headers() that does exactly that.
>
> Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
> ---
>  .../include/private/trace-cmd-private.h       |  1 +
>  lib/trace-cmd/trace-input.c                   | 25 ++++++++++++++++---
>  tracecmd/trace-restore.c                      |  2 +-
>  3 files changed, 24 insertions(+), 4 deletions(-)
>
> diff --git a/lib/trace-cmd/include/private/trace-cmd-private.h b/lib/trace-cmd/include/private/trace-cmd-private.h
> index fc968cc9efe1..c7ef3af7c8f7 100644
> --- a/lib/trace-cmd/include/private/trace-cmd-private.h
> +++ b/lib/trace-cmd/include/private/trace-cmd-private.h
> @@ -153,6 +153,7 @@ typedef void (*tracecmd_handle_init_func)(struct tracecmd_input *handle,
>  struct tracecmd_input *tracecmd_alloc(const char *file, int flags);
>  struct tracecmd_input *tracecmd_alloc_fd(int fd, int flags);
>  void tracecmd_ref(struct tracecmd_input *handle);
> +int tracecmd_read_pre_headers(struct tracecmd_input *handle);
>  int tracecmd_read_headers(struct tracecmd_input *handle);
>  int tracecmd_get_parsing_failures(struct tracecmd_input *handle);
>  int tracecmd_long_size(struct tracecmd_input *handle);
> diff --git a/lib/trace-cmd/trace-input.c b/lib/trace-cmd/trace-input.c
> index 9ef7b9f16951..9e1a44540201 100644
> --- a/lib/trace-cmd/trace-input.c
> +++ b/lib/trace-cmd/trace-input.c
> @@ -772,14 +772,17 @@ static int read_cpus(struct tracecmd_input *handle)
>  }
>
>  /**
> - * tracecmd_read_headers - read the header information from trace.dat
> + * tracecmd_read_pre_headers - read the header information from trace.dat
>   * @handle: input handle for the trace.dat file
>   *
>   * This reads the trace.dat file for various information. Like the
>   * format of the ring buffer, event formats, ftrace formats, kallsyms
> - * and printk.
> + * and printk, but stops before reading cpu and options.
> + *
> + * This is needed by the restore operation where the header does not
> + * have the CPU information yet.
>   */
> -int tracecmd_read_headers(struct tracecmd_input *handle)
> +int tracecmd_read_pre_headers(struct tracecmd_input *handle)
>  {
>         int ret;
>
> @@ -815,6 +818,22 @@ int tracecmd_read_headers(struct tracecmd_input *handle)
>                 return -1;
>         handle->file_state = TRACECMD_FILE_CMD_LINES;
>
> +       return 0;
> +}
> +
> +/**
> + * tracecmd_read_headers - read the header information from trace.dat
> + * @handle: input handle for the trace.dat file
> + *
> + * This reads the trace.dat file for various information. Like the
> + * format of the ring buffer, event formats, ftrace formats, kallsyms
> + * and printk.
> + */
> +int tracecmd_read_headers(struct tracecmd_input *handle)
> +{
> +       if (tracecmd_read_pre_headers(handle))
> +               return -1;
> +
>         if (read_cpus(handle) < 0)
>                 return -1;
>         handle->file_state = TRACECMD_FILE_CPU_COUNT;
> diff --git a/tracecmd/trace-restore.c b/tracecmd/trace-restore.c
> index 13f803053582..bf6940991178 100644
> --- a/tracecmd/trace-restore.c
> +++ b/tracecmd/trace-restore.c
> @@ -122,7 +122,7 @@ void trace_restore (int argc, char **argv)
>                 if (!ihandle)
>                         die("error reading file %s", input);
>                 /* make sure headers are ok */
> -               if (tracecmd_read_headers(ihandle) < 0)
> +               if (tracecmd_read_pre_headers(ihandle) < 0)
>                         die("error reading file %s headers", input);
>
>                 handle = tracecmd_copy(ihandle, output);
> --
> 2.30.0
>
>


--
Tzvetomir (Ceco) Stoyanov
VMware Open Source Technology Center

Re: [PATCH 2/8] trace-cmd: Create API tracecmd_read_pre_headers()

[Steven Rostedt]

On Tue, 2 Mar 2021 06:49:18 +0200
Tzvetomir Stoyanov <tz.stoyanov@gmail.com> wrote:

> While looking at these changes, I think it could be more flexible instead of
> introducing a new API, to enhance the existing tracecmd_read_headers().
> We can add an additional (optional) parameter to point out what headers
> to be read, i.e. the desired new state of reading the file. Something like that:
>    int tracecmd_read_headers(struct tracecmd_input *handle, int state);
> It will read the file from its current file_state to the desired state.
> The API could be called multiple times, to progress the reading of the
> file, header by header if needed, or a bunch of headers.
> 

I actually like this idea. The only downside of it, is that we need to make
the states part of the API (export them).

-- Steve

[PATCH 3/8] trace-cmd: Move tracecmd_write_cmdlines() out of tracecmd_append_cpu_data()

[Steven Rostedt]

From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>

When the saved_cmdlines was moved to after the recording, as it makes sense
because they are created during the recording, the saving was just tossed
into tracecmd_append_cpu_data() as that was called at the end of recording.

Unfortunately, the trace-cmd restore (as well as trace-cmd split) used the
tracecmd_append_cpu_data() but expecting it not store the saved_cmdlines.
This broke both of them.

Now that there's an API called tracecmd_write_cmdlines(), have those that
need it call it directly, and remove it out of tracecmd_append_cpu_data().

Note, although this can help the trace-cmd restore code, it appears that the
code for trace-cmd split may still be broken, and needs to be fixed.

Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 lib/trace-cmd/trace-output.c | 10 +++-------
 tracecmd/trace-record.c      |  3 +++
 tracecmd/trace-split.c       |  4 ++++
 3 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/lib/trace-cmd/trace-output.c b/lib/trace-cmd/trace-output.c
index c8f8a106c295..917d20cfcfd6 100644
--- a/lib/trace-cmd/trace-output.c
+++ b/lib/trace-cmd/trace-output.c
@@ -1446,13 +1446,6 @@ int tracecmd_append_cpu_data(struct tracecmd_output *handle,
 {
 	int ret;
 
-	/*
-	 * Save the command lines;
-	 */
-	ret = tracecmd_write_cmdlines(handle);
-	if (ret)
-		return ret;
-
 	ret = tracecmd_write_cpus(handle, cpus);
 	if (ret)
 		return ret;
@@ -1554,6 +1547,9 @@ tracecmd_create_file_glob(const char *output_file,
 	if (!handle)
 		return NULL;
 
+	if (tracecmd_write_cmdlines(handle))
+		return NULL;
+
 	if (tracecmd_append_cpu_data(handle, cpus, cpu_data_files) < 0) {
 		tracecmd_output_close(handle);
 		return NULL;
diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c
index 4337967e11e5..e7428788b1bb 100644
--- a/tracecmd/trace-record.c
+++ b/tracecmd/trace-record.c
@@ -4236,6 +4236,9 @@ static void record_data(struct common_record_context *ctx)
 				add_guest_info(handle, instance);
 		}
 
+		if (tracecmd_write_cmdlines(handle))
+			die("Writing cmdlines");
+
 		tracecmd_append_cpu_data(handle, local_cpu_count, temp_files);
 
 		for (i = 0; i < max_cpu_count; i++)
diff --git a/tracecmd/trace-split.c b/tracecmd/trace-split.c
index 8366d1286d9e..7c9863d481bc 100644
--- a/tracecmd/trace-split.c
+++ b/tracecmd/trace-split.c
@@ -384,6 +384,10 @@ static double parse_file(struct tracecmd_input *handle,
 	for (cpu = 0; cpu < cpus; cpu ++)
 		cpu_list[cpu] = cpu_data[cpu].file;
 
+	/* TODO: Fix me, this is suppose to come from handle */
+	if (tracecmd_write_cmdlines(ohandle))
+		die("Writing cmdlines");
+
 	tracecmd_append_cpu_data(ohandle, cpus, cpu_list);
 
 	current = end;
-- 
2.30.0

[PATCH 4/8] trace-cmd: Move the output state updates into the functions that change the state

[Steven Rostedt]

From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>

It makes more sense to have the functions that change the state of the
descriptor to change the value that stores the state. This makes it more
robust in case these functions are called by something other than
create_file_fd(). That way the state changes with the update, and this
removes the dependency on create_file_fd with the state changes.

Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 lib/trace-cmd/trace-output.c | 97 ++++++++++++++++++++++++------------
 1 file changed, 65 insertions(+), 32 deletions(-)

diff --git a/lib/trace-cmd/trace-output.c b/lib/trace-cmd/trace-output.c
index 917d20cfcfd6..6d504cbaf133 100644
--- a/lib/trace-cmd/trace-output.c
+++ b/lib/trace-cmd/trace-output.c
@@ -297,6 +297,33 @@ int tracecmd_ftrace_enable(int set)
 	return ret;
 }
 
+static int check_out_state(struct tracecmd_output *handle, int new_state)
+{
+	if (!handle)
+		return -1;
+
+	switch (new_state) {
+	case TRACECMD_FILE_HEADERS:
+	case TRACECMD_FILE_FTRACE_EVENTS:
+	case TRACECMD_FILE_ALL_EVENTS:
+	case TRACECMD_FILE_KALLSYMS:
+	case TRACECMD_FILE_PRINTK:
+	case TRACECMD_FILE_CMD_LINES:
+	case TRACECMD_FILE_CPU_COUNT:
+	case TRACECMD_FILE_OPTIONS:
+		if (handle->file_state == (new_state - 1))
+			return 0;
+		break;
+	case TRACECMD_FILE_CPU_LATENCY:
+	case TRACECMD_FILE_CPU_FLYRECORD:
+		if (handle->file_state == TRACECMD_FILE_OPTIONS)
+			return 0;
+		break;
+	}
+
+	return -1;
+}
+
 static int read_header_files(struct tracecmd_output *handle)
 {
 	tsize_t size, check_size, endian8;
@@ -305,6 +332,12 @@ static int read_header_files(struct tracecmd_output *handle)
 	int fd;
 	int ret;
 
+	if (check_out_state(handle, TRACECMD_FILE_HEADERS) < 0) {
+		warning("Cannot read header files, unexpected state 0x%X",
+			handle->file_state);
+		return -1;
+	}
+
 	path = get_tracing_file(handle, "events/header_page");
 	if (!path)
 		return -1;
@@ -373,6 +406,9 @@ static int read_header_files(struct tracecmd_output *handle)
 		return -1;
 	}
 	put_tracing_file(path);
+
+	handle->file_state = TRACECMD_FILE_HEADERS;
+
 	return 0;
 
  out_close:
@@ -609,12 +645,20 @@ static int read_ftrace_files(struct tracecmd_output *handle)
 	struct tracecmd_event_list list = { .glob = "ftrace/*" };
 	int ret;
 
+	if (check_out_state(handle, TRACECMD_FILE_FTRACE_EVENTS) < 0) {
+		warning("Cannot read ftrace files, unexpected state 0x%X",
+			handle->file_state);
+		return -1;
+	}
+
 	create_event_list_item(handle, &systems, &list);
 
 	ret = copy_event_system(handle, systems);
 
 	free_list_events(systems);
 
+	handle->file_state = TRACECMD_FILE_FTRACE_EVENTS;
+
 	return ret;
 }
 
@@ -642,6 +686,11 @@ static int read_event_files(struct tracecmd_output *handle,
 	int endian4;
 	int ret;
 
+	if (check_out_state(handle, TRACECMD_FILE_ALL_EVENTS) < 0) {
+		warning("Cannot read event files, unexpected state 0x%X",
+			handle->file_state);
+		return -1;
+	}
 	/*
 	 * If any of the list is the special keyword "all" then
 	 * just do all files.
@@ -674,6 +723,7 @@ static int read_event_files(struct tracecmd_output *handle,
 		ret = copy_event_system(handle, slist);
 	}
 
+	handle->file_state = TRACECMD_FILE_ALL_EVENTS;
  out_free:
 	free_list_events(systems);
 
@@ -728,6 +778,12 @@ static int read_proc_kallsyms(struct tracecmd_output *handle,
 	struct stat st;
 	int ret;
 
+	if (check_out_state(handle, TRACECMD_FILE_KALLSYMS) < 0) {
+		warning("Cannot read kallsyms, unexpected state 0x%X",
+			handle->file_state);
+		return -1;
+	}
+
 	if (kallsyms)
 		path = kallsyms;
 
@@ -755,6 +811,8 @@ static int read_proc_kallsyms(struct tracecmd_output *handle,
 	}
 	set_proc_kptr_restrict(1);
 
+	handle->file_state = TRACECMD_FILE_KALLSYMS;
+
 	return 0;
 }
 
@@ -765,6 +823,12 @@ static int read_ftrace_printk(struct tracecmd_output *handle)
 	char *path;
 	int ret;
 
+	if (check_out_state(handle, TRACECMD_FILE_PRINTK) < 0) {
+		warning("Cannot read printk, unexpected state 0x%X",
+			handle->file_state);
+		return -1;
+	}
+
 	path = get_tracing_file(handle, "printk_formats");
 	if (!path)
 		return -1;
@@ -790,6 +854,7 @@ static int read_ftrace_printk(struct tracecmd_output *handle)
 	}
 
  out:
+	handle->file_state = TRACECMD_FILE_PRINTK;
 	put_tracing_file(path);
 	return 0;
  fail:
@@ -836,33 +901,6 @@ out_free:
 	return ret;
 }
 
-static int check_out_state(struct tracecmd_output *handle, int new_state)
-{
-	if (!handle)
-		return -1;
-
-	switch (new_state) {
-	case TRACECMD_FILE_HEADERS:
-	case TRACECMD_FILE_FTRACE_EVENTS:
-	case TRACECMD_FILE_ALL_EVENTS:
-	case TRACECMD_FILE_KALLSYMS:
-	case TRACECMD_FILE_PRINTK:
-	case TRACECMD_FILE_CMD_LINES:
-	case TRACECMD_FILE_CPU_COUNT:
-	case TRACECMD_FILE_OPTIONS:
-		if (handle->file_state == (new_state - 1))
-			return 0;
-		break;
-	case TRACECMD_FILE_CPU_LATENCY:
-	case TRACECMD_FILE_CPU_FLYRECORD:
-		if (handle->file_state == TRACECMD_FILE_OPTIONS)
-			return 0;
-		break;
-	}
-
-	return -1;
-}
-
 static struct tracecmd_output *
 create_file_fd(int fd, struct tracecmd_input *ihandle,
 	       const char *tracing_dir,
@@ -939,23 +977,18 @@ create_file_fd(int fd, struct tracecmd_input *ihandle,
 
 	if (read_header_files(handle))
 		goto out_free;
-	handle->file_state = TRACECMD_FILE_HEADERS;
 
 	if (read_ftrace_files(handle))
 		goto out_free;
-	handle->file_state = TRACECMD_FILE_FTRACE_EVENTS;
 
 	if (read_event_files(handle, list))
 		goto out_free;
-	handle->file_state = TRACECMD_FILE_ALL_EVENTS;
 
 	if (read_proc_kallsyms(handle, kallsyms))
 		goto out_free;
-	handle->file_state = TRACECMD_FILE_KALLSYMS;
 
 	if (read_ftrace_printk(handle))
 		goto out_free;
-	handle->file_state = TRACECMD_FILE_PRINTK;
 
 	return handle;
 
-- 
2.30.0

[PATCH 5/8] trace-cmd: Move the input state updates into the functions that change the state

[Steven Rostedt]

From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>

It makes more sense to have the functions that change the state of the
descriptor to change the value that stores the state. This makes it more
robust in case these functions are called by something other than
tracecmd_read_headers(). That way the state changes with the update, and this
removes the dependency on create_file_fd with the state changes.

Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 lib/trace-cmd/trace-input.c | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/lib/trace-cmd/trace-input.c b/lib/trace-cmd/trace-input.c
index 9e1a44540201..9a4b1f4e118a 100644
--- a/lib/trace-cmd/trace-input.c
+++ b/lib/trace-cmd/trace-input.c
@@ -395,6 +395,8 @@ static int read_header_files(struct tracecmd_input *handle)
 	handle->ftrace_files_start =
 		lseek64(handle->fd, 0, SEEK_CUR);
 
+	handle->file_state = TRACECMD_FILE_HEADERS;
+
 	return 0;
 
  failed_read:
@@ -596,6 +598,8 @@ static int read_ftrace_files(struct tracecmd_input *handle, const char *regex)
 		regfree(ereg);
 	}
 
+	handle->file_state = TRACECMD_FILE_FTRACE_EVENTS;
+
 	return 0;
 }
 
@@ -678,6 +682,8 @@ static int read_event_files(struct tracecmd_input *handle, const char *regex)
 		regfree(ereg);
 	}
 
+	handle->file_state = TRACECMD_FILE_ALL_EVENTS;
+
 	return 0;
 
  failed:
@@ -713,6 +719,9 @@ static int read_proc_kallsyms(struct tracecmd_input *handle)
 	tracecmd_parse_proc_kallsyms(pevent, buf, size);
 
 	free(buf);
+
+	handle->file_state = TRACECMD_FILE_KALLSYMS;
+
 	return 0;
 }
 
@@ -740,6 +749,8 @@ static int read_ftrace_printk(struct tracecmd_input *handle)
 
 	free(buf);
 
+	handle->file_state = TRACECMD_FILE_PRINTK;
+
 	return 0;
 }
 
@@ -791,32 +802,27 @@ int tracecmd_read_pre_headers(struct tracecmd_input *handle)
 	ret = read_header_files(handle);
 	if (ret < 0)
 		return -1;
-	handle->file_state = TRACECMD_FILE_HEADERS;
+
 	tep_set_long_size(handle->pevent, handle->long_size);
 
 	ret = read_ftrace_files(handle, NULL);
 	if (ret < 0)
 		return -1;
-	handle->file_state = TRACECMD_FILE_FTRACE_EVENTS;
 
 	ret = read_event_files(handle, NULL);
 	if (ret < 0)
 		return -1;
-	handle->file_state = TRACECMD_FILE_ALL_EVENTS;
 
 	ret = read_proc_kallsyms(handle);
 	if (ret < 0)
 		return -1;
-	handle->file_state = TRACECMD_FILE_KALLSYMS;
 
 	ret = read_ftrace_printk(handle);
 	if (ret < 0)
 		return -1;
-	handle->file_state = TRACECMD_FILE_PRINTK;
 
 	if (read_and_parse_cmdlines(handle) < 0)
 		return -1;
-	handle->file_state = TRACECMD_FILE_CMD_LINES;
 
 	return 0;
 }
@@ -2848,6 +2854,9 @@ static int read_and_parse_cmdlines(struct tracecmd_input *handle)
 	cmdlines[size] = 0;
 	tracecmd_parse_cmdlines(pevent, cmdlines, size);
 	free(cmdlines);
+
+	handle->file_state = TRACECMD_FILE_CMD_LINES;
+
 	return 0;
 }
 
-- 
2.30.0

[PATCH 6/8] trace-cmd output: Set file_state of output handle after copy of headers

[Steven Rostedt]

From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>

Now that the input and output handles know the state they are at in reading
or writing, the tracecmd_copy() has to set the state of the output handle it
creates.

Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 lib/trace-cmd/trace-output.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/trace-cmd/trace-output.c b/lib/trace-cmd/trace-output.c
index 6d504cbaf133..1156899a85d3 100644
--- a/lib/trace-cmd/trace-output.c
+++ b/lib/trace-cmd/trace-output.c
@@ -1656,6 +1656,8 @@ struct tracecmd_output *tracecmd_copy(struct tracecmd_input *ihandle,
 	if (tracecmd_copy_headers(ihandle, handle->fd) < 0)
 		goto out_free;
 
+	handle->file_state = TRACECMD_FILE_CMD_LINES;
+
 	/* The file is all ready to have cpu data attached */
 	return handle;
 
-- 
2.30.0

Re: [PATCH 6/8] trace-cmd output: Set file_state of output handle after copy of headers

[Tzvetomir Stoyanov]

On Mon, Mar 1, 2021 at 4:38 PM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
>
> Now that the input and output handles know the state they are at in reading
> or writing, the tracecmd_copy() has to set the state of the output handle it
> creates.
>
> Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
> ---
>  lib/trace-cmd/trace-output.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/lib/trace-cmd/trace-output.c b/lib/trace-cmd/trace-output.c
> index 6d504cbaf133..1156899a85d3 100644
> --- a/lib/trace-cmd/trace-output.c
> +++ b/lib/trace-cmd/trace-output.c
> @@ -1656,6 +1656,8 @@ struct tracecmd_output *tracecmd_copy(struct tracecmd_input *ihandle,
>         if (tracecmd_copy_headers(ihandle, handle->fd) < 0)
>                 goto out_free;
>
> +       handle->file_state = TRACECMD_FILE_CMD_LINES;

Why is the state overwritten here, isn't it more logical to be set in
tracecmd_copy_headers(), by each function that copies a header to set
the relevant state. The last call in tracecmd_copy_headers()
is copy_command_lines(), which should set state to
TRACECMD_FILE_CMD_LINES in case of success.
The state is already TRACECMD_FILE_CMD_LINES
in tracecmd_copy_headers(), but right before its exit it
is overwritten to the old file state. And here again it is
overwritten back to TRACECMD_FILE_CMD_LINES.
May be I miss something here, cannot understand the logic.

> +
>         /* The file is all ready to have cpu data attached */
>         return handle;
>
> --
> 2.30.0
>
>


-- 
Tzvetomir (Ceco) Stoyanov
VMware Open Source Technology Center

Re: [PATCH 6/8] trace-cmd output: Set file_state of output handle after copy of headers

[Steven Rostedt]

On Tue, 2 Mar 2021 10:10:24 +0200
Tzvetomir Stoyanov <tz.stoyanov@gmail.com> wrote:

> On Mon, Mar 1, 2021 at 4:38 PM Steven Rostedt <rostedt@goodmis.org> wrote:
> >
> > From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
> >
> > Now that the input and output handles know the state they are at in reading
> > or writing, the tracecmd_copy() has to set the state of the output handle it
> > creates.
> >
> > Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
> > ---
> >  lib/trace-cmd/trace-output.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/lib/trace-cmd/trace-output.c b/lib/trace-cmd/trace-output.c
> > index 6d504cbaf133..1156899a85d3 100644
> > --- a/lib/trace-cmd/trace-output.c
> > +++ b/lib/trace-cmd/trace-output.c
> > @@ -1656,6 +1656,8 @@ struct tracecmd_output *tracecmd_copy(struct tracecmd_input *ihandle,
> >         if (tracecmd_copy_headers(ihandle, handle->fd) < 0)
> >                 goto out_free;
> >
> > +       handle->file_state = TRACECMD_FILE_CMD_LINES;  
> 
> Why is the state overwritten here, isn't it more logical to be set in
> tracecmd_copy_headers(), by each function that copies a header to set

That's because the handle is not passed into tracecmd_copy_headers.

And because the handle is a struct tracecmd_output, the
tracecmd_copy_headers() which is in trace-input.c doesn't have access to
this structure, and I prefer to keep it that way.

That said, we could modify tracecmd_copy_header() to return the state that
it copied up to, or negative on error.

	state = tracecmd_copy_headers(ihandle, handle->fd);
	if (state < 0)
		goto out_free;

	handle->file_state = state;

That would be more robust!

-- Steve


> the relevant state. The last call in tracecmd_copy_headers()
> is copy_command_lines(), which should set state to
> TRACECMD_FILE_CMD_LINES in case of success.
> The state is already TRACECMD_FILE_CMD_LINES
> in tracecmd_copy_headers(), but right before its exit it
> is overwritten to the old file state. And here again it is
> overwritten back to TRACECMD_FILE_CMD_LINES.
> May be I miss something here, cannot understand the logic.
> 
> > +
> >         /* The file is all ready to have cpu data attached */
> >         return handle;
> >
> > --
> > 2.30.0
> >
> >  
> 
> 

Re: [PATCH 6/8] trace-cmd output: Set file_state of output handle after copy of headers

[Tzvetomir Stoyanov]

On Tue, Mar 2, 2021 at 4:19 PM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> On Tue, 2 Mar 2021 10:10:24 +0200
> Tzvetomir Stoyanov <tz.stoyanov@gmail.com> wrote:
>
> > On Mon, Mar 1, 2021 at 4:38 PM Steven Rostedt <rostedt@goodmis.org> wrote:
> > >
> > > From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
> > >
> > > Now that the input and output handles know the state they are at in reading
> > > or writing, the tracecmd_copy() has to set the state of the output handle it
> > > creates.
> > >
> > > Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
> > > ---
> > >  lib/trace-cmd/trace-output.c | 2 ++
> > >  1 file changed, 2 insertions(+)
> > >
> > > diff --git a/lib/trace-cmd/trace-output.c b/lib/trace-cmd/trace-output.c
> > > index 6d504cbaf133..1156899a85d3 100644
> > > --- a/lib/trace-cmd/trace-output.c
> > > +++ b/lib/trace-cmd/trace-output.c
> > > @@ -1656,6 +1656,8 @@ struct tracecmd_output *tracecmd_copy(struct tracecmd_input *ihandle,
> > >         if (tracecmd_copy_headers(ihandle, handle->fd) < 0)
> > >                 goto out_free;
> > >
> > > +       handle->file_state = TRACECMD_FILE_CMD_LINES;
> >
> > Why is the state overwritten here, isn't it more logical to be set in
> > tracecmd_copy_headers(), by each function that copies a header to set
>
> That's because the handle is not passed into tracecmd_copy_headers.
>
> And because the handle is a struct tracecmd_output, the
> tracecmd_copy_headers() which is in trace-input.c doesn't have access to
> this structure, and I prefer to keep it that way.
>
> That said, we could modify tracecmd_copy_header() to return the state that
> it copied up to, or negative on error.
>
>         state = tracecmd_copy_headers(ihandle, handle->fd);
>         if (state < 0)
>                 goto out_free;
>
>         handle->file_state = state;

The output handle should have the same state as the input handle,
so we can just have:

      handle->file_state = tracecmd_get_file_state(ihandle);

There is exactly the same use case in tracecmd_get_output_handle_fd(),
where the out handle is built on a partially written file.

>
> That would be more robust!
>
> -- Steve
>
>
> > the relevant state. The last call in tracecmd_copy_headers()
> > is copy_command_lines(), which should set state to
> > TRACECMD_FILE_CMD_LINES in case of success.
> > The state is already TRACECMD_FILE_CMD_LINES
> > in tracecmd_copy_headers(), but right before its exit it
> > is overwritten to the old file state. And here again it is
> > overwritten back to TRACECMD_FILE_CMD_LINES.
> > May be I miss something here, cannot understand the logic.
> >
> > > +
> > >         /* The file is all ready to have cpu data attached */
> > >         return handle;
> > >
> > > --
> > > 2.30.0
> > >
> > >
> >
> >
>


-- 
Tzvetomir (Ceco) Stoyanov
VMware Open Source Technology Center

Re: [PATCH 6/8] trace-cmd output: Set file_state of output handle after copy of headers

[Steven Rostedt]

On Tue, 2 Mar 2021 16:51:56 +0200
Tzvetomir Stoyanov <tz.stoyanov@gmail.com> wrote:

> >         handle->file_state = state;  
> 
> The output handle should have the same state as the input handle,
> so we can just have:
> 
>       handle->file_state = tracecmd_get_file_state(ihandle);
> 
> There is exactly the same use case in tracecmd_get_output_handle_fd(),
> where the out handle is built on a partially written file.

The above is pretty much exactly what I did, but it eliminates error
checking. Should there be a file_state = TRACECMD_FILE_ERROR ?

-- Steve

Re: [PATCH 6/8] trace-cmd output: Set file_state of output handle after copy of headers

[Tzvetomir Stoyanov]

On Tue, Mar 2, 2021 at 5:48 PM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> On Tue, 2 Mar 2021 16:51:56 +0200
> Tzvetomir Stoyanov <tz.stoyanov@gmail.com> wrote:
>
> > >         handle->file_state = state;
> >
> > The output handle should have the same state as the input handle,
> > so we can just have:
> >
> >       handle->file_state = tracecmd_get_file_state(ihandle);
> >
> > There is exactly the same use case in tracecmd_get_output_handle_fd(),
> > where the out handle is built on a partially written file.
>
> The above is pretty much exactly what I did, but it eliminates error

There is an error checking, if tracecmd_copy_headers() returns 0 then
the ihandle state must be valid and we can use it safely.
The tracecmd_get_file_state() could fail only in case of a NULL
ihandle pointer.

> checking. Should there be a file_state = TRACECMD_FILE_ERROR ?

file_state should point to the last valid file read / write state, in case of
read / write broken section of the file, the state should not be updated.
The use case for invalid state can be an initial state, before
TRACECMD_FILE_INIT.
May be TRACECMD_FILE_UKOWN, but I cannot find a use case for it
in the current implementation.

>
> -- Steve



-- 
Tzvetomir (Ceco) Stoyanov
VMware Open Source Technology Center

Re: [PATCH 6/8] trace-cmd output: Set file_state of output handle after copy of headers

[Steven Rostedt]

On Tue, 2 Mar 2021 19:35:11 +0200
Tzvetomir Stoyanov <tz.stoyanov@gmail.com> wrote:

> On Tue, Mar 2, 2021 at 5:48 PM Steven Rostedt <rostedt@goodmis.org> wrote:
> >
> > On Tue, 2 Mar 2021 16:51:56 +0200
> > Tzvetomir Stoyanov <tz.stoyanov@gmail.com> wrote:
> >  
> > > >         handle->file_state = state;  
> > >
> > > The output handle should have the same state as the input handle,
> > > so we can just have:
> > >
> > >       handle->file_state = tracecmd_get_file_state(ihandle);
> > >
> > > There is exactly the same use case in tracecmd_get_output_handle_fd(),
> > > where the out handle is built on a partially written file.  
> >
> > The above is pretty much exactly what I did, but it eliminates error  
> 
> There is an error checking, if tracecmd_copy_headers() returns 0 then
> the ihandle state must be valid and we can use it safely.
> The tracecmd_get_file_state() could fail only in case of a NULL
> ihandle pointer.

Nevermind, I mistaken the "tracecmd_get_file_state()" as
"tracecmd_copy_headers()", I didn't notice that you introduced another API.

Sure something like this would work too.

-- Steve

Re: [PATCH 6/8] trace-cmd output: Set file_state of output handle after copy of headers

[Steven Rostedt]

On Tue, 2 Mar 2021 10:10:24 +0200
Tzvetomir Stoyanov <tz.stoyanov@gmail.com> wrote:

> Why is the state overwritten here, isn't it more logical to be set in
> tracecmd_copy_headers(), by each function that copies a header to set
> the relevant state. The last call in tracecmd_copy_headers()
> is copy_command_lines(), which should set state to
> TRACECMD_FILE_CMD_LINES in case of success.
> The state is already TRACECMD_FILE_CMD_LINES
> in tracecmd_copy_headers(), but right before its exit it
> is overwritten to the old file state. And here again it is
> overwritten back to TRACECMD_FILE_CMD_LINES.
> May be I miss something here, cannot understand the logic.

Also, as I believe you noticed, I saved the state in
tracecmd_copy_headers() and restored it. But thinking about this more, I'm
not sure I like that, and was thinking of just leaving the state of the
input handle in the last state that it was updated in.

In other words, I wasn't sure the best way to handle this, and reset the
state because the original version didn't modify the state, and I was just
keeping that the same. But since the fd is now different, it may be a good
idea to change the state of the handle.

This was something that I wanted to discuss with you, and I'm glad you
brought it up, because I forgot about it ;-)

-- Steve

[PATCH 7/8] trace-cmd input: Validate the input handle when copying from it

[Steven Rostedt]

From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>

Now that there's validation states, make sure that the input handle is at
the correct state to validate it.

Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 lib/trace-cmd/trace-input.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/lib/trace-cmd/trace-input.c b/lib/trace-cmd/trace-input.c
index 9a4b1f4e118a..53c2722f46e7 100644
--- a/lib/trace-cmd/trace-input.c
+++ b/lib/trace-cmd/trace-input.c
@@ -3491,6 +3491,10 @@ static int copy_header_files(struct tracecmd_input *handle, int fd)
 {
 	unsigned long long size;
 
+	/* The input handle has to have at least read the headers */
+	if (handle->file_state < TRACECMD_FILE_HEADERS)
+		return -1;
+
 	lseek64(handle->fd, handle->header_files_start, SEEK_SET);
 
 	/* "header_page"  */
@@ -3522,6 +3526,10 @@ static int copy_ftrace_files(struct tracecmd_input *handle, int fd)
 	unsigned int count;
 	unsigned int i;
 
+	/* The input handle has to have at least read the ftrace events */
+	if (handle->file_state < TRACECMD_FILE_FTRACE_EVENTS)
+		return -1;
+
 	if (read_copy_size4(handle, fd, &count) < 0)
 		return -1;
 
@@ -3545,6 +3553,10 @@ static int copy_event_files(struct tracecmd_input *handle, int fd)
 	unsigned int count;
 	unsigned int i,x;
 
+	/* The input handle has to have at least read all its events */
+	if (handle->file_state < TRACECMD_FILE_ALL_EVENTS)
+		return -1;
+
 	if (read_copy_size4(handle, fd, &systems) < 0)
 		return -1;
 
@@ -3577,6 +3589,10 @@ static int copy_proc_kallsyms(struct tracecmd_input *handle, int fd)
 {
 	unsigned int size;
 
+	/* The input handle has to have at least has kallsyms */
+	if (handle->file_state < TRACECMD_FILE_KALLSYMS)
+		return -1;
+
 	if (read_copy_size4(handle, fd, &size) < 0)
 		return -1;
 	if (!size)
@@ -3592,6 +3608,10 @@ static int copy_ftrace_printk(struct tracecmd_input *handle, int fd)
 {
 	unsigned int size;
 
+	/* The input handle has to have at least has printk stored */
+	if (handle->file_state < TRACECMD_FILE_PRINTK)
+		return -1;
+
 	if (read_copy_size4(handle, fd, &size) < 0)
 		return -1;
 	if (!size)
@@ -3607,6 +3627,10 @@ static int copy_command_lines(struct tracecmd_input *handle, int fd)
 {
 	unsigned long long size;
 
+	/* The input handle has to have at least read the cmdlines */
+	if (handle->file_state < TRACECMD_FILE_CMD_LINES)
+		return -1;
+
 	if (read_copy_size8(handle, fd, &size) < 0)
 		return -1;
 	if (!size)
-- 
2.30.0

[PATCH 8/8] trace-cmd input: Add validation updates to the copy of a handle

[Steven Rostedt]

From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>

When copying the input handle the file descriptor is changed. Change its
state along with that.

Currently the tracecmd_copy_headers() restores the original state, but does
not restore the file descriptor. That may need to change.

Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 lib/trace-cmd/trace-input.c | 52 +++++++++++++++++++++++--------------
 1 file changed, 33 insertions(+), 19 deletions(-)

diff --git a/lib/trace-cmd/trace-input.c b/lib/trace-cmd/trace-input.c
index 53c2722f46e7..15d6d2b3ca43 100644
--- a/lib/trace-cmd/trace-input.c
+++ b/lib/trace-cmd/trace-input.c
@@ -3491,12 +3491,14 @@ static int copy_header_files(struct tracecmd_input *handle, int fd)
 {
 	unsigned long long size;
 
-	/* The input handle has to have at least read the headers */
 	if (handle->file_state < TRACECMD_FILE_HEADERS)
 		return -1;
 
 	lseek64(handle->fd, handle->header_files_start, SEEK_SET);
 
+	/* Now that the file handle has moved, change its state */
+	handle->file_state = TRACECMD_FILE_HEADERS;
+
 	/* "header_page"  */
 	if (read_copy_data(handle, 12, fd) < 0)
 		return -1;
@@ -3526,8 +3528,7 @@ static int copy_ftrace_files(struct tracecmd_input *handle, int fd)
 	unsigned int count;
 	unsigned int i;
 
-	/* The input handle has to have at least read the ftrace events */
-	if (handle->file_state < TRACECMD_FILE_FTRACE_EVENTS)
+	if (handle->file_state != TRACECMD_FILE_FTRACE_EVENTS - 1)
 		return -1;
 
 	if (read_copy_size4(handle, fd, &count) < 0)
@@ -3542,6 +3543,8 @@ static int copy_ftrace_files(struct tracecmd_input *handle, int fd)
 			return -1;
 	}
 
+	handle->file_state = TRACECMD_FILE_FTRACE_EVENTS;
+
 	return 0;
 }
 
@@ -3553,8 +3556,7 @@ static int copy_event_files(struct tracecmd_input *handle, int fd)
 	unsigned int count;
 	unsigned int i,x;
 
-	/* The input handle has to have at least read all its events */
-	if (handle->file_state < TRACECMD_FILE_ALL_EVENTS)
+	if (handle->file_state != TRACECMD_FILE_ALL_EVENTS - 1)
 		return -1;
 
 	if (read_copy_size4(handle, fd, &systems) < 0)
@@ -3582,6 +3584,8 @@ static int copy_event_files(struct tracecmd_input *handle, int fd)
 		}
 	}
 
+	handle->file_state = TRACECMD_FILE_ALL_EVENTS;
+
 	return 0;
 }
 
@@ -3589,8 +3593,7 @@ static int copy_proc_kallsyms(struct tracecmd_input *handle, int fd)
 {
 	unsigned int size;
 
-	/* The input handle has to have at least has kallsyms */
-	if (handle->file_state < TRACECMD_FILE_KALLSYMS)
+	if (handle->file_state != TRACECMD_FILE_KALLSYMS - 1)
 		return -1;
 
 	if (read_copy_size4(handle, fd, &size) < 0)
@@ -3601,6 +3604,8 @@ static int copy_proc_kallsyms(struct tracecmd_input *handle, int fd)
 	if (read_copy_data(handle, size, fd) < 0)
 		return -1;
 
+	handle->file_state = TRACECMD_FILE_KALLSYMS;
+
 	return 0;
 }
 
@@ -3608,8 +3613,7 @@ static int copy_ftrace_printk(struct tracecmd_input *handle, int fd)
 {
 	unsigned int size;
 
-	/* The input handle has to have at least has printk stored */
-	if (handle->file_state < TRACECMD_FILE_PRINTK)
+	if (handle->file_state != TRACECMD_FILE_PRINTK - 1)
 		return -1;
 
 	if (read_copy_size4(handle, fd, &size) < 0)
@@ -3620,6 +3624,8 @@ static int copy_ftrace_printk(struct tracecmd_input *handle, int fd)
 	if (read_copy_data(handle, size, fd) < 0)
 		return -1;
 
+	handle->file_state = TRACECMD_FILE_PRINTK;
+
 	return 0;
 }
 
@@ -3627,8 +3633,7 @@ static int copy_command_lines(struct tracecmd_input *handle, int fd)
 {
 	unsigned long long size;
 
-	/* The input handle has to have at least read the cmdlines */
-	if (handle->file_state < TRACECMD_FILE_CMD_LINES)
+	if (handle->file_state != TRACECMD_FILE_CMD_LINES - 1)
 		return -1;
 
 	if (read_copy_size8(handle, fd, &size) < 0)
@@ -3639,38 +3644,47 @@ static int copy_command_lines(struct tracecmd_input *handle, int fd)
 	if (read_copy_data(handle, size, fd) < 0)
 		return -1;
 
+	handle->file_state = TRACECMD_FILE_CMD_LINES;
+
 	return 0;
 }
 
 int tracecmd_copy_headers(struct tracecmd_input *handle, int fd)
 {
+	int save_state = handle->file_state;
 	int ret;
 
+	/* Make sure that the input handle is up to cmd lines */
+	if (handle->file_state < TRACECMD_FILE_CMD_LINES)
+		return -1;
+
 	ret = copy_header_files(handle, fd);
 	if (ret < 0)
-		return -1;
+		goto out;
 
 	ret = copy_ftrace_files(handle, fd);
 	if (ret < 0)
-		return -1;
+		goto out;
 
 	ret = copy_event_files(handle, fd);
 	if (ret < 0)
-		return -1;
+		goto out;
 
 	ret = copy_proc_kallsyms(handle, fd);
 	if (ret < 0)
-		return -1;
+		goto out;
 
 	ret = copy_ftrace_printk(handle, fd);
 	if (ret < 0)
-		return -1;
+		goto out;
 
 	ret = copy_command_lines(handle, fd);
-	if (ret < 0)
-		return -1;
 
-	return 0;
+ out:
+	/* Restore the handle back to its original state */
+	handle->file_state = save_state;
+
+	return ret < 0 ? -1 : 0;
 }
 
 /**
-- 
2.30.0