devtools: ignore PREFER_FALLTHROUGH
diff mbox series

Message ID 20200804073837.88189-1-amorenoz@redhat.com
State Accepted, archived
Delegated to: Thomas Monjalon
Headers show
Series
  • devtools: ignore PREFER_FALLTHROUGH
Related show

Checks

Context Check Description
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-testing success Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/travis-robot success Travis build: passed
ci/Intel-compilation success Compilation OK
ci/checkpatch warning coding style issues

Commit Message

Adrian Moreno Aug. 4, 2020, 7:38 a.m. UTC
The PREFER_FALLTHROUGH check warns if a passthrough comment is found
because, in the kernel, the special macro "fallthrough" is prefered.

Since that keyword is not defined in DPDK, ignore the warning.

Ignoring this check does not affect the MISSING_BREAK check that will
warn if a switch case/default is not preceded by break or a fallthrough
comment.

Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
---
 devtools/checkpatches.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Ferruh Yigit Aug. 4, 2020, 7:43 a.m. UTC | #1
On 8/4/2020 8:38 AM, Adrian Moreno wrote:
> The PREFER_FALLTHROUGH check warns if a passthrough comment is found
> because, in the kernel, the special macro "fallthrough" is prefered.
> 
> Since that keyword is not defined in DPDK, ignore the warning.
> 
> Ignoring this check does not affect the MISSING_BREAK check that will
> warn if a switch case/default is not preceded by break or a fallthrough
> comment.
> 
> Signed-off-by: Adrian Moreno <amorenoz@redhat.com>

Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>
Xia, Chenbo Aug. 4, 2020, 8 a.m. UTC | #2
> -----Original Message-----
> From: Adrian Moreno <amorenoz@redhat.com>
> Sent: Tuesday, August 4, 2020 3:39 PM
> To: dev@dpdk.org
> Cc: thomas@monjalon.net; Xia, Chenbo <chenbo.xia@intel.com>;
> david.marchand@redhat.com; Adrian Moreno <amorenoz@redhat.com>
> Subject: [PATCH] devtools: ignore PREFER_FALLTHROUGH
> 
> The PREFER_FALLTHROUGH check warns if a passthrough comment is found
> because, in the kernel, the special macro "fallthrough" is prefered.
> 
> Since that keyword is not defined in DPDK, ignore the warning.
> 
> Ignoring this check does not affect the MISSING_BREAK check that will warn if a
> switch case/default is not preceded by break or a fallthrough comment.
> 
> Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
> ---
>  devtools/checkpatches.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/devtools/checkpatches.sh b/devtools/checkpatches.sh index
> acf921ae0..4283ca82b 100755
> --- a/devtools/checkpatches.sh
> +++ b/devtools/checkpatches.sh
> @@ -33,7 +33,7 @@
> VOLATILE,PREFER_PACKED,PREFER_ALIGNED,PREFER_PRINTF,\
>  PREFER_KERNEL_TYPES,BIT_MACRO,CONST_STRUCT,\
>  SPLIT_STRING,LONG_LINE_STRING,C99_COMMENT_TOLERANCE,\
> 
> LINE_SPACING,PARENTHESIS_ALIGNMENT,NETWORKING_BLOCK_COMMENT_
> STYLE,\
> -NEW_TYPEDEFS,COMPARISON_TO_NULL"
> +NEW_TYPEDEFS,COMPARISON_TO_NULL,PREFER_FALLTHROUGH"
>  options="$options $DPDK_CHECKPATCH_OPTIONS"
> 
>  print_usage () {
> --
> 2.26.2

Acked-by: Chenbo Xia <chenbo.xia@intel.com>
Thomas Monjalon Aug. 5, 2020, 9:12 a.m. UTC | #3
04/08/2020 09:38, Adrian Moreno:
> The PREFER_FALLTHROUGH check warns if a passthrough comment is found
> because, in the kernel, the special macro "fallthrough" is prefered.
> 
> Since that keyword is not defined in DPDK, ignore the warning.

We could ask why not defining a similar keyword?

> 
> Ignoring this check does not affect the MISSING_BREAK check that will
> warn if a switch case/default is not preceded by break or a fallthrough
> comment.
> 
> Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
> ---
>  devtools/checkpatches.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/devtools/checkpatches.sh b/devtools/checkpatches.sh
> index acf921ae0..4283ca82b 100755
> --- a/devtools/checkpatches.sh
> +++ b/devtools/checkpatches.sh
> @@ -33,7 +33,7 @@ VOLATILE,PREFER_PACKED,PREFER_ALIGNED,PREFER_PRINTF,\
>  PREFER_KERNEL_TYPES,BIT_MACRO,CONST_STRUCT,\
>  SPLIT_STRING,LONG_LINE_STRING,C99_COMMENT_TOLERANCE,\
>  LINE_SPACING,PARENTHESIS_ALIGNMENT,NETWORKING_BLOCK_COMMENT_STYLE,\
> -NEW_TYPEDEFS,COMPARISON_TO_NULL"
> +NEW_TYPEDEFS,COMPARISON_TO_NULL,PREFER_FALLTHROUGH"

I would add this option between PREFER_KERNEL_TYPES and BIT_MACRO
to maintain a bit of logic ordering.
Adrian Moreno Aug. 5, 2020, 1:34 p.m. UTC | #4
Hi Thomas,

On 8/5/20 11:12 AM, Thomas Monjalon wrote:
> 04/08/2020 09:38, Adrian Moreno:
>> The PREFER_FALLTHROUGH check warns if a passthrough comment is found
>> because, in the kernel, the special macro "fallthrough" is prefered.
>>
>> Since that keyword is not defined in DPDK, ignore the warning.
> 
> We could ask why not defining a similar keyword?
> 

Surely, we can also add the keyword. Given that unintended fallthrough is
already protected by the "MISSING_BREAK" and that fallthrough comments are
already used in DPDK, I thought it made sense to ignore the check.

If you prefer to add the keyword, let me ask:
- Where is the right place for it? Maybe rte_common.h?
- Should all the comments be replaced with the pseudo-keyword?


>>
>> Ignoring this check does not affect the MISSING_BREAK check that will
>> warn if a switch case/default is not preceded by break or a fallthrough
>> comment.
>>
>> Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
>> ---
>>  devtools/checkpatches.sh | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/devtools/checkpatches.sh b/devtools/checkpatches.sh
>> index acf921ae0..4283ca82b 100755
>> --- a/devtools/checkpatches.sh
>> +++ b/devtools/checkpatches.sh
>> @@ -33,7 +33,7 @@ VOLATILE,PREFER_PACKED,PREFER_ALIGNED,PREFER_PRINTF,\
>>  PREFER_KERNEL_TYPES,BIT_MACRO,CONST_STRUCT,\
>>  SPLIT_STRING,LONG_LINE_STRING,C99_COMMENT_TOLERANCE,\
>>  LINE_SPACING,PARENTHESIS_ALIGNMENT,NETWORKING_BLOCK_COMMENT_STYLE,\
>> -NEW_TYPEDEFS,COMPARISON_TO_NULL"
>> +NEW_TYPEDEFS,COMPARISON_TO_NULL,PREFER_FALLTHROUGH"
> 
> I would add this option between PREFER_KERNEL_TYPES and BIT_MACRO
> to maintain a bit of logic ordering.
> 
OK. I'll reorder it if the final decision is to ignore the check.

Thanks
Thomas Monjalon Aug. 5, 2020, 1:54 p.m. UTC | #5
05/08/2020 15:34, Adrian Moreno:
> Hi Thomas,
> 
> On 8/5/20 11:12 AM, Thomas Monjalon wrote:
> > 04/08/2020 09:38, Adrian Moreno:
> >> The PREFER_FALLTHROUGH check warns if a passthrough comment is found
> >> because, in the kernel, the special macro "fallthrough" is prefered.
> >>
> >> Since that keyword is not defined in DPDK, ignore the warning.
> > 
> > We could ask why not defining a similar keyword?
> > 
> 
> Surely, we can also add the keyword. Given that unintended fallthrough is
> already protected by the "MISSING_BREAK" and that fallthrough comments are
> already used in DPDK, I thought it made sense to ignore the check.

Yes this patch makes sense.
And anyway, we'll never use the same keyword as in kernel.

> If you prefer to add the keyword, let me ask:
> - Where is the right place for it? Maybe rte_common.h?

Yes

> - Should all the comments be replaced with the pseudo-keyword?

Yes

Before doing that, please send a RFC.
I remember we already tried that but failed.

> >> --- a/devtools/checkpatches.sh
> >> +++ b/devtools/checkpatches.sh
> >> @@ -33,7 +33,7 @@ VOLATILE,PREFER_PACKED,PREFER_ALIGNED,PREFER_PRINTF,\
> >>  PREFER_KERNEL_TYPES,BIT_MACRO,CONST_STRUCT,\
> >>  SPLIT_STRING,LONG_LINE_STRING,C99_COMMENT_TOLERANCE,\
> >>  LINE_SPACING,PARENTHESIS_ALIGNMENT,NETWORKING_BLOCK_COMMENT_STYLE,\
> >> -NEW_TYPEDEFS,COMPARISON_TO_NULL"
> >> +NEW_TYPEDEFS,COMPARISON_TO_NULL,PREFER_FALLTHROUGH"
> > 
> > I would add this option between PREFER_KERNEL_TYPES and BIT_MACRO
> > to maintain a bit of logic ordering.
> > 
> OK. I'll reorder it if the final decision is to ignore the check.

Yes thanks
Thomas Monjalon Aug. 7, 2020, 11:35 a.m. UTC | #6
05/08/2020 15:54, Thomas Monjalon:
> 05/08/2020 15:34, Adrian Moreno:
> > On 8/5/20 11:12 AM, Thomas Monjalon wrote:
> > > 04/08/2020 09:38, Adrian Moreno:
> > >> --- a/devtools/checkpatches.sh
> > >> +++ b/devtools/checkpatches.sh
> > >> @@ -33,7 +33,7 @@ VOLATILE,PREFER_PACKED,PREFER_ALIGNED,PREFER_PRINTF,\
> > >>  PREFER_KERNEL_TYPES,BIT_MACRO,CONST_STRUCT,\
> > >>  SPLIT_STRING,LONG_LINE_STRING,C99_COMMENT_TOLERANCE,\
> > >>  LINE_SPACING,PARENTHESIS_ALIGNMENT,NETWORKING_BLOCK_COMMENT_STYLE,\
> > >> -NEW_TYPEDEFS,COMPARISON_TO_NULL"
> > >> +NEW_TYPEDEFS,COMPARISON_TO_NULL,PREFER_FALLTHROUGH"
> > > 
> > > I would add this option between PREFER_KERNEL_TYPES and BIT_MACRO
> > > to maintain a bit of logic ordering.
> > > 
> > OK. I'll reorder it if the final decision is to ignore the check.
> 
> Yes thanks

Applied with above change.

Patch
diff mbox series

diff --git a/devtools/checkpatches.sh b/devtools/checkpatches.sh
index acf921ae0..4283ca82b 100755
--- a/devtools/checkpatches.sh
+++ b/devtools/checkpatches.sh
@@ -33,7 +33,7 @@  VOLATILE,PREFER_PACKED,PREFER_ALIGNED,PREFER_PRINTF,\
 PREFER_KERNEL_TYPES,BIT_MACRO,CONST_STRUCT,\
 SPLIT_STRING,LONG_LINE_STRING,C99_COMMENT_TOLERANCE,\
 LINE_SPACING,PARENTHESIS_ALIGNMENT,NETWORKING_BLOCK_COMMENT_STYLE,\
-NEW_TYPEDEFS,COMPARISON_TO_NULL"
+NEW_TYPEDEFS,COMPARISON_TO_NULL,PREFER_FALLTHROUGH"
 options="$options $DPDK_CHECKPATCH_OPTIONS"
 
 print_usage () {