Fix zlibWrapper build

[pstef]

Just after a clone I'm getting this:

~/zstd/zlibWrapper$ cc -c zstd_zlibwrapper.o gz*.c -lz -lzstd -DSTDC
gzwrite.c: In function ‘gz_write’:
gzwrite.c:226:43: error: ‘z_uInt’ undeclared (first use in this function); did you mean ‘uInt’?
  226 |             state.state->strm.avail_in = (z_uInt)n;
      |                                           ^~~~~~
      |                                           uInt
gzwrite.c:226:43: note: each undeclared identifier is reported only once for each function it appears in
gzwrite.c:226:50: error: expected ‘;’ before ‘n’
  226 |             state.state->strm.avail_in = (z_uInt)n;
      |                                                  ^
      |                                                  ;

z_uInt is never used directly, zconf.h redefines uInt to z_uInt under the condition that Z_PREFIX is set. All examples use uInt, and the type of avail_in is also uInt.

In this commit I modify the cast to refer to the same type as the type of lvalue.

Arguably, the real fix here is to handle possible overflows, but that's beyond the scope of this commit.

[facebook-github-bot]

Hi @pstef!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks!

[facebook-github-bot]

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks!

[Cyan4973]

Simple enough.

I wonder though why we did not observe this error in CI ?
Maybe we are lacking a test there.

edit :
tried the same command cc -c zstd_zlibwrapper.o gz*.c -lz -lzstd -DSTDC on a local system,
and got no such warning.
So I guess the described outcome of this test is not a given, and may depend on additional factors, such as, possibly, the zlib version, or the compiler version, or even the target platform.

[pstef]

I guess I'm right for the wrong reasons, probably.

I was almost ready to blame Fedora Rawhide or gcc-14, each of which is a development version at this time. But I also tested gcc-14 under Debian Testing and it doesn't produce the error, so it has to be either Rawhide's "fault" or maybe I did something weird to my system. I'll try to use gcc -H or -M and see if I can figure it out.

But at this point I stand by the conclusion that the cast should be modified as proposed here.

[pstef]

On my system, files /usr/include/{zlib,zconf}.h are provided by the package zlib-ng-compat-devel. zlib by design includes zconf. zconf usually handles Z_PREFIX and "reacts" accordingly, but the file on my system lacks this support. This is why I was getting the error.

So for the zlibWrapper example to be more portable, it should point people to invoke their compiler more or less like this:
cc -c zstd_zlibwrapper.o gz*.c -lz -lzstd -DSTDC -I.

And with -I., the -DSTDC doesn't seem required anymore.

[pstef]

@Cyan4973 I was preparing an update to the README file, adding just the -I., but I wonder if maybe the instructions should include providing also zconf.h on top of zlib.h? The way I see it, zlib.h will unconditionally include zconf.h and users might want to have control over where it's coming from, just like with zlib.h.

On the other hand, the instructions never say one should copy the files, only that they are not provided by this repository. So maybe instead of adding -I., it should be clarified whether they are supposed to be system headers, a local copy, or maybe it's a choice and the users should be made aware of it?

[Cyan4973]

I think the initial idea was that zlib would probably be provided as a system-level dependency, both for the library binary and the included header files.
But of course, anyone can setup its config differently, and manually select another source for zlib.
But at that point, I don't think it's the right place to describe here all the possible build combinations. It becomes a build topic, tied to the local build environment.