madler / zlib

A massively spiffy yet delicately unobtrusive compression library.
http://zlib.net/
Other
5.46k stars 2.41k forks source link

Request to Clear 'cast-qual' Compiler Warnings #939

Closed Lwisce closed 4 months ago

Lwisce commented 4 months ago

Hello. I noticed that there are 'cast-qual' compiler warnings in the code. Would it be possible for me to clear these warnings? I'd like to submit a patch to address them.

In Inflate.c:645 (1.3.1 version) the code is: *strm->msg = (char)"incorrect header check";*

The example above will trigger a 'cast-qual' warning in my compiler (gcc-c++14), I anticipate resolving this warning by adding 'z_const' before (char*) like this: *strm->msg = (z_const char)"incorrect header check";*

How do you think about it?

madler commented 4 months ago

You need to compile with Z_CONST defined. ./configure -w --const will do that, and will enable the cast-qual warning. With that setting there will be no warnings. Without Z_CONST, you simply should not enable, or should disable, the cast-qual warning. Such warnings cannot be avoided with the original zlib interface structure with no const's. Z_CONST is not the default because it can be incompatible with code that relies on no const's in the z_stream structure.

(By the way, you left out the asterisks in the types.)

Lwisce commented 4 months ago

By define z_const doesn't clear that warning. I believe the warning arises from casting const char* to char*. My proposal is to use (z_const char*) instead of (char*) for the conversion. This won't affect the original code, and as long as I define z_const, the warning should be cleared.

madler commented 4 months ago

I can't make any sense out of what you wrote. You say defining z_const doesn't clear the warning, and then you say defining z_const clears the warning. And what do you mean "should be cleared"? You didn't even try it?

In any case, I get no warning there with -Wcast-qual without Z_CONST with gcc or clang. The warnings are elsewere. There are no warnings with -Wcast-qual when you define Z_CONST or use ./configure -w --const.

Lwisce commented 4 months ago

I apologize for the lack of clarity in my statement.

Let me show what warnings I get when I compile with Z_CONST defined:

../../../../utilities/zlib/lib/inffast.c: In function ‘inflate_fast’: ../../../../utilities/zlib/lib/inffast.c:172:33: error: cast discards ‘const’ qualifier from pointer target type [-Werror=cast-qual] 172 | (char )"invalid distance too far back"; | ^ ../../../../utilities/zlib/lib/inffast.c:268:29: error: cast discards ‘const’ qualifier from pointer target type [-Werror=cast-qual] 268 | strm->msg = (char )"invalid distance code"; | ^ ../../../../utilities/zlib/lib/inffast.c:283:25: error: cast discards ‘const’ qualifier from pointer target type [-Werror=cast-qual] 283 | strm->msg = (char *)"invalid literal/length code"; | ^

What I meaned is I want to modify the code from strm->msg = (char *)"invalid distance code"; to strm->msg = (z_const char *)"invalid distance code"; to claen the warning I got.

madler commented 4 months ago

What compiler and version is this on? I do not see these warnings.

Lwisce commented 4 months ago

Here is the complete compiler information.

Using built-in specs. COLLECT_GCC=gcc COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-linux-gnu/10/lto-wrapper OFFLOAD_TARGET_NAMES=nvptx-none:amdgcn-amdhsa:hsa OFFLOAD_TARGET_DEFAULT=1 Target: x86_64-linux-gnu Configured with: ../src/configure -v --with-pkgversion='Ubuntu 10.5.0-1ubuntu1~20.04' --with-bugurl=file:///usr/share/doc/gcc-10/README.Bugs --enable-languages=c,ada,c++,go,brig,d,fortran,objc,obj-c++,m2 --prefix=/usr --with-gcc-major-version-only --program-suffix=-10 --program-prefix=x86_64-linux-gnu- --enable-shared --enable-linker-build-id --libexecdir=/usr/lib --without-included-gettext --enable-threads=posix --libdir=/usr/lib --enable-nls --enable-bootstrap --enable-clocale=gnu --enable-libstdcxx-debug --enable-libstdcxx-time=yes --with-default-libstdcxx-abi=new --enable-gnu-unique-object --disable-vtable-verify --enable-plugin --enable-default-pie --with-system-zlib --enable-libphobos-checking=release --with-target-system-zlib=auto --enable-objc-gc=auto --enable-multiarch --disable-werror --with-arch-32=i686 --with-abi=m64 --with-multilib-list=m32,m64,mx32 --enable-multilib --with-tune=generic --enable-offload-targets=nvptx-none=/build/gcc-10-g5VkgL/gcc-10-10.5.0/debian/tmp-nvptx/usr,amdgcn-amdhsa=/build/gcc-10-g5VkgL/gcc-10-10.5.0/debian/tmp-gcn/usr,hsa --without-cuda-driver --enable-checking=release --build=x86_64-linux-gnu --host=x86_64-linux-gnu --target=x86_64-linux-gnu --with-build-config=bootstrap-lto-lean --enable-link-mutex Thread model: posix Supported LTO compression algorithms: zlib zstd

