linux-kernel - Re: [GIT PULL] fallthrough fixes for Clang for 5.14-rc1

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4a2ef94c-c5cb-c6d4-d36a-e6f5c23c076b@embeddedor.com>
Date:   Mon, 28 Jun 2021 22:46:28 -0500
From:   "Gustavo A. R. Silva" <gustavo@...eddedor.com>
To:     Linus Torvalds <torvalds@...ux-foundation.org>,
        "Gustavo A. R. Silva" <gustavoars@...nel.org>
Cc:     Kees Cook <keescook@...omium.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [GIT PULL] fallthrough fixes for Clang for 5.14-rc1



On 6/28/21 22:12, Linus Torvalds wrote:
> On Mon, Jun 28, 2021 at 1:58 PM Gustavo A. R. Silva
> <gustavoars@...nel.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