Re: [GIT PULL] fallthrough fixes for Clang for 5.17-rc1

[Linus Torvalds]

On Tue, Jan 11, 2022 at 11:08 AM Gustavo A. R. Silva
<gustavoars@xxxxxxxxxx> wrote:
>
> Please, pull the following patches that fix some fall-through warnings
> when building with Clang and -Wimplicit-fallthrough.

Ugh. I started pulling that, and then I actually looked at this one:

> Gustavo A. R. Silva (1):
>       MIPS: mm: tlbex: Fix fall-through warning for Clang

and that's just too ugly to live.

It was ugly before, but now it's just insane.

It literally has an if-statement with a block statement that contains
other cases.

Fine, that's not illegal, and Duff's device made the model famous.

But in this case, there's no actual _reason_ for it. It literally
looks like a mistake to me.

There's no reason to have that block statement in the first place, and
there's *doubly* no reason to add a "fallthrough" to other case
statements that then just do a "break" anyway.

I notice that we actually had that exact same pattern earlier, see
line 2166 in that same line. And it's equally bogus there. Actually,
it's even more bogus there, because the indentation is wrong too!

So I _think_ that patch should do something like the attached, but I
didn't actually test this in any way (I didn't check my mips
cross-build setup), and this is all so ugly that I have to avert my
eyes to even attach that patch to this email.

I also don't understand why it's doing that 'switch ()' on the
current_cpu_type(), only to avoid the 'cpu_has_mips_r2_exec_hazard'
test, Which is just _another_ switch on current_cpu_type().

All this code is completely incomprehensibly oddly written. It looks
like some kind of cut-and-paste thing with no actual understanding of
the code. And I refuse to make it worse than it already is by adding
even *more* mindless blather to it.

               Linus
 arch/mips/mm/tlbex.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/arch/mips/mm/tlbex.c b/arch/mips/mm/tlbex.c
index b131e6a77383..74f557ed50b0 100644
--- a/arch/mips/mm/tlbex.c
+++ b/arch/mips/mm/tlbex.c
@@ -2161,15 +2161,14 @@ static void build_r4000_tlb_load_handler(void)
 
 		switch (current_cpu_type()) {
 		default:
-			if (cpu_has_mips_r2_exec_hazard) {
+			if (cpu_has_mips_r2_exec_hazard)
 				uasm_i_ehb(&p);
-			fallthrough;
+			break;
 
 		case CPU_CAVIUM_OCTEON:
 		case CPU_CAVIUM_OCTEON_PLUS:
 		case CPU_CAVIUM_OCTEON2:
-				break;
-			}
+			break;
 		}
 
 		/* Examine  entrylo 0 or 1 based on ptr. */
@@ -2237,14 +2236,14 @@ static void build_r4000_tlb_load_handler(void)
 
 		switch (current_cpu_type()) {
 		default:
-			if (cpu_has_mips_r2_exec_hazard) {
+			if (cpu_has_mips_r2_exec_hazard)
 				uasm_i_ehb(&p);
+			break;
 
 		case CPU_CAVIUM_OCTEON:
 		case CPU_CAVIUM_OCTEON_PLUS:
 		case CPU_CAVIUM_OCTEON2:
-				break;
-			}
+			break;
 		}
 
 		/* Examine  entrylo 0 or 1 based on ptr. */