gcc version 10.5.0 (Ubuntu 10.5.0-1ubuntu1~20.04)

It's worth mentioning that I used my own build instead of zlib's build.

madler commented 4 months ago

What do you get with ./configure -w --const and make test from an unmodified zlib 1.3.1?

Lwisce commented 4 months ago

I got no warnings with ./configure -w --constand make test from an unmodified zlib 1.3.1.

Lwisce commented 4 months ago

I searched online and try some tests, and I found that if using g++ instead of gcc can get the warning with ./configure -w --const The command I used was: ./configure -w --const make CC=g++ test And I got:

infback.c:307:29: warning: cast from type 'const char' to type 'char' casts away qualifiers [-Wcast-qual] 307 | strm->msg = (char )"invalid block type"; | ^~~~~~~~ infback.c:318:29: warning: cast from type 'const char' to type 'char' casts away qualifiers [-Wcast-qual] 318 | strm->msg = (char )"invalid stored block lengths"; | ^~~~~~~~~~ infback.c:356:29: warning: cast from type 'const char' to type 'char' casts away qualifiers [-Wcast-qual] 356 | strm->msg = (char )"too many length or distance symbols"; | ^~~~~~~~~~~~~ infback.c:378:29: warning: cast from type 'const char' to type 'char' casts away qualifiers [-Wcast-qual] 378 | strm->msg = (char )"invalid code lengths set";

Lwisce commented 4 months ago

I looked through the standards of C and C++, and found the issue lies in the inconsistency between the standards. In C standard, writing it like (char*)"Hello World" is not a problem. But in C++ standard, standard before C++14 will report a warning, and after C++14 will report an error. We shouldn't use g++ to compile C files, so this isn't a problem. Please disregard this.

madler commented 4 months ago

Thank you.

Lwisce commented 4 months ago

I'm sorry for the mistake in my judgment. I just conducted another test and found that gcccan also report this warning! To find this warning requires a modification to the configure script first:

@@ -207,7 +207,7 @@ if test "$gcc" -eq 1 && ($cc -c $test.c) >> configure.log 2>&1; then fi if test "$warn" -eq 1; then if test "$zconst" -eq 1; then - CFLAGS="${CFLAGS} -Wall -Wextra -Wcast-qual -DZLIB_CONST" + CFLAGS="${CFLAGS} -Wall -Wextra -Wcast-qual -Wwrite-strings -DZLIB_CONST" else CFLAGS="${CFLAGS} -Wall -Wextra" fi

And then after ./configure -w --const and make test, I got:

infback.c: In function ‘inflateBack’: infback.c:307:29: warning: cast discards ‘const’ qualifier from pointer target type [-Wcast-qual] 307 | strm->msg = (char )"invalid block type"; | ^ infback.c:318:29: warning: cast discards ‘const’ qualifier from pointer target type [-Wcast-qual] 318 | strm->msg = (char )"invalid stored block lengths"; | ^ infback.c:356:29: warning: cast discards ‘const’ qualifier from pointer target type [-Wcast-qual] 356 | strm->msg = (char )"too many length or distance symbols"; | ^ infback.c:378:29: warning: cast discards ‘const’ qualifier from pointer target type [-Wcast-qual] 378 | strm->msg = (char )"invalid code lengths set"; | ^ infback.c:401:41: warning: cast discards ‘const’ qualifier from pointer target type [-Wcast-qual] 401 | strm->msg = (char *)"invalid bit length repeat"; | ^

I have reproduced this warning in multiple versions of GCC.

Lwisce commented 4 months ago

I have found more informatio,provide to you for your reference.

-Wwrite-strings When compiling C, give string constants the type const char[length] so that copying the address of one into a non-const char * pointer produces a warning. These warnings help you find at compile time code that can try to write into a string constant, but only if you have been very careful about using const in declarations and prototypes. Otherwise, it is just a nuisance. This is why we did not make -Wall request these warnings. When compiling C++, warn about the deprecated conversion from string literals to char *. This warning is enabled by default for C++ programs. This warning is upgraded to an error by -pedantic-errors in C++11 mode or later.

And I got a link of some people disscuss about it : https://stackoverflow.com/a/59687/22106624