diff mbox series

[4/5] trace-cmd: Add context to compression hooks

Message ID 20220302045131.387658-5-tz.stoyanov@gmail.com (mailing list archive)
State Superseded
Headers show
Series trace-cmd: Improvements in compression logic | expand

Commit Message

Tzvetomir Stoyanov (VMware) March 2, 2022, 4:51 a.m. UTC 
Some compression libraries require a context for compression and
decompression. Currently the internal compression API does not have such
context. That could cause problems in the future, if a multithread
compression logic is implemented.

Suggested-by: Sebastian Andrzej Siewior <sebastian@breakpoint.cc>
Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
---
 .../include/private/trace-cmd-private.h       |  8 +++--
 lib/trace-cmd/trace-compress-zlib.c           |  6 ++--
 lib/trace-cmd/trace-compress-zstd.c           |  6 ++--
 lib/trace-cmd/trace-compress.c                | 33 +++++++++++++------
 4 files changed, 34 insertions(+), 19 deletions(-)

Comments

Sebastian Andrzej Siewior March 2, 2022, 7:13 a.m. UTC | #1 
On 2022-03-02 06:51:30 [+0200], Tzvetomir Stoyanov (VMware) wrote:
> Some compression libraries require a context for compression and
> decompression. Currently the internal compression API does not have such
> context. That could cause problems in the future, if a multithread
> compression logic is implemented.
> 
> Suggested-by: Sebastian Andrzej Siewior <sebastian@breakpoint.cc>
> Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>

Acked-by: Sebastian Andrzej Siewior <sebastian@breakpoint.cc>

Sebastian
Steven Rostedt March 3, 2022, 1:10 a.m. UTC | #2 
On Wed,  2 Mar 2022 06:51:30 +0200
"Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote:

