Walkthrough

The update modifies the function signatures in the zlib-ng.h.in header, replacing all int32_t and uint32_t types with the standard C types int and unsigned for return values and parameters in various compression, decompression, and gzip-related APIs. No logic or control flow changes are present.

Changes

Suggested labels

bug

Suggested reviewers

• Dead2
• nmoinvaz

Learnt from: mtl1979
PR: zlib-ng/zlib-ng#1925
File: arch/loongarch/lasxintrin_ext.h:38-65
Timestamp: 2025-07-04T16:59:44.683Z
Learning: In zlib-ng, the policy for supporting old compilers is to maintain compatibility for up to two years after GitHub removes support for them in CI, or when there is no long-time contributor left testing for the architecture if the architecture was not supported by GitHub Actions at all.

Learnt from: samrussell
PR: zlib-ng/zlib-ng#1914
File: arch/generic/crc32_chorba_c.c:30-32
Timestamp: 2025-05-13T06:56:12.407Z
Learning: In zlib-ng, pointer casts between z_word_t* and uint64_t* are safe because: 1) On 64-bit platforms, z_word_t is defined as uint64_t, making the cast a no-op; 2) On 32-bit platforms where z_word_t is uint32_t, the code paths using uint64_t* are skipped in favor of 32-bit specific implementations.

Learnt from: pps83
PR: zlib-ng/zlib-ng#0
File: :0-0
Timestamp: 2025-01-13T18:28:11.751Z
Learning: In zlib-ng, removing preprocessor guards (like HAVE_*) should be described as removing unused conditional compilation checks rather than removing functionality, as the underlying implementation often remains intact.

Learnt from: KungFuJesus
PR: zlib-ng/zlib-ng#1872
File: arch/x86/chorba_sse2.c:0-0
Timestamp: 2025-02-18T02:53:23.621Z
Learning: In zlib-ng, when macro names and parameters are self-explanatory and scalar versions of the code provide context, additional documentation for the macros may not be necessary.

Learnt from: KungFuJesus
PR: zlib-ng/zlib-ng#1872
File: arch/x86/chorba_sse2.c:0-0
Timestamp: 2025-02-23T16:49:52.043Z
Learning: In zlib-ng, bounds checking for CRC32 computation is handled by the caller, not within the individual CRC32 implementation functions like `crc32_chorba_sse2`.

Learnt from: phprus
PR: zlib-ng/zlib-ng#1925
File: arch/loongarch/slide_hash_lsx.c:64-64
Timestamp: 2025-06-18T19:28:32.987Z
Learning: In zlib-ng's slide_hash functions, when calling slide_hash_chain(), the fourth parameter should be the window size (wsize) representing the number of entries in the prev table, not s->w_size directly. The local variable wsize is typically cast to the appropriate type (uint16_t) from s->w_size for this purpose.

Learnt from: KungFuJesus
PR: zlib-ng/zlib-ng#1872
File: arch/x86/chorba_sse2.c:14-24
Timestamp: 2025-02-21T01:41:50.358Z
Learning: In zlib-ng's SSE2 vectorized Chorba CRC implementation, the code that calls READ_NEXT macro ensures 16-byte alignment, making explicit alignment checks unnecessary within the macro.

Learnt from: Dead2
PR: zlib-ng/zlib-ng#1928
File: gzlib.c:90-90
Timestamp: 2025-07-04T20:17:58.504Z
Learning: In zlib-ng's gz_buffer_alloc() function, the condition `(out_mult >= 1 && state->out == NULL)` is necessary because state->out is intentionally left as NULL when out_mult == 0 (write + direct mode), but should be allocated when out_mult >= 1. The condition properly distinguishes between intentionally NULL buffers and allocation failures.

Learnt from: KungFuJesus
PR: zlib-ng/zlib-ng#1872
File: arch/x86/chorba_sse2.c:0-0
Timestamp: 2025-02-18T02:53:23.621Z
Learning: In zlib-ng, when macro names and parameters are self-explanatory and scalar versions of the code provide context, additional documentation for the macros may not be necessary.

Learnt from: pps83
PR: zlib-ng/zlib-ng#0
File: :0-0
Timestamp: 2025-01-13T18:28:11.751Z
Learning: In zlib-ng, removing preprocessor guards (like HAVE_*) should be described as removing unused conditional compilation checks rather than removing functionality, as the underlying implementation often remains intact.

Learnt from: samrussell
PR: zlib-ng/zlib-ng#1914
File: arch/generic/crc32_chorba_c.c:30-32
Timestamp: 2025-05-13T06:56:12.407Z
Learning: In zlib-ng, pointer casts between z_word_t* and uint64_t* are safe because: 1) On 64-bit platforms, z_word_t is defined as uint64_t, making the cast a no-op; 2) On 32-bit platforms where z_word_t is uint32_t, the code paths using uint64_t* are skipped in favor of 32-bit specific implementations.

