netdev - Re: [PATCH RESEND][next] rtl8xxxu: Fix fall-through warnings for Clang

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] [day] [month] [year] [list]

Message-ID: <55e04b5e-9ec5-6a9c-7381-024c4bf11c6d@gmail.com>
Date:   Mon, 19 Apr 2021 07:56:29 -0400
From:   Jes Sorensen <jes.sorensen@...il.com>
To:     Joe Perches <joe@...ches.com>, Kalle Valo <kvalo@...eaurora.org>,
        "Gustavo A. R. Silva" <gustavoars@...nel.org>
Cc:     "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        linux-wireless@...r.kernel.org, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-hardening@...r.kernel.org
Subject: Re: [PATCH RESEND][next] rtl8xxxu: Fix fall-through warnings for
 Clang

On 4/17/21 8:09 PM, Joe Perches wrote:
> On Sat, 2021-04-17 at 14:30 -0400, Jes Sorensen wrote:
>> On 4/17/21 1:52 PM, Kalle Valo wrote:
>>> "Gustavo A. R. Silva" <gustavoars@...nel.org> wrote:
>>>
>>>> In preparation to enable -Wimplicit-fallthrough for Clang, fix
>>>> multiple warnings by replacing /* fall through */ comments with
>>>> the new pseudo-keyword macro fallthrough; instead of letting the
>>>> code fall through to the next case.
>>>>
>>>> Notice that Clang doesn't recognize /* fall through */ comments as
>>>> implicit fall-through markings.
>>>>
>>>> Link: https://github.com/KSPP/linux/issues/115
>>>> Signed-off-by: Gustavo A. R. Silva <gustavoars@...nel.org>
>>>
>>> Patch applied to wireless-drivers-next.git, thanks.
>>>
>>> bf3365a856a1 rtl8xxxu: Fix fall-through warnings for Clang
>>>
>>
>> Sorry this junk patch should not have been applied.
> 
> I don't believe it's a junk patch.
> I believe your characterization of it as such is flawed.
> 
> You don't like the style, that's fine, but:
> 
> Any code in the kernel should not be a unique style of your own choosing
> when it could cause various compilers to emit unnecessary warnings.
> 
> Please remember the kernel code base is a formed by a community with a
> nominally generally accepted style.  There is a real desire in that
> community to both enable compiler warnings that might show defects and
> simultaneously avoid unnecessary compiler warnings.
> 
> This particular change just avoids a possible compiler warning.

Please go back and look at the thread. This patch fixes nothing, it
mutilates the code by introducing non C for the sole purpose of avoiding
to work with the Clang community to fix their broken compiler. The
author of this patch ignored previous feedback and just reposted the
same patch unaltered and when it was called out, the answer was other
people was fine with it. None of the issues raised have been addressed,
so yes, that makes the patch junk.

Jes