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: <03028798-d42c-d864-8c88-94bb43da42e5@gmail.com>
Date:   Mon, 19 Apr 2021 07:58:50 -0400
From:   Jes Sorensen <jes.sorensen@...il.com>
To:     "Gustavo A. R. Silva" <gustavo@...eddedor.com>,
        Kees Cook <keescook@...omium.org>
Cc:     Kalle Valo <kvalo@...eaurora.org>,
        "Gustavo A. R. Silva" <gustavoars@...nel.org>,
        "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 3:24 PM, Gustavo A. R. Silva wrote:
> 
> 
> On 4/17/21 13:29, Jes Sorensen wrote:
>> On 3/10/21 3:59 PM, Kees Cook wrote:
>>> On Wed, Mar 10, 2021 at 02:51:24PM -0500, Jes Sorensen wrote:
>>>> On 3/10/21 2:45 PM, Kees Cook wrote:
>>>>> On Wed, Mar 10, 2021 at 02:31:57PM -0500, Jes Sorensen wrote:
>>>>>> On 3/10/21 2:14 PM, Kees Cook wrote:
>>>>>>> Hm, this conversation looks like a miscommunication, mainly? I see
>>>>>>> Gustavo, as requested by many others[1], replacing the fallthrough
>>>>>>> comments with the "fallthrough" statement. (This is more than just a
>>>>>>> "Clang doesn't parse comments" issue.)
>>>>>>>
>>>>>>> This could be a tree-wide patch and not bother you, but Greg KH has
>>>>>>> generally advised us to send these changes broken out. Anyway, this
>>>>>>> change still needs to land, so what would be the preferred path? I think
>>>>>>> Gustavo could just carry it for Linus to merge without bothering you if
>>>>>>> that'd be preferred?
>>>>>>
>>>>>> I'll respond with the same I did last time, fallthrough is not C and
>>>>>> it's ugly.
>>>>>
>>>>> I understand your point of view, but this is not the consensus[1] of
>>>>> the community. "fallthrough" is a macro, using the GCC fallthrough
>>>>> attribute, with the expectation that we can move to the C17/C18
>>>>> "[[fallthrough]]" statement once it is finalized by the C standards
>>>>> body.
>>>>
>>>> I don't know who decided on that, but I still disagree. It's an ugly and
>>>> pointless change that serves little purpose. We shouldn't have allowed
>>>> the ugly /* fall-through */ comments in either, but at least they didn't
>>>> mess with the code. I guess when you give someone an inch, they take a mile.
>>>>
>>>> Last time this came up, the discussion was that clang refused to fix
>>>> their brokenness and therefore this nonsense was being pushed into the
>>>> kernel. It's still a pointless argument, if clang can't fix it's crap,
>>>> then stop using it.
>>>>
>>>> As Kalle correctly pointed out, none of the previous comments to this
>>>> were addressed, the patches were just reposted as fact. Not exactly a
>>>> nice way to go about it either.
>>>
>>> Do you mean changing the commit log to re-justify these changes? I
>>> guess that could be done, but based on the thread, it didn't seem to
>>> be needed. The change is happening to match the coding style consensus
>>> reached to give the kernel the flexibility to move from a gcc extension
>>> to the final C standards committee results without having to do treewide
>>> commits again (i.e. via the macro).
>>
>> No, I am questioning why Gustavo continues to push this nonsense that
>> serves no purpose whatsoever. In addition he has consistently ignored
>> comments and just keep reposting it. But I guess that is how it works,
>> ignore feedback, repost junk, repeat.
> 
> I was asking for feedback here[1] and here[2] after people (you and Kalle)
> commented on this patch. How is that ignoring people? And -again- why
> people ignored my requests for feedback in this conversation? It's a mystery
> to me, honestly.

All you did was post a pointer to the fact that some other people
couldn't be bothered speaking out against the patch, and let it go in.
You haven't addressed any of the original concerns raised.

The big mistake here was of course to allow the pointless /* fallthrough
*/ changes to go in in the first place.

Jes