Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [GIT PULL] fallthrough fixes for Clang for 5.14-rc1
To:     Linus Torvalds <torvalds@linux-foundation.org>,
        "Gustavo A. R. Silva" <gustavoars@kernel.org>
Cc:     Kees Cook <keescook@chromium.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
References: <20210628205947.GA10869@embeddedor>
 <CAHk-=wik-iqVYJyDxfPsOk_8vt1BrsKF-H-nunAYHw3HHuvZqA@mail.gmail.com>
From:   "Gustavo A. R. Silva" <gustavo@embeddedor.com>
Message-ID: <4a2ef94c-c5cb-c6d4-d36a-e6f5c23c076b@embeddedor.com>
Date:   Mon, 28 Jun 2021 22:46:28 -0500
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.11.0
MIME-Version: 1.0
In-Reply-To: <CAHk-=wik-iqVYJyDxfPsOk_8vt1BrsKF-H-nunAYHw3HHuvZqA@mail.gmail.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 6/28/21 22:12, Linus Torvalds wrote:
> On Mon, Jun 28, 2021 at 1:58 PM Gustavo A. R. Silva
> <gustavoars@kernel.org> wrote:
>>
>> Please, pull the following patches that fix many fall-through warnings
>> when building with Clang 12.0.0 and this[1] change reverted. Notice
>> that in order to enable -Wimplicit-fallthrough for Clang, such change[1]
>> is meant to be reverted at some point. So, these patches help to move
>> in that direction.
> 
> I've pulled this, but I really don't like how random it is.
> 
> Just as an example - and there are many others - look at the patch to
> net/netrom/nr_route.c.
> 
> It does
> 
>                 case 0:
>                         nr_node->routes[0] = nr_node->routes[1];
>                         fallthrough;
>                 case 1:
>                         nr_node->routes[1] = nr_node->routes[2];
> +                       fallthrough;
>                 case 2:
>                         break;
> 
> and then about a hundred lines later it does
>                 case 0:
>                         s->routes[0] = s->routes[1];
>                         fallthrough;
>                 case 1:
>                         s->routes[1] = s->routes[2];
> +                       break;
>                 case 2:
>                         break;
> 
> Notice? One does a 'fallthrough' to the next case that does the
> 'break', and the other - very much equivalent case - does a 'break'.
> 
> So the whole "add 'fallthrough' or 'break'" decision doesn't seem to
> have any pattern or rule at all.

I see, you're right.

I still have another PR with more of these fixes (the last ones before finally
being able to enable -Wimplicit-fallthrough for Clang :) ) pending to be sent
later this week. I'll double check and fix any similar issues before sending it.

Thanks for the feedback.
--
Gustavo