> @@ -453,6 +460,8 @@ struct tracecmd_compression *tracecmd_compress_alloc(const char *name, const cha
>  void tracecmd_compress_destroy(struct tracecmd_compression *handle)
>  {
>  	tracecmd_compress_reset(handle);
> +	if (handle->proto->free_context)
> +		handle->proto->free_context(handle->context);
>  	free(handle);
>  }
>  

I segfaulted here with:

	# trace-cmd restore -c -o /tmp/head

Need to test handle is NULL or not.

Perhaps even just exit normally at the start?

	if (!handle)
		return;

Which makes it not crash.

-- Steve
Tzvetomir Stoyanov (VMware) March 3, 2022, 4:33 p.m. UTC | #3 
On Thu, Mar 3, 2022 at 3:10 AM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> On Wed,  2 Mar 2022 06:51:30 +0200
> "Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote:
>
> > @@ -453,6 +460,8 @@ struct tracecmd_compression *tracecmd_compress_alloc(const char *name, const cha
> >  void tracecmd_compress_destroy(struct tracecmd_compression *handle)
> >  {
> >       tracecmd_compress_reset(handle);
> > +     if (handle->proto->free_context)
> > +             handle->proto->free_context(handle->context);
> >       free(handle);
> >  }
> >
>
> I segfaulted here with:
>
>         # trace-cmd restore -c -o /tmp/head
>
> Need to test handle is NULL or not.
>
> Perhaps even just exit normally at the start?
>
>         if (!handle)
>                 return;
>

ops :)  I'll send v2 of the patch set with this fix.

Thanks Steven!

> Which makes it not crash.
>
> -- Steve
diff mbox series

Patch

diff --git a/lib/trace-cmd/include/private/trace-cmd-private.h b/lib/trace-cmd/include/private/trace-cmd-private.h
index 45d89270..69343765 100644
--- a/lib/trace-cmd/include/private/trace-cmd-private.h
+++ b/lib/trace-cmd/include/private/trace-cmd-private.h
@@ -518,10 +518,12 @@  struct tracecmd_compression_proto {
 	int weight;
 	const char *name;
 	const char *version;
-	int (*compress)(const void *in, int in_bytes, void *out, int out_bytes);
-	int (*uncompress)(const void *in, int in_bytes, void *out, int out_bytes);
-	unsigned int (*compress_size)(unsigned int bytes);
+	int (*compress)(void *ctx, const void *in, int in_bytes, void *out, int out_bytes);
+	int (*uncompress)(void *ctx, const void *in, int in_bytes, void *out, int out_bytes);
+	unsigned int (*compress_size)(void *ctx, unsigned int bytes);
 	bool (*is_supported)(const char *name, const char *version);
+	void *(*new_context)(void);
+	void (*free_context)(void *ctx);
 };
 
 struct tracecmd_compression *tracecmd_compress_alloc(const char *name, const char *version,
diff --git a/lib/trace-cmd/trace-compress-zlib.c b/lib/trace-cmd/trace-compress-zlib.c
index 0dfd4f15..819a739d 100644
--- a/lib/trace-cmd/trace-compress-zlib.c
+++ b/lib/trace-cmd/trace-compress-zlib.c
@@ -13,7 +13,7 @@ 
 #define __ZLIB_NAME		"zlib"
 #define __ZLIB_WEIGTH		10
 
-static int zlib_compress(const void *in, int in_bytes, void *out, int out_bytes)
+static int zlib_compress(void *ctx, const void *in, int in_bytes, void *out, int out_bytes)
 {
 	unsigned long obytes = out_bytes;
 	int ret;
@@ -43,7 +43,7 @@  static int zlib_compress(const void *in, int in_bytes, void *out, int out_bytes)
 	return -1;
 }
 
-static int zlib_decompress(const void *in, int in_bytes, void *out, int out_bytes)
+static int zlib_decompress(void *ctx, const void *in, int in_bytes, void *out, int out_bytes)
 {
 	unsigned long obytes = out_bytes;
 	int ret;
@@ -73,7 +73,7 @@  static int zlib_decompress(const void *in, int in_bytes, void *out, int out_byte
 	return -1;
 }
 
-static unsigned int zlib_compress_bound(unsigned int in_bytes)
+static unsigned int zlib_compress_bound(void *ctx, unsigned int in_bytes)
 {
 	return compressBound(in_bytes);
 }
diff --git a/lib/trace-cmd/trace-compress-zstd.c b/lib/trace-cmd/trace-compress-zstd.c
index eee4e5d6..98eaac00 100644
--- a/lib/trace-cmd/trace-compress-zstd.c
+++ b/lib/trace-cmd/trace-compress-zstd.c
@@ -15,7 +15,7 @@ 
 static ZSTD_CCtx *ctx_c;
 static ZSTD_DCtx *ctx_d;
 
-static int zstd_compress(const void *in, int in_bytes, void *out, int out_bytes)
+static int zstd_compress(void *ctx, const void *in, int in_bytes, void *out, int out_bytes)
 {
 	size_t ret;
 
@@ -26,7 +26,7 @@  static int zstd_compress(const void *in, int in_bytes, void *out, int out_bytes)
 	return ret;
 }
 
-static int zstd_decompress(const void *in, int in_bytes, void *out, int out_bytes)
+static int zstd_decompress(void *ctx, const void *in, int in_bytes, void *out, int out_bytes)
 {
 	size_t ret;
 
@@ -40,7 +40,7 @@  static int zstd_decompress(const void *in, int in_bytes, void *out, int out_byte
 	return ret;
 }
 
-static unsigned int zstd_compress_bound(unsigned int in_bytes)
+static unsigned int zstd_compress_bound(void *ctx, unsigned int in_bytes)
 {
 	return ZSTD_compressBound(in_bytes);
 }
diff --git a/lib/trace-cmd/trace-compress.c b/lib/trace-cmd/trace-compress.c
index 6263439c..9371d3cc 100644
--- a/lib/trace-cmd/trace-compress.c
+++ b/lib/trace-cmd/trace-compress.c
@@ -18,10 +18,12 @@  struct compress_proto {
 	char *proto_version;
 	int weight;
 
-	int (*compress_block)(const void *in, int in_bytes, void *out, int out_bytes);
-	int (*uncompress_block)(const void *in,  int in_bytes, void *out, int out_bytes);
-	unsigned int (*compress_size)(unsigned int bytes);
+	int (*compress_block)(void *ctx, const void *in, int in_bytes, void *out, int out_bytes);
+	int (*uncompress_block)(void *ctx, const void *in,  int in_bytes, void *out, int out_bytes);
+	unsigned int (*compress_size)(void *ctx, unsigned int bytes);
 	bool (*is_supported)(const char *name, const char *version);
+	void *(*new_context)(void);
+	void (*free_context)(void *ctx);
 };
 
 static struct compress_proto *proto_list;
@@ -35,6 +37,7 @@  struct tracecmd_compression {
 	struct compress_proto		*proto;
 	struct tep_handle		*tep;
 	struct tracecmd_msg_handle	*msg_handle;
+	void				*context;
 };
 
 static int read_fd(int fd, char *dst, int len)
@@ -271,7 +274,8 @@  int tracecmd_uncompress_block(struct tracecmd_compression *handle)
 	if (read_fd(handle->fd, bytes, s_compressed) < 0)
 		goto error;
 
-	ret = handle->proto->uncompress_block(bytes, s_compressed, handle->buffer, size);
+	ret = handle->proto->uncompress_block(handle->context,
+					      bytes, s_compressed, handle->buffer, size);
 	if (ret < 0)
 		goto error;
 
@@ -308,13 +312,13 @@  int tracecmd_compress_block(struct tracecmd_compression *handle)
 	    !handle->proto->compress_size || !handle->proto->compress_block)
 		return -1;
 
-	size = handle->proto->compress_size(handle->pointer);
+	size = handle->proto->compress_size(handle->context, handle->pointer);
 
 	buf = malloc(size);
 	if (!buf)
 		return -1;
 
-	ret = handle->proto->compress_block(handle->buffer, handle->pointer, buf, size);
+	ret = handle->proto->compress_block(handle->context, handle->buffer, handle->pointer, buf, size);
 	if (ret < 0)
 		goto out;
 
@@ -443,6 +447,9 @@  struct tracecmd_compression *tracecmd_compress_alloc(const char *name, const cha
 	new->tep = tep;
 	new->msg_handle = msg_handle;
 	new->proto = proto;
+	if (proto->new_context)
+		new->context = proto->new_context();
+
 	return new;
 }
 
@@ -453,6 +460,8 @@  struct tracecmd_compression *tracecmd_compress_alloc(const char *name, const cha
 void tracecmd_compress_destroy(struct tracecmd_compression *handle)
 {
 	tracecmd_compress_reset(handle);
+	if (handle->proto->free_context)
+		handle->proto->free_context(handle->context);
 	free(handle);
 }
 
@@ -546,6 +555,8 @@  int tracecmd_compress_proto_register(struct tracecmd_compression_proto *proto)
 	new->is_supported = proto->is_supported;
 	new->weight = proto->weight;
 	new->next = proto_list;
+	new->new_context = proto->new_context;
+	new->free_context = proto->free_context;
 	proto_list = new;
 	return 0;
 
@@ -672,7 +683,7 @@  int tracecmd_compress_copy_from(struct tracecmd_compression *handle, int fd, int
 
 	if (read_size)
 		rmax = *read_size;
-	csize = handle->proto->compress_size(chunk_size);
+	csize = handle->proto->compress_size(handle->context, chunk_size);
 	buf_from = malloc(chunk_size);
 	if (!buf_from)
 		return -1;
@@ -706,7 +717,8 @@  int tracecmd_compress_copy_from(struct tracecmd_compression *handle, int fd, int
 		rsize += all;
 		size = csize;
 		if (all > 0) {
-			ret = handle->proto->compress_block(buf_from, all, buf_to, size);
+			ret = handle->proto->compress_block(handle->context,
+							    buf_from, all, buf_to, size);
 			if (ret < 0) {
 				if (errno == EINTR)
 					continue;
@@ -863,7 +875,8 @@  int tracecmd_uncompress_chunk(struct tracecmd_compression *handle,
 	if (read_fd(handle->fd, bytes_in, chunk->zsize) < 0)
 		goto out;
 
-	if (handle->proto->uncompress_block(bytes_in, chunk->zsize, data, chunk->size) < 0)
+	if (handle->proto->uncompress_block(handle->context,
+					    bytes_in, chunk->zsize, data, chunk->size) < 0)
 		goto out;
 
 	ret = 0;
@@ -948,7 +961,7 @@  int tracecmd_uncompress_copy_to(struct tracecmd_compression *handle, int fd,
 			break;
 
 		rsize += s_compressed;
-		ret = handle->proto->uncompress_block(bytes_in, s_compressed,
+		ret = handle->proto->uncompress_block(handle->context, bytes_in, s_compressed,
 						      bytes_out, s_uncompressed);
 		if (ret < 0)
 			break;