2020-08-20 13:01:51

by He Zhe

[permalink] [raw]
Subject: [PATCH] mips/oprofile: Fix fallthrough placement

From: He Zhe <[email protected]>

We want neither
"
include/linux/compiler_attributes.h:201:41: warning: statement will never
be executed [-Wswitch-unreachable]
201 | # define fallthrough __attribute__((__fallthrough__))
| ^~~~~~~~~~~~~
"
nor
"
include/linux/compiler_attributes.h:201:41: warning: attribute
'fallthrough' not preceding a case label or default label
201 | # define fallthrough __attribute__((__fallthrough__))
| ^~~~~~~~~~~~~
"

It's not worth adding one more macro. Let's simply place the fallthrough
in between the expansions.

Signed-off-by: He Zhe <[email protected]>
---
arch/mips/oprofile/op_model_mipsxx.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/mips/oprofile/op_model_mipsxx.c b/arch/mips/oprofile/op_model_mipsxx.c
index 1493c49ca47a..55d7b7fd18b6 100644
--- a/arch/mips/oprofile/op_model_mipsxx.c
+++ b/arch/mips/oprofile/op_model_mipsxx.c
@@ -245,7 +245,6 @@ static int mipsxx_perfcount_handler(void)

switch (counters) {
#define HANDLE_COUNTER(n) \
- fallthrough; \
case n + 1: \
control = r_c0_perfctrl ## n(); \
counter = r_c0_perfcntr ## n(); \
@@ -256,8 +255,11 @@ static int mipsxx_perfcount_handler(void)
handled = IRQ_HANDLED; \
}
HANDLE_COUNTER(3)
+ fallthrough;
HANDLE_COUNTER(2)
+ fallthrough;
HANDLE_COUNTER(1)
+ fallthrough;
HANDLE_COUNTER(0)
}

--
2.17.1


2020-08-21 07:51:23

by Thomas Bogendoerfer

[permalink] [raw]
Subject: Re: [PATCH] mips/oprofile: Fix fallthrough placement

On Thu, Aug 20, 2020 at 08:54:40PM +0800, [email protected] wrote:
> From: He Zhe <[email protected]>
>
> We want neither
> "
> include/linux/compiler_attributes.h:201:41: warning: statement will never
> be executed [-Wswitch-unreachable]
> 201 | # define fallthrough __attribute__((__fallthrough__))
> | ^~~~~~~~~~~~~
> "
> nor
> "
> include/linux/compiler_attributes.h:201:41: warning: attribute
> 'fallthrough' not preceding a case label or default label
> 201 | # define fallthrough __attribute__((__fallthrough__))
> | ^~~~~~~~~~~~~
> "
>
> It's not worth adding one more macro. Let's simply place the fallthrough
> in between the expansions.
>
> Signed-off-by: He Zhe <[email protected]>

there is already another patch for the problem, which I've applied
to mips-fixes.

Thomas.

--
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea. [ RFC1925, 2.3 ]

2020-08-21 08:50:48

by He Zhe

[permalink] [raw]
Subject: Re: [PATCH] mips/oprofile: Fix fallthrough placement



On 8/21/20 3:48 PM, Thomas Bogendoerfer wrote:
> On Thu, Aug 20, 2020 at 08:54:40PM +0800, [email protected] wrote:
>> From: He Zhe <[email protected]>
>>
>> We want neither
>> "
>> include/linux/compiler_attributes.h:201:41: warning: statement will never
>> be executed [-Wswitch-unreachable]
>> 201 | # define fallthrough __attribute__((__fallthrough__))
>> | ^~~~~~~~~~~~~
>> "
>> nor
>> "
>> include/linux/compiler_attributes.h:201:41: warning: attribute
>> 'fallthrough' not preceding a case label or default label
>> 201 | # define fallthrough __attribute__((__fallthrough__))
>> | ^~~~~~~~~~~~~
>> "
>>
>> It's not worth adding one more macro. Let's simply place the fallthrough
>> in between the expansions.
>>
>> Signed-off-by: He Zhe <[email protected]>
> there is already another patch for the problem, which I've applied
> to mips-fixes.

You mean the below one?
https://git.kernel.org/pub/scm/linux/kernel/git/mips/linux.git/commit/?h=mips-fixes&id=5900acb374fe2f4f42bbcb2c84db64f582d917a1

That patch handles the first warning in my commit log but does not handle the
second one which is introduced since gcc v10.1.0 commit 6c80b1b56dec
("Make more bad uses of fallthrough attribute into pedwarns.").

Zhe

>
> Thomas.
>

