1. Patchew
  2. Xen
  3. [XEN PATCH 00/13] x86: address violations of MISRA C Rule 16.3
  4. View patch

[XEN PATCH 01/13] automation/eclair: consider also hypened fall-through

Federico Serafini posted 13 patches 5 months, 1 week ago
There is a newer version of this series
[XEN PATCH 01/13] automation/eclair: consider also hypened fall-through
Posted by Federico Serafini 5 months, 1 week ago 
Update ECLAIR configuration to deviate MISRA C Rule 16.3
using different version of hypened fall-through and search for
the comment for 2 lines after the last statement.

Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
---
 automation/eclair_analysis/ECLAIR/deviations.ecl | 2 +-
 docs/misra/deviations.rst                        | 4 ++++
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/automation/eclair_analysis/ECLAIR/deviations.ecl b/automation/eclair_analysis/ECLAIR/deviations.ecl
index b8f9155267..b99a6b8a92 100644
--- a/automation/eclair_analysis/ECLAIR/deviations.ecl
+++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
@@ -400,7 +400,7 @@ safe."
 -doc_end
 
 -doc_begin="Switch clauses ending with an explicit comment indicating the fallthrough intention are safe."
--config=MC3R1.R16.3,reports+={safe, "any_area(end_loc(any_exp(text(^(?s).*/\\* [fF]all ?through.? \\*/.*$,0..1))))"}
+-config=MC3R1.R16.3,reports+={safe, "any_area(end_loc(any_exp(text(^(?s).*/\\* [fF]all[ -]?through.? \\*/.*$,0..2))))"}
 -doc_end
 
 -doc_begin="Switch statements having a controlling expression of enum type deliberately do not have a default case: gcc -Wall enables -Wswitch which warns (and breaks the build as we use -Werror) if one of the enum labels is missing from the switch."
diff --git a/docs/misra/deviations.rst b/docs/misra/deviations.rst
index 41cdfbe5f5..411e1fed3d 100644
--- a/docs/misra/deviations.rst
+++ b/docs/misra/deviations.rst
@@ -353,6 +353,10 @@ Deviations related to MISRA C:2012 Rules:
        However, the use of such comments in new code is deprecated:
        the pseudo-keyword "fallthrough" shall be used.
      - Tagged as `safe` for ECLAIR. The accepted comments are:
+         - /\* fall-through \*/
+         - /\* Fall-through. \*/
+         - /\* Fall-through \*/
+         - /\* fall-through. \*/
          - /\* fall through \*/
          - /\* fall through. \*/
          - /\* fallthrough \*/
-- 
2.34.1
Re: [XEN PATCH 01/13] automation/eclair: consider also hypened fall-through
Posted by Julien Grall 5 months, 1 week ago 
Hi,

On 20/06/2024 15:02, Federico Serafini wrote:
> Update ECLAIR configuration to deviate MISRA C Rule 16.3
> using different version of hypened fall-through and search for
> the comment for 2 lines after the last statement.
> 
> Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
> ---
>   automation/eclair_analysis/ECLAIR/deviations.ecl | 2 +-
>   docs/misra/deviations.rst                        | 4 ++++
>   2 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/automation/eclair_analysis/ECLAIR/deviations.ecl b/automation/eclair_analysis/ECLAIR/deviations.ecl
> index b8f9155267..b99a6b8a92 100644
> --- a/automation/eclair_analysis/ECLAIR/deviations.ecl
> +++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
> @@ -400,7 +400,7 @@ safe."
>   -doc_end
>   
>   -doc_begin="Switch clauses ending with an explicit comment indicating the fallthrough intention are safe."
> --config=MC3R1.R16.3,reports+={safe, "any_area(end_loc(any_exp(text(^(?s).*/\\* [fF]all ?through.? \\*/.*$,0..1))))"}
> +-config=MC3R1.R16.3,reports+={safe, "any_area(end_loc(any_exp(text(^(?s).*/\\* [fF]all[ -]?through.? \\*/.*$,0..2))))"}
>   -doc_end
>   
>   -doc_begin="Switch statements having a controlling expression of enum type deliberately do not have a default case: gcc -Wall enables -Wswitch which warns (and breaks the build as we use -Werror) if one of the enum labels is missing from the switch."
> diff --git a/docs/misra/deviations.rst b/docs/misra/deviations.rst
> index 41cdfbe5f5..411e1fed3d 100644
> --- a/docs/misra/deviations.rst
> +++ b/docs/misra/deviations.rst
> @@ -353,6 +353,10 @@ Deviations related to MISRA C:2012 Rules:
>          However, the use of such comments in new code is deprecated:
>          the pseudo-keyword "fallthrough" shall be used.
>        - Tagged as `safe` for ECLAIR. The accepted comments are:
> +         - /\* fall-through \*/
> +         - /\* Fall-through. \*/
> +         - /\* Fall-through \*/
> +         - /\* fall-through. \*/

