diff mbox series

[3/5] trace-cmd: Use errno from zlib, if available

Message ID 20220302045131.387658-4-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 zlib APIs set the errno in case of an error and return Z_ERRNO.
In these cases, errno should not be overwritten by the tarce-cmd zlib
wrappers.

Suggested-by: Sebastian Andrzej Siewior <sebastian@breakpoint.cc>
Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
---
 lib/trace-cmd/trace-compress-zlib.c | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Sebastian Andrzej Siewior March 2, 2022, 7:15 a.m. UTC | #1
On 2022-03-02 06:51:29 [+0200], Tzvetomir Stoyanov (VMware) wrote:
> Some zlib APIs set the errno in case of an error and return Z_ERRNO.
> In these cases, errno should not be overwritten by the tarce-cmd zlib
> wrappers.
> 
> Suggested-by: Sebastian Andrzej Siewior <sebastian@breakpoint.cc>
> Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
> ---
>  lib/trace-cmd/trace-compress-zlib.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/lib/trace-cmd/trace-compress-zlib.c b/lib/trace-cmd/trace-compress-zlib.c
> index 41342597..0dfd4f15 100644
> --- a/lib/trace-cmd/trace-compress-zlib.c
> +++ b/lib/trace-cmd/trace-compress-zlib.c
> @@ -33,6 +33,8 @@ static int zlib_compress(const void *in, int in_bytes, void *out, int out_bytes)
>  	case Z_STREAM_ERROR:
>  		errno = -EINVAL;
>  		break;
> +	case Z_ERRNO:
> +		break;
>  	default:
>  		errno = -EFAULT;
>  		break;
> @@ -61,6 +63,8 @@ static int zlib_decompress(const void *in, int in_bytes, void *out, int out_byte
>  	case Z_DATA_ERROR:
>  		errno = -EINVAL;
>  		break;
> +	case Z_ERRNO:
> +		break;
>  	default:
>  		errno = -EFAULT;
>  		break;

I was thinking about returning the error for compress/decompress via the
return value and not touching errno at all.

Sebastian
Steven Rostedt March 2, 2022, 3:52 p.m. UTC | #2
On Wed, 2 Mar 2022 08:15:30 +0100
Sebastian Andrzej Siewior <sebastian@breakpoint.cc> wrote:

> I was thinking about returning the error for compress/decompress via the
> return value and not touching errno at all.

I know you hate the use of ERRNO in libraries and tooling, but we've decided
to go that way for all of trace-cmd and the libraries (libtraceevent and
libtracefs). This is just being consistent with all the other callers, and
to do it differently will create an inconsistency in the API.

-- Steve
Sebastian Andrzej Siewior March 3, 2022, 7:09 a.m. UTC | #3
On 2022-03-02 10:52:30 [-0500], Steven Rostedt wrote:
> On Wed, 2 Mar 2022 08:15:30 +0100
> Sebastian Andrzej Siewior <sebastian@breakpoint.cc> wrote:
> 
> > I was thinking about returning the error for compress/decompress via the
> > return value and not touching errno at all.
> 
> I know you hate the use of ERRNO in libraries and tooling, but we've decided
> to go that way for all of trace-cmd and the libraries (libtraceevent and
> libtracefs). This is just being consistent with all the other callers, and
> to do it differently will create an inconsistency in the API.

Ah okay then. In that case, the zstd interface does not set errno in the
compress or decompress callback. This might need an update to be
consistent. Otherwise the return value is < 0 and errno might still be
0.

> -- Steve

Sebastian
diff mbox series

Patch

diff --git a/lib/trace-cmd/trace-compress-zlib.c b/lib/trace-cmd/trace-compress-zlib.c
index 41342597..0dfd4f15 100644
--- a/lib/trace-cmd/trace-compress-zlib.c
+++ b/lib/trace-cmd/trace-compress-zlib.c
@@ -33,6 +33,8 @@  static int zlib_compress(const void *in, int in_bytes, void *out, int out_bytes)
 	case Z_STREAM_ERROR:
 		errno = -EINVAL;
 		break;
+	case Z_ERRNO:
+		break;
 	default:
 		errno = -EFAULT;
 		break;
@@ -61,6 +63,8 @@  static int zlib_decompress(const void *in, int in_bytes, void *out, int out_byte
 	case Z_DATA_ERROR:
 		errno = -EINVAL;
 		break;
+	case Z_ERRNO:
+		break;
 	default:
 		errno = -EFAULT;
 		break;