2020-08-21 09:40:01

by Gustavo A. R. Silva

[permalink] [raw]
Subject: Re: [PATCH] mips/oprofile: Fix fallthrough placement



On 8/21/20 03:46, He Zhe wrote:
>
>
> On 8/21/20 3:48 PM, Thomas Bogendoerfer wrote:
>> On Thu, Aug 20, 2020 at 08:54:40PM +0800, [email protected] wrote:
>>> From: He Zhe <[email protected]>
>>>
>>> We want neither
>>> "
>>> include/linux/compiler_attributes.h:201:41: warning: statement will never
>>> be executed [-Wswitch-unreachable]
>>> 201 | # define fallthrough __attribute__((__fallthrough__))
>>> | ^~~~~~~~~~~~~
>>> "
>>> nor
>>> "
>>> include/linux/compiler_attributes.h:201:41: warning: attribute
>>> 'fallthrough' not preceding a case label or default label
>>> 201 | # define fallthrough __attribute__((__fallthrough__))
>>> | ^~~~~~~~~~~~~
>>> "
>>>
>>> It's not worth adding one more macro. Let's simply place the fallthrough
>>> in between the expansions.
>>>
>>> Signed-off-by: He Zhe <[email protected]>
>> there is already another patch for the problem, which I've applied
>> to mips-fixes.
>
> You mean the below one?
> https://git.kernel.org/pub/scm/linux/kernel/git/mips/linux.git/commit/?h=mips-fixes&id=5900acb374fe2f4f42bbcb2c84db64f582d917a1
>
> That patch handles the first warning in my commit log but does not handle the
> second one which is introduced since gcc v10.1.0 commit 6c80b1b56dec
> ("Make more bad uses of fallthrough attribute into pedwarns.").
>

This is true.

Thomas, please apply this patch instead of mine. Also, consider adding
the Fixes tag and CC stable due to the non-executable code error, and
my Reviewed-by:

Fixes: c9b029903466 ("MIPS: Use fallthrough for arch/mips")
Cc: [email protected]
Reviewed-by: Gustavo A. R. Silva <[email protected]>

Thanks
--
Gustavo

2020-08-21 14:08:52

by Thomas Bogendoerfer

[permalink] [raw]
Subject: Re: [PATCH] mips/oprofile: Fix fallthrough placement

On Fri, Aug 21, 2020 at 04:23:28AM -0500, Gustavo A. R. Silva wrote:
>
>
> On 8/21/20 03:46, He Zhe wrote:
> >
> >
> > On 8/21/20 3:48 PM, Thomas Bogendoerfer wrote:
> >> On Thu, Aug 20, 2020 at 08:54:40PM +0800, [email protected] wrote:
> >>> From: He Zhe <[email protected]>
> >>>
> >>> We want neither
> >>> "
> >>> include/linux/compiler_attributes.h:201:41: warning: statement will never
> >>> be executed [-Wswitch-unreachable]
> >>> 201 | # define fallthrough __attribute__((__fallthrough__))
> >>> | ^~~~~~~~~~~~~
> >>> "
> >>> nor
> >>> "
> >>> include/linux/compiler_attributes.h:201:41: warning: attribute
> >>> 'fallthrough' not preceding a case label or default label
> >>> 201 | # define fallthrough __attribute__((__fallthrough__))
> >>> | ^~~~~~~~~~~~~
> >>> "
> >>>
> >>> It's not worth adding one more macro. Let's simply place the fallthrough
> >>> in between the expansions.
> >>>
> >>> Signed-off-by: He Zhe <[email protected]>
> >> there is already another patch for the problem, which I've applied
> >> to mips-fixes.
> >
> > You mean the below one?
> > https://git.kernel.org/pub/scm/linux/kernel/git/mips/linux.git/commit/?h=mips-fixes&id=5900acb374fe2f4f42bbcb2c84db64f582d917a1
> >
> > That patch handles the first warning in my commit log but does not handle the
> > second one which is introduced since gcc v10.1.0 commit 6c80b1b56dec
> > ("Make more bad uses of fallthrough attribute into pedwarns.").
> >
>
> This is true.
>
> Thomas, please apply this patch instead of mine. Also, consider adding
> the Fixes tag and CC stable due to the non-executable code error, and
> my Reviewed-by:
>
> Fixes: c9b029903466 ("MIPS: Use fallthrough for arch/mips")
> Cc: [email protected]
> Reviewed-by: Gustavo A. R. Silva <[email protected]>

done.

Thomas.

--
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea. [ RFC1925, 2.3 ]