Learnt from: mtl1979
PR: zlib-ng/zlib-ng#1925
File: arch/loongarch/lasxintrin_ext.h:38-65
Timestamp: 2025-07-04T16:59:44.683Z
Learning: In zlib-ng, the policy for supporting old compilers is to maintain compatibility for up to two years after GitHub removes support for them in CI, or when there is no long-time contributor left testing for the architecture if the architecture was not supported by GitHub Actions at all.

Learnt from: phprus
PR: zlib-ng/zlib-ng#1925
File: arch/loongarch/slide_hash_lsx.c:64-64
Timestamp: 2025-06-18T19:28:32.987Z
Learning: In zlib-ng's slide_hash functions, when calling slide_hash_chain(), the fourth parameter should be the window size (wsize) representing the number of entries in the prev table, not s->w_size directly. The local variable wsize is typically cast to the appropriate type (uint16_t) from s->w_size for this purpose.

Learnt from: Dead2
PR: zlib-ng/zlib-ng#1928
File: gzlib.c:90-90
Timestamp: 2025-07-04T20:17:58.504Z
Learning: In zlib-ng's gz_buffer_alloc() function, the condition `(out_mult >= 1 && state->out == NULL)` is necessary because state->out is intentionally left as NULL when out_mult == 0 (write + direct mode), but should be allocated when out_mult >= 1. The condition properly distinguishes between intentionally NULL buffers and allocation failures.

Learnt from: KungFuJesus
PR: zlib-ng/zlib-ng#1872
File: arch/x86/chorba_sse2.c:0-0
Timestamp: 2025-02-23T16:49:52.043Z
Learning: In zlib-ng, bounds checking for CRC32 computation is handled by the caller, not within the individual CRC32 implementation functions like `crc32_chorba_sse2`.

Learnt from: KungFuJesus
PR: zlib-ng/zlib-ng#1872
File: arch/x86/chorba_sse2.c:14-24
Timestamp: 2025-02-21T01:41:50.358Z
Learning: In zlib-ng's SSE2 vectorized Chorba CRC implementation, the code that calls READ_NEXT macro ensures 16-byte alignment, making explicit alignment checks unnecessary within the macro.

Learnt from: Dead2
PR: zlib-ng/zlib-ng#1837
File: arch/generic/crc32_c.c:19-29
Timestamp: 2025-01-23T22:01:53.422Z
Learning: The Chorba CRC32 functions (crc32_chorba_118960_nondestructive, crc32_chorba_32768_nondestructive, crc32_chorba_small_nondestructive, crc32_chorba_small_nondestructive_32bit) are declared in crc32_c.h.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.67%. Comparing base (f891bd3) to head (192783f).
⚠️ Report is 64 commits behind head on develop.

@@             Coverage Diff              @@
##           develop    #1934       +/-   ##
============================================
+ Coverage    39.39%   81.67%   +42.28%     
============================================
  Files           74      150       +76     
  Lines         7885    13618     +5733     
  Branches      1303     3052     +1749     
============================================
+ Hits          3106    11123     +8017     
+ Misses        4536     1547     -2989     
- Partials       243      948      +705     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

So both int and long are 32-bit, is that right?
I fail to see why the application using zlib-ng cannot use int32_t like we define, does it not have stdint.h or something?

I don't see the connection between your description and the changes you propose, so please elaborate.

Also, a feature of zlib-ng is that it uses stdint datatypes so that they are always the same size on any platform, unlike int and long that can vary.

This PR fixes type mismatches between function declarations and definitions:

We currently do not support Newlib, so it is pointless to try fixing any issues with it... We however already do use preprocessor magic to hide differences between declarations of some types in toolchains, but proper support needs configure time checks.

There is already different types for compat and non-compat builds and that is intended to avoid overflows on 64-bit platforms.

@mtl1979

I think this is not just for Newlib support.
This is a fix for the differences between function declarations and definitions.

I think that the functions themselves should instead be changed to use int32_t, as that would be in line with what we are trying to do in zlib-ng and would not change the the zlib-ng native api.
I do agree that it is a bug that the function declarations and definitions mismatch, although the difference is nil in most cases. If int != int32_t then the header will say 32bit while the actual function is whatever int might be on that platform (16bit, 32bit or possibly even 64 bit).

We might need to use a custom type though, because we still need the functions to use int in compat mode and int32_t in the native mode. The alternative to that is to use wrapper functions, but I think that would be much more complicated.

I still don't know whether that would fix the problems with Newlib though, since the problem description is unclear on the specifics.