How many places use the 4 above? The reason I am asking is I am not 
particularly happy to add yet another set of variant.

I would rather prefer if we settle down with a single comment and 
therefore convert them to the pseudo-keyword.

Cheers,

-- 
Julien Grall
Re: [XEN PATCH 01/13] automation/eclair: consider also hypened fall-through
Posted by Federico Serafini 5 months, 1 week ago 
On 20/06/24 16:39, Julien Grall wrote:
> Hi,
> 
> On 20/06/2024 15:02, Federico Serafini wrote:
>> Update ECLAIR configuration to deviate MISRA C Rule 16.3
>> using different version of hypened fall-through and search for
>> the comment for 2 lines after the last statement.
>>
>> Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
>> ---
>>   automation/eclair_analysis/ECLAIR/deviations.ecl | 2 +-
>>   docs/misra/deviations.rst                        | 4 ++++
>>   2 files changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/automation/eclair_analysis/ECLAIR/deviations.ecl 
>> b/automation/eclair_analysis/ECLAIR/deviations.ecl
>> index b8f9155267..b99a6b8a92 100644
>> --- a/automation/eclair_analysis/ECLAIR/deviations.ecl
>> +++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
>> @@ -400,7 +400,7 @@ safe."
>>   -doc_end
>>   -doc_begin="Switch clauses ending with an explicit comment 
>> indicating the fallthrough intention are safe."
>> --config=MC3R1.R16.3,reports+={safe, 
>> "any_area(end_loc(any_exp(text(^(?s).*/\\* [fF]all ?through.? 
>> \\*/.*$,0..1))))"}
>> +-config=MC3R1.R16.3,reports+={safe, 
>> "any_area(end_loc(any_exp(text(^(?s).*/\\* [fF]all[ -]?through.? 
>> \\*/.*$,0..2))))"}
>>   -doc_end
>>   -doc_begin="Switch statements having a controlling expression of 
>> enum type deliberately do not have a default case: gcc -Wall enables 
>> -Wswitch which warns (and breaks the build as we use -Werror) if one 
>> of the enum labels is missing from the switch."
>> diff --git a/docs/misra/deviations.rst b/docs/misra/deviations.rst
>> index 41cdfbe5f5..411e1fed3d 100644
>> --- a/docs/misra/deviations.rst
>> +++ b/docs/misra/deviations.rst
>> @@ -353,6 +353,10 @@ Deviations related to MISRA C:2012 Rules:
>>          However, the use of such comments in new code is deprecated:
>>          the pseudo-keyword "fallthrough" shall be used.
>>        - Tagged as `safe` for ECLAIR. The accepted comments are:
>> +         - /\* fall-through \*/
>> +         - /\* Fall-through. \*/
>> +         - /\* Fall-through \*/
>> +         - /\* fall-through. \*/
> 
> How many places use the 4 above? The reason I am asking is I am not 
> particularly happy to add yet another set of variant.
> 
> I would rather prefer if we settle down with a single comment and 
> therefore convert them to the pseudo-keyword.

7 occurrences and 3 of them are in xen/arch/x86/hvm/emulate.c
PATCH 07/13 modifies emulate.c anyway, so I could remove them in a v2.

-- 
Federico Serafini, M.Sc.

Software Engineer, BUGSENG (http://bugseng.com)
Re: [XEN PATCH 01/13] automation/eclair: consider also hypened fall-through
Posted by Jan Beulich 5 months, 1 week ago 
On 20.06.2024 16:02, Federico Serafini wrote:
> --- a/automation/eclair_analysis/ECLAIR/deviations.ecl
> +++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
> @@ -400,7 +400,7 @@ safe."
>  -doc_end
>  
>  -doc_begin="Switch clauses ending with an explicit comment indicating the fallthrough intention are safe."
> --config=MC3R1.R16.3,reports+={safe, "any_area(end_loc(any_exp(text(^(?s).*/\\* [fF]all ?through.? \\*/.*$,0..1))))"}
> +-config=MC3R1.R16.3,reports+={safe, "any_area(end_loc(any_exp(text(^(?s).*/\\* [fF]all[ -]?through.? \\*/.*$,0..2))))"}

Is is a regex, isn't it? Doesn't the period also need escaping (or enclosing
in square brackets)? (I realize it was like this before, but still.)

> --- a/docs/misra/deviations.rst
> +++ b/docs/misra/deviations.rst
> @@ -353,6 +353,10 @@ Deviations related to MISRA C:2012 Rules:
>         However, the use of such comments in new code is deprecated:
>         the pseudo-keyword "fallthrough" shall be used.
>       - Tagged as `safe` for ECLAIR. The accepted comments are:
> +         - /\* fall-through \*/
> +         - /\* Fall-through. \*/
> +         - /\* Fall-through \*/
> +         - /\* fall-through. \*/
>           - /\* fall through \*/
>           - /\* fall through. \*/
>           - /\* fallthrough \*/

Nit: Can the capital-F and non-capital-f variants please be next to each other?

Jan
Re: [XEN PATCH 01/13] automation/eclair: consider also hypened fall-through
Posted by Federico Serafini 5 months, 1 week ago 
On 20/06/24 16:15, Jan Beulich wrote:
> On 20.06.2024 16:02, Federico Serafini wrote:
>> --- a/automation/eclair_analysis/ECLAIR/deviations.ecl
>> +++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
>> @@ -400,7 +400,7 @@ safe."
>>   -doc_end
>>   
>>   -doc_begin="Switch clauses ending with an explicit comment indicating the fallthrough intention are safe."
>> --config=MC3R1.R16.3,reports+={safe, "any_area(end_loc(any_exp(text(^(?s).*/\\* [fF]all ?through.? \\*/.*$,0..1))))"}
>> +-config=MC3R1.R16.3,reports+={safe, "any_area(end_loc(any_exp(text(^(?s).*/\\* [fF]all[ -]?through.? \\*/.*$,0..2))))"}
> 
> Is is a regex, isn't it? Doesn't the period also need escaping (or enclosing
> in square brackets)? (I realize it was like this before, but still.)

Yes, thanks for noticing.

> 
>> --- a/docs/misra/deviations.rst
>> +++ b/docs/misra/deviations.rst
>> @@ -353,6 +353,10 @@ Deviations related to MISRA C:2012 Rules:
>>          However, the use of such comments in new code is deprecated:
>>          the pseudo-keyword "fallthrough" shall be used.
>>        - Tagged as `safe` for ECLAIR. The accepted comments are:
>> +         - /\* fall-through \*/
>> +         - /\* Fall-through. \*/
>> +         - /\* Fall-through \*/
>> +         - /\* fall-through. \*/
>>            - /\* fall through \*/
>>            - /\* fall through. \*/
>>            - /\* fallthrough \*/
> 
> Nit: Can the capital-F and non-capital-f variants please be next to each other?

Ok.

-- 
Federico Serafini, M.Sc.

Software Engineer, BUGSENG (http://bugseng.com)

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.