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

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>
  

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

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.
  

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
  

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
  

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.