We might need to use a custom type though, because we still need the functions to use int in compat mode and int32_t in the native mode. The alternative to that is to use wrapper functions, but I think that would be much more complicated.

The main issue with using custom type is that standard C doesn't allow overloading function parameters, so using custom type would break zlib compatibility. GNU extension for C does however allow overloading function parameters only when the types are different size.

We could use preprocessor magic using ZLIB_COMPAT to use two different function declarations, like we have done before. This means duplicating everything before first {, but that will add 4 extra lines per function declaration and hurt readability a little.

The main issue with using custom type is that standard C doesn't allow overloading function parameters, so using custom type would break zlib compatibility. GNU extension for C does however allow overloading function parameters only when the types are different size.

Are you saying this would not work? Changing the functions themselves to use zint, then use int in zlib.h and uint32_t in zlib-ng.h.

#  if defined(ZLIB_COMPAT)
    typedef int zint;
#  else
    typedef uint32_t zint;
#  endif

If that does not work, it should still be possible by using #define zint int etc.

That does not address any problems in the functions with platforms where int is not actually 32-bit of course.

IMO the long-term solution is to make a compat-shim, in effect a compat library that loads the native library and just has shim functions for the entire zlib library, and only has the required checks and data-type translation that we do not need in native.
This has been on my TODO list for a long time, is is not really hard to do, just takes a while. It would let us get rid of all the PREFIX stuff as well, so the code would become a lot cleaner.

The main issue with using custom type is that standard C doesn't allow overloading function parameters, so using custom type would break zlib compatibility. GNU extension for C does however allow overloading function parameters only when the types are different size.

Are you saying this would not work? Changing the functions themselves to use zint, then use int in zlib.h and uint32_t in zlib-ng.h.

I'm saying the fix has to go to the *.c files, not the header files as changing header files would change the ABI (and cause CI to fail). This is obviously because the compiler doesn't load header files separately, but instead included from the source files. For proper fix we would need to have Newlib on our CI workflows. There was already several issues in stock zlib if long isn't the same bit width as a pointer, as is the case also with 64-bit Windows.

Using shim functions works in release builds as long as there is no indirect jumps. When using shim functions, it doesn't matter if int is 32 bits or less or long is 64 bits or less as the shim function must always handle any truncating or overflow protection.

@mtl1979

I think this is not just for Newlib support. This is a fix for the differences between function declarations and definitions.

I agree, but in most cases using long instead of int or long long will cause worse issues than trying to avoid int32_t or uint32_t. Same issue did exist already in stock zlib, so we have to be careful to not break compatibility with either it or our own API, which has less issues.

I just encountered this issue while trying to compile for 32-bit armv7, and int32_t is defined as long int, and I do not use newlib or anything but the standard libc and libstdc++ from gcc. I heavily support this being merged.

I just encountered this issue while trying to compile for 32-bit armv7, and int32_t is defined as long int, and I do not use newlib or anything but the standard libc and libstdc++ from gcc. I heavily support this being merged.

This PR needs complete rewrite as in current form it is unacceptable... As per contributing guidelines of zlib-ng, original author must either abandon the PR before someone else can rewrite it, or rewrite it oneself.

when looking into some warnings from zlib I saw it was using int32_t for its structures/functions.
I find it weird. Should just use int and static assert for any platform with 16bit ints

when looking into some warnings from zlib I saw it was using int32_t for its structures/functions.
I find it weird. Should just use int and static assert for any platform with 16bit ints

This is by convention... Using explicit integer widths allows optimising for minimal structure padding. Biggest issue with original zlib was to assume width of long type is always same as width of a pointer, which caused overflow on Windows and few other operating systems where int and long are same width... Static asserts were not part of C standard, so they are not portable.

What comes to function parameters, passing pointers of different size is not allowed, so using explicit integer width avoids compiler errors while being compatible with both original zlib API and zlib-ng's API.

When we started, types like size_t and ssize_t were not standardised yet across compilers, so with even different compiler command-line parameters one could get different integer width.

IMO the long-term solution is to make a compat-shim, in effect a compat library that loads the native library and just has shim functions for the entire zlib library, and only has the required checks and data-type translation that we do not need in native.

@Dead2, for shared libraries that would mean two dynamic loaded library files, correct? I wonder if that would be acceptable.

@Dead2, for shared libraries that would mean two dynamic loaded library files, correct? I wonder if that would be acceptable.

Just the different SONAME is enough to require two shared libraries. Then need to make sure all symbol names are unique, as linker can bypass the compat shim if it wants due to strict symbol lookup order. Compat shim has to come before other shared libraries and no static library can use same symbol names.

Closing this in favor of #1980