Hi Dmitry,
On Fri, Nov 03, 2017 at 03:11:21PM +0300, Dmitry Sivachenko wrote:
> Hello,
>
> several new warnings from clang, some look meaningful:
Thanks, Olivier also reported some of them. Some are valid or easy
to address, others might need some -Wno-something I guess.
> src/server.c:875:14: warning: address of array 'check->desc' will always
> evaluate to 'true' [-Wpointer-bool-conversion]
> if (check->desc)
> ~~ ~~~~~~~^~~~
This one needs to be fixed, I think we moved from a pointer to a char[]
and the test is not needed anymore.
> src/cfgparse.c:5044:34: warning: implicit conversion from 'int' to 'char'
> changes value from 130 to -126 [-Wconstant-conversion]
> ...curproxy->check_req[5] = 130;
> ~ ^~~
> src/cfgparse.c:5070:33: warning: implicit conversion from 'int' to 'char'
> changes value from 128 to -128 [-Wconstant-conversion]
> ...curproxy->check_req[5] = 128;
> ~ ^~~
We've seen this one the other way around in the past, you assing a negative
number to shut it up and it complains on arm where chars are unsigned. I
*seem* to remember that it doesn't complain when entered in hexadecimal
form though, we'll have to try (if you want to, feel free to send a patch
if it works using 0x82 and 0x80).
> cc -Iinclude -Iebtree -Wall -O2 -pipe -fstack-protector
> -fno-strict-aliasing -fno-strict-aliasing -Wdeclaration-after-statement
> -fwrapv -Wno-address-of-packed-member -Wno-null-dereference
> -Wno-unused-label -DFREEBSD_PORTS -DTPROXY -DCONFIG_HAP_CRYPT
> -DUSE_GETADDRINFO -DUSE_ZLIB -DENABLE_POLL -DENABLE_KQUEUE
> -DUSE_CPU_AFFINITY -DUSE_ACCEPT4 -DCONFIG_REGPARM=3 -DUSE_THREAD
> -DUSE_OPENSSL -DUSE_PCRE -I/usr/local/include -DUSE_PCRE_JIT
> -DCONFIG_HAPROXY_VERSION=\"1.8-rc1-901f75c\"
> -DCONFIG_HAPROXY_DATE=\"2017/10/31\" -c -o src/sample.o src/sample.c
> src/peers.c:255:16: warning: implicit conversion from 'int' to 'char' changes
> value from 133 to -123 [-Wconstant-conversion]
> *msg_type = PEER_MSG_STKT_UPDATE_TIMED;
> ~ ^~~~~~~~~~~~~~~~~~~~~~~~~~
Same here, though this one is an enum and I would hate to have to start
to use casts for this, casts are only made to secretly hide bugs :-(
I think the pointer could be an unsigned char though. We'll have to
check if it yells somewhere else.
> cc -Iinclude -Iebtree -Wall -O2 -pipe -fstack-protector
> -fno-strict-aliasing -fno-strict-aliasing -Wdeclaration-after-statement
> -fwrapv -Wno-address-of-packed-member -Wno-null-dereference
> -Wno-unused-label -DFREEBSD_PORTS -DTPROXY -DCONFIG_HAP_CRYPT
> -DUSE_GETADDRINFO -DUSE_ZLIB -DENABLE_POLL -DENABLE_KQUEUE
> -DUSE_CPU_AFFINITY -DUSE_ACCEPT4 -DCONFIG_REGPARM=3 -DUSE_THREAD
> -DUSE_OPENSSL -DUSE_PCRE -I/usr/local/include -DUSE_PCRE_JIT
> -DCONFIG_HAPROXY_VERSION=\"1.8-rc1-901f75c\"
> -DCONFIG_HAPROXY_DATE=\"2017/10/31\" -c -o src/freq_ctr.o src/freq_ctr.c
> src/mux_h2.c:1734:15: warning: implicit conversion from enumeration type
> 'enum h2_ss' to different enumeration type 'enum h2_cs'
> [-Wenum-conversion]
> h2c->st0 = H2_SS_ERROR;
> ~ ^~~~~~~~~~~
I'm aware (and ashamed) of these ones already, I need to understand what I
wanted to set to know how to fix it (currently dealing with a much uglier
one).
> src/mux_h2.c:526:24: warning: unused function 'h2_get_n64' [-Wunused-function]
Ah now it starts to report this also for inlines ? It'll get pretty annoying
then as it prevents one from creating all the useful tools at once for later
use :-/ No idea how to shut up this one, maybe in the long term we'll also
need -Wno-unused-function.
> src/cache.c:176:7: warning: variable 'ret' is used uninitialized whenever 'if'
> condition is false [-Wsometimes-uninitialized]
> if (len) {
> ^~~
> src/cache.c:202:7: note: uninitialized use occurs here
> if ((ret != len) ||
> ^~~
> src/cache.c:176:3: note: remove the 'if' if its condition is always true
> if (len) {
> ^~~~~~~~~
> src/cache.c:151:9: note: initialize the variable 'ret' to silence this warning
> int ret;
> ^
> = 0
Thanks, these ones may indeed be useful.
> src/cache.c:566:9: warning: implicit conversion from enumeration type
> 'enum act_return' to different enumeration type 'enum act_parse_ret'
> [-Wenum-conversion]
> return ACT_RET_ERR;
> ~~~~~~ ^~~~~~~~~~~
I noted these ones as well.
> In file included from ../../ebtree/ebtree.c:21:
> ../../ebtree/ebtree.h:469:35: warning: taking address of packed member
> 'branches' of class or structure 'eb_node' may result in an unaligned
> pointer value [-Waddress-of-packed-member]
> eb_troot_t *new_left = eb_dotag(&new->branches, EB_LEFT);
> ^~~~~~~~~~~~~
> (a lot of similar)
Olivier reported them. It's a real pain because it's made like this
on purpose and as usual C language developers are trying to kill C
usage around them by making the language unusable for what it was made
for : manipulating data in a portable way :-( This warning is so much
ridiculous as the so called unaligned pointer is the first member of
the structure! I guess we'll have to hide this one as well but it's
annoying for anyone using ebtree :-/
Thanks very much!
Willy
