diff mbox series

[v7,05/25] trace-cmd library: Add internal helper function for writing headers before file sections

Message ID 20211210105448.97850-6-tz.stoyanov@gmail.com (mailing list archive)
State Superseded
Headers show
Series Trace file version 7 - sections | expand

Commit Message

Tzvetomir Stoyanov (VMware) Dec. 10, 2021, 10:54 a.m. UTC
Introduce headers before each file section, in trace file version 7. The
section header has the following format:
 <2 bytes>, header ID
 <4 bytes>, offset within the strings section, where the section
            description string is located.
 <2 bytes>, section flags:
     1: the section is compressed
 <8 bytes>, size of the section

Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
---
 include/trace-cmd/trace-cmd.h                 |  5 ++
 .../include/private/trace-cmd-private.h       |  1 +
 lib/trace-cmd/include/trace-cmd-local.h       |  5 ++
 lib/trace-cmd/trace-output.c                  | 77 ++++++++++++++++++-
 4 files changed, 87 insertions(+), 1 deletion(-)

Comments

Steven Rostedt Jan. 15, 2022, 12:58 p.m. UTC | #1
On Fri, 10 Dec 2021 12:54:28 +0200
"Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote:

> Introduce headers before each file section, in trace file version 7. The
> section header has the following format:
>  <2 bytes>, header ID
>  <4 bytes>, offset within the strings section, where the section
>             description string is located.
>  <2 bytes>, section flags:
>      1: the section is compressed

Would in make more sense to have the ID and flags together, followed by
the string? That way you could use a normal structure to represent it.

struct {
	unsigned short			id;
	unsigned short			flags;
	unsigned int			string_offset;
	unsigned long long		size;
};

Otherwise, all tools to read it will be forced to read each part
individually. I can imaging that being annoying in the future.

-- Steve


>  <8 bytes>, size of the section
> 
> Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
> ---
Tzvetomir Stoyanov (VMware) Jan. 17, 2022, 10:08 a.m. UTC | #2
On Sat, Jan 15, 2022 at 2:59 PM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> On Fri, 10 Dec 2021 12:54:28 +0200
> "Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote:
>
> > Introduce headers before each file section, in trace file version 7. The
> > section header has the following format:
> >  <2 bytes>, header ID
> >  <4 bytes>, offset within the strings section, where the section
> >             description string is located.
> >  <2 bytes>, section flags:
> >      1: the section is compressed
>
> Would in make more sense to have the ID and flags together, followed by
> the string? That way you could use a normal structure to represent it.
>
It would make sense in the previous version of this implementation,
where a dynamic string was stored between the ID and the flags. Now as
all these are in fixed positions, the order should not be a problem.
But anyway, I'll change it in the next version.

> struct {
>         unsigned short                  id;
>         unsigned short                  flags;
>         unsigned int                    string_offset;
>         unsigned long long              size;
> };
>
> Otherwise, all tools to read it will be forced to read each part
> individually. I can imaging that being annoying in the future.
>
> -- Steve
>
>
> >  <8 bytes>, size of the section
> >
> > Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
> > ---
diff mbox series

Patch

diff --git a/include/trace-cmd/trace-cmd.h b/include/trace-cmd/trace-cmd.h
index 7fea4e01..5d71e8ba 100644
--- a/include/trace-cmd/trace-cmd.h
+++ b/include/trace-cmd/trace-cmd.h
@@ -15,6 +15,11 @@  enum tracecmd_open_flags {
 	TRACECMD_FL_LOAD_NO_PLUGINS		= 1 << 0, /* Do not load plugins */
 	TRACECMD_FL_LOAD_NO_SYSTEM_PLUGINS	= 1 << 1, /* Do not load system plugins */
 };
+
+enum tracecmd_section_flags {
+	TRACECMD_SEC_FL_COMPRESS	= 1 << 0, /* the section is compressed */
+};
+
 struct tracecmd_input *tracecmd_open_head(const char *file, int flags);
 struct tracecmd_input *tracecmd_open(const char *file, int flags);
 struct tracecmd_input *tracecmd_open_fd(int fd, int flags);
diff --git a/lib/trace-cmd/include/private/trace-cmd-private.h b/lib/trace-cmd/include/private/trace-cmd-private.h
index ed8fbf11..1584d65a 100644
--- a/lib/trace-cmd/include/private/trace-cmd-private.h
+++ b/lib/trace-cmd/include/private/trace-cmd-private.h
@@ -139,6 +139,7 @@  enum {
 	TRACECMD_OPTION_GUEST,
 	TRACECMD_OPTION_TSC2NSEC,
 	TRACECMD_OPTION_STRINGS,
+	TRACECMD_OPTION_MAX,
 };
 
 enum {
diff --git a/lib/trace-cmd/include/trace-cmd-local.h b/lib/trace-cmd/include/trace-cmd-local.h
index 7f280533..4a0a691c 100644
--- a/lib/trace-cmd/include/trace-cmd-local.h
+++ b/lib/trace-cmd/include/trace-cmd-local.h
@@ -39,6 +39,11 @@  struct data_file_write {
 bool check_file_state(unsigned long file_version, int current_state, int new_state);
 bool check_out_state(struct tracecmd_output *handle, int new_state);
 
+unsigned long long
+out_write_section_header(struct tracecmd_output *handle, unsigned short header_id,
+			 char *description, int flags, bool option);
+int out_update_section_header(struct tracecmd_output *handle, unsigned long long offset);
+
 struct cpu_data_source {
 	int fd;
 	int size;
diff --git a/lib/trace-cmd/trace-output.c b/lib/trace-cmd/trace-output.c
index ed505db6..457da305 100644
--- a/lib/trace-cmd/trace-output.c
+++ b/lib/trace-cmd/trace-output.c
@@ -360,6 +360,82 @@  int tracecmd_ftrace_enable(int set)
 	return ret;
 }
 
+__hidden unsigned long long
+out_write_section_header(struct tracecmd_output *handle, unsigned short header_id,
+			 char *description, int flags, bool option)
+{
+	tsize_t endian8;
+	tsize_t offset;
+	long long size;
+	short endian2;
+	int endian4;
+	int desc;
+
+	if (header_id >= TRACECMD_OPTION_MAX)
+		return -1;
+	if (!HAS_SECTIONS(handle))
+		return 0;
+	offset = do_lseek(handle, 0, SEEK_CUR);
+	if (option) {
+		endian8 = convert_endian_8(handle, offset);
+		if (!tracecmd_add_option(handle, header_id, 8, &endian8))
+			return -1;
+	}
+	/* Section ID */
+	endian2 = convert_endian_2(handle, header_id);
+	if (do_write_check(handle, &endian2, 2))
+		return (off64_t)-1;
+
+	/* Section description */
+	if (description)
+		desc = add_string(handle, description);
+	else
+		desc = -1;
+	endian4 = convert_endian_4(handle, desc);
+	if (do_write_check(handle, &endian4, 4))
+		return (off64_t)-1;
+
+	/* Section flags */
+	endian2 = convert_endian_2(handle, flags);
+	if (do_write_check(handle, &endian2, 2))
+		return (off64_t)-1;
+
+	offset = do_lseek(handle, 0, SEEK_CUR);
+	size = 0;
+	/* Reserve for section size */
+	if (do_write_check(handle, &size, 8))
+		return (off64_t)-1;
+	return offset;
+}
+
+__hidden int out_update_section_header(struct tracecmd_output *handle, tsize_t offset)
+{
+	tsize_t current;
+	tsize_t endian8;
+	tsize_t size;
+
+	if (!HAS_SECTIONS(handle) || offset == 0)
+		return 0;
+
+	current = do_lseek(handle, 0, SEEK_CUR);
+	/* The real size is the difference between the saved offset and
+	 * the current offset - 8 bytes, the reserved space for the section size.
+	 */
+	size = current - offset;
+	if (size < 8)
+		return -1;
+	size -= 8;
+	if (do_lseek(handle, offset, SEEK_SET) == (off64_t)-1)
+		return -1;
+
+	endian8 = convert_endian_8(handle, size);
+	if (do_write_check(handle, &endian8, 8))
+		return -1;
+	if (do_lseek(handle, current, SEEK_SET) == (off64_t)-1)
+		return -1;
+	return 0;
+}
+
 static int save_string_section(struct tracecmd_output *handle)
 {
 	if (!handle->strings || !handle->strings_p)
@@ -385,7 +461,6 @@  error:
 	return -1;
 }
 
-
 static int read_header_files(struct tracecmd_output *handle)
 {
 	tsize_t size, check_size, endian8;