DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] devtools: ignore PREFER_FALLTHROUGH
@ 2020-08-04  7:38 Adrian Moreno
  2020-08-04  7:43 ` Ferruh Yigit
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Adrian Moreno @ 2020-08-04  7:38 UTC (permalink / raw)
  To: dev; +Cc: thomas, chenbo.xia, david.marchand, 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.

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


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [dpdk-dev] [PATCH] devtools: ignore PREFER_FALLTHROUGH
  2020-08-04  7:38 [dpdk-dev] [PATCH] devtools: ignore PREFER_FALLTHROUGH Adrian Moreno
@ 2020-08-04  7:43 ` Ferruh Yigit
  2020-08-04  8:00 ` Xia, Chenbo
  2020-08-05  9:12 ` Thomas Monjalon
  2 siblings, 0 replies; 7+ messages in thread
From: Ferruh Yigit @ 2020-08-04  7:43 UTC (permalink / raw)
  To: Adrian Moreno, dev; +Cc: thomas, chenbo.xia, david.marchand

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>


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [dpdk-dev] [PATCH] devtools: ignore PREFER_FALLTHROUGH
  2020-08-04  7:38 [dpdk-dev] [PATCH] devtools: ignore PREFER_FALLTHROUGH Adrian Moreno
  2020-08-04  7:43 ` Ferruh Yigit
@ 2020-08-04  8:00 ` Xia, Chenbo
  2020-08-05  9:12 ` Thomas Monjalon
  2 siblings, 0 replies; 7+ messages in thread
From: Xia, Chenbo @ 2020-08-04  8:00 UTC (permalink / raw)
  To: Adrian Moreno, dev; +Cc: thomas, david.marchand


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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [dpdk-dev] [PATCH] devtools: ignore PREFER_FALLTHROUGH
  2020-08-04  7:38 [dpdk-dev] [PATCH] devtools: ignore PREFER_FALLTHROUGH Adrian Moreno
  2020-08-04  7:43 ` Ferruh Yigit
  2020-08-04  8:00 ` Xia, Chenbo
@ 2020-08-05  9:12 ` Thomas Monjalon
  2020-08-05 13:34   ` Adrian Moreno
  2 siblings, 1 reply; 7+ messages in thread
From: Thomas Monjalon @ 2020-08-05  9:12 UTC (permalink / raw)
  To: Adrian Moreno; +Cc: dev, chenbo.xia, david.marchand

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.




^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [dpdk-dev] [PATCH] devtools: ignore PREFER_FALLTHROUGH
  2020-08-05  9:12 ` Thomas Monjalon
@ 2020-08-05 13:34   ` Adrian Moreno
  2020-08-05 13:54     ` Thomas Monjalon
  0 siblings, 1 reply; 7+ messages in thread
From: Adrian Moreno @ 2020-08-05 13:34 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev, chenbo.xia, david.marchand

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
-- 
Adrián Moreno


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [dpdk-dev] [PATCH] devtools: ignore PREFER_FALLTHROUGH
  2020-08-05 13:34   ` Adrian Moreno
@ 2020-08-05 13:54     ` Thomas Monjalon
  2020-08-07 11:35       ` Thomas Monjalon
  0 siblings, 1 reply; 7+ messages in thread
From: Thomas Monjalon @ 2020-08-05 13:54 UTC (permalink / raw)
  To: Adrian Moreno; +Cc: dev, chenbo.xia, david.marchand

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



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [dpdk-dev] [PATCH] devtools: ignore PREFER_FALLTHROUGH
  2020-08-05 13:54     ` Thomas Monjalon
@ 2020-08-07 11:35       ` Thomas Monjalon
  0 siblings, 0 replies; 7+ messages in thread
From: Thomas Monjalon @ 2020-08-07 11:35 UTC (permalink / raw)
  To: Adrian Moreno; +Cc: dev, chenbo.xia, david.marchand, ferruh.yigit

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.




^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2020-08-07 11:35 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-04  7:38 [dpdk-dev] [PATCH] devtools: ignore PREFER_FALLTHROUGH Adrian Moreno
2020-08-04  7:43 ` Ferruh Yigit
2020-08-04  8:00 ` Xia, Chenbo
2020-08-05  9:12 ` Thomas Monjalon
2020-08-05 13:34   ` Adrian Moreno
2020-08-05 13:54     ` Thomas Monjalon
2020-08-07 11:35       ` Thomas Monjalon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).