Re: [PATCH] clk: samsung: s3c2443: Mark expected switch fall-through
From: Kees Cook
Date: Tue Feb 12 2019 - 13:57:26 EST
On Mon, Feb 11, 2019 at 11:41 PM Krzysztof Kozlowski <krzk@xxxxxxxxxx> wrote:
> On Mon, 11 Feb 2019 at 19:40, Gustavo A. R. Silva
> <gustavo@xxxxxxxxxxxxxx> wrote:
> > In preparation to enabling -Wimplicit-fallthrough, mark switch
> > cases where we are expecting to fall through.
> > This patch fixes the following warnings:
> > drivers/clk/samsung/clk-s3c2443.c: In function âs3c2443_common_clk_initâ:
> > drivers/clk/samsung/clk-s3c2443.c:390:3: warning: this statement may fall through [-Wimplicit-fallthrough=]
> > samsung_clk_register_alias(ctx, s3c2450_aliases,
> > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > ARRAY_SIZE(s3c2450_aliases));
> > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > drivers/clk/samsung/clk-s3c2443.c:393:2: note: here
> > case S3C2416:
> > ^~~~
> > Warning level 3 was used: -Wimplicit-fallthrough=3
> > Notice that, in this particular case, the code comment is modified
> > in accordance with what GCC is expecting to find.
> > This patch is part of the ongoing efforts to enable
> > -Wimplicit-fallthrough.
> I saw this in multiple places already and I think fix is wrong. The
> point is that the code is correct - the fall through is marked
> It is just the GCC which has to be fixed not the code. You want to
> adjust the code for specific version of GCC and what if GCC changes
> its warning? For example GCC might require "fall through: "... or any
> other syntax. Another point - what about clang's syntax?
-Wimplicit-fallthrough=3 is stricter and maps to -Wextra, hence its
choice. GCC's levels were chosen based on the existing linters, static
analyzers, etc. The patterns are unlikely to change (see the gcc
Clang doesn't recognize anything in C mode (hopefully this will be
fixed in the future).
As long as one of the compilers is able to check this, we'll avoid the
bugs associated with this mis-pattern. Gustavo's efforts here have
found kind of a lot of bugs, so I think it's worth a little churn to
add these (and make minor adjustments to existing comments).