Hi Tim,
On Fri, May 29, 2020 at 02:35:49PM +0200, Tim Duesterhus wrote:
> Hi there,
>
> These two patches re-enable some build warnings in the Makefile. The first one
> only enables those that did not trigger for me at all.
OK, I agree with not removing warnings that don't trigger.
> The second one re-enables -Wimplicit-fallthrough. I could not adjust all
> places, because I was not sure whether they work as intended.
>
> I've Cc't Willy (stream_interface, pattern) and William (cli), because they
> where the ones that wrote the switch statements. I've also added Ilya, because
> applying the second patch will most likely fail the Travis build.
Indeed if there are so few left, it might be worth trying to address
them and re-enabling the warning.
> I'd say that the one in 'pattern' is a bug and that the other two just need a
> /* fall through */ comment.
No opinion yet since I've not read that code for a while. However I'd like
the code to be fixed before we enable the warning, as I absolutely don't
want to see a build warning during regular development. We encourage
building with -Werror and this would force developers to remove it which
would be a huge step backwards!
That reminds me, I seem to have noticed some warnings a few months ago
when building for i386, maybe something related to printf formats having
%l when ints were used instead. It's been a long time and I don't even
remember if we've fixed that. It just happens to be the right period in
the development cycle to start to address such things. The cleaner the
better.
Thanks!
Willy
> See the build log below for what I'm seeing within a debian:sid Docker
> container:
>
> root@0292bc6019cc:/pwd# make -j4 all TARGET=linux-glibc USE_OPENSSL=1
> USE_LUA=1 USE_ZLIB=1 USE_PCRE2=1 USE_PCRE2_JIT=1 USE_GETADDRINFO=1
> CC src/ev_poll.o
> CC src/ev_epoll.o
> CC src/ssl_sample.o
> CC src/ssl_sock.o
> CC src/ssl_crtlist.o
> CC src/ssl_ckch.o
> CC src/ssl_utils.o
> CC src/cfgparse-ssl.o
> CC src/hlua.o
> CC src/hlua_fcn.o
> CC src/namespace.o
> CC src/mux_h2.o
> CC src/stream.o
> CC src/mux_fcgi.o
> CC src/cfgparse-listen.o
> CC src/http_ana.o
> CC src/stats.o
> CC src/mux_h1.o
> CC src/flt_spoe.o
> CC src/server.o
> CC src/cfgparse.o
> CC src/checks.o
> CC src/backend.o
> CC src/log.o
> CC src/peers.o
> CC src/cli.o
> src/cli.c: In function 'cli_io_handler_show_cli_sock':
> src/cli.c:1200:16: warning: this statement may fall through
> [-Wimplicit-fallthrough=]
> 1200 | appctx->st2 = STAT_ST_LIST;
> | ~~~~~~~~~~~~^~~~~~~~~~~~~~
> src/cli.c:1202:3: note: here
> 1202 | case STAT_ST_LIST:
> | ^~~~
> src/cli.c:1203:7: warning: this statement may fall through
> [-Wimplicit-fallthrough=]
> 1203 | if (global.stats_fe) {
> | ^
> src/cli.c:1277:3: note: here
> 1277 | default:
> | ^~~~~~~
> CC src/haproxy.o
> CC src/stick_table.o
> CC src/standard.o
> CC src/sample.o
> CC src/proxy.o
> CC src/stream_interface.o
> src/stream_interface.c: In function 'stream_int_shutw':
> src/stream_interface.c:238:13: warning: this statement may fall through
> [-Wimplicit-fallthrough=]
> 238 | si->state = SI_ST_DIS;
> | ~~~~~~~~~~^~~~~~~~~~~
> src/stream_interface.c:239:2: note: here
> 239 | default:
> | ^~~~~~~
> src/stream_interface.c: In function 'stream_int_shutw_applet':
> src/stream_interface.c:1673:13: warning: this statement may fall through
> [-Wimplicit-fallthrough=]
> 1673 | si->state = SI_ST_DIS;
> | ~~~~~~~~~~^~~~~~~~~~~
> src/stream_interface.c:1674:2: note: here
> 1674 | default:
> | ^~~~~~~
> CC src/pattern.o
> CC src/dns.o
> src/pattern.c: In function 'pattern_exec_match':
> src/pattern.c:2563:68: warning: this statement may fall through
> [-Wimplicit-fallthrough=]
> 2563 | static_sample_data.u.str.area[static_sample_data.u.str.data] =
> 0;
> | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~
> src/pattern.c:2564:6: note: here
> 2564 | case SMP_T_IPV4:
> | ^~~~
> src/pattern.c:2567:7: warning: this statement may fall through
> [-Wimplicit-fallthrough=]
> 2567 | memcpy(&static_sample_data, pat->data, sizeof(struct
> sample_data));
> |
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> src/pattern.c:2568:6: note: here
> 2568 | default:
> | ^~~~~~~
> CC src/proto_tcp.o
> CC src/listener.o
> CC src/cfgparse-global.o
> CC src/h1.o
> CC src/http_rules.o
> CC src/http_fetch.o
> CC src/cache.o
> CC src/session.o
> CC src/fcgi-app.o
> CC src/connection.o
> CC src/tcp_rules.o
> CC src/filters.o
> CC src/task.o
> CC src/mworker.o
> CC src/map.o
> CC src/h1_htx.o
> CC src/trace.o
> CC src/flt_trace.o
> CC src/acl.o
> CC src/http_htx.o
> CC src/flt_http_comp.o
> CC src/payload.o
> CC src/vars.o
> CC src/debug.o
> CC src/mux_pt.o
> CC src/http_act.o
> CC src/h2.o
> CC src/queue.o
> CC src/fd.o
> CC src/proto_uxst.o
> CC src/lb_chash.o
> CC src/ring.o
> CC src/frontend.o
> CC src/raw_sock.o
> CC src/xprt_handshake.o
> CC src/htx.o
> CC src/memory.o
> CC src/applet.o
> CC src/channel.o
> CC src/signal.o
> CC src/lb_fwrr.o
> CC src/ev_select.o
> CC src/sink.o
> CC src/http_conv.o
> CC src/proto_sockpair.o
> CC src/mworker-prog.o
> CC src/activity.o
> CC src/lb_fwlc.o
> CC src/http.o
> CC src/lb_fas.o
> CC src/uri_auth.o
> CC src/hathreads.o
> CC src/regex.o
> CC src/auth.o
> CC src/buffer.o
> CC src/compression.o
> CC src/proto_udp.o
> CC src/lb_map.o
> CC src/chunk.o
> CC src/wdt.o
> CC src/hpack-dec.o
> CC src/action.o
> CC src/xxhash.o
> CC src/pipe.o
> CC src/shctx.o
> CC src/hpack-tbl.o
> CC src/http_acl.o
> CC src/sha1.o
> CC src/time.o
> CC src/hpack-enc.o
> CC src/fcgi.o
> CC src/arg.o
> CC src/base64.o
> CC src/protocol.o
> CC src/freq_ctr.o
> CC src/lru.o
> CC src/hpack-huff.o
> CC src/dict.o
> CC src/hash.o
> CC src/mailers.o
> CC src/version.o
> CC ebtree/ebtree.o
> CC ebtree/eb32sctree.o
> CC ebtree/eb32tree.o
> CC ebtree/eb64tree.o
> CC ebtree/ebmbtree.o
> CC ebtree/ebsttree.o
> CC ebtree/ebimtree.o
> CC ebtree/ebistree.o
> LD haproxy
>
> Tim Düsterhus (2):
> BUILD: Remove nowarn for warnings that do not trigger
> BUILD: Re-enable -Wimplicit-fallthrough
>
> Makefile | 3 ---
> src/acl.c | 2 ++
> src/checks.c | 4 ++--
> src/hlua.c | 2 ++
> src/peers.c | 8 ++++----
> 5 files changed, 10 insertions(+), 9 deletions(-)
>
> --
> 2.26.2
