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

On 3/11/21 01:00, Kalle Valo wrote:

Kees Cook [off-list ref] writes:

quoted

On Fri, Mar 05, 2021 at 03:40:33PM +0200, Kalle Valo wrote:

quoted

"Gustavo A. R. Silva" [off-list ref] writes:

quoted

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@kernel.org>

It's not cool that you ignore the comments you got in [1], then after a
while mark the patch as "RESEND" and not even include a changelog why it
was resent.

[1] https://patchwork.kernel.org/project/linux-wireless/patch/d522f387b2d0dde774785c7169c1f25aa529989d.1605896060.git.gustavoars@kernel.org/

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.)

v1 was clearly rejected by Jes, so sending a new version without any
changelog or comments is not the way to go. The changelog shoud at least
have had "v1 was rejected but I'm resending this again because <insert
reason here>" or something like that to make it clear what's happening.

Why the fact that I replied to that original thread with the message
below is being ignored?

"Just notice that the idea behind this and the following patch is exactly
the same:

https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-drivers-next.git/commit/?id=3f95e92c8a8516b745594049dfccc8c5f8895eea

I could resend this same patch with a different changelog text, but I
don't think such a thing is necessary. However, if people prefer that
approach, just let me know and I can do it.

Thanks
--
Gustavo"

Why no one replied to what I was proposing at the time?

It seems to me that the person that was ignored was actually me, and not the
other way around. :/

--
Gustavo

quoted

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 agree with Greg. Please don't do cleanups like this via another tree
as that just creates more work due to conflicts between the trees, which
is a lot more annoying to deal with than applying few patches. But when
submitting patches please follow the rules, don't cut corners.

Jes, I don't like 'fallthrough' either and prefer the original comment,
but the ship has sailed on this one. Maybe we should just take it?