[XEN PATCH v2 1/6][RESEND] automation/eclair: address violations of MISRA C Rule 20.7

Nicola Vetrini posted 6 patches 5 months, 1 week ago
There is a newer version of this series
[XEN PATCH v2 1/6][RESEND] automation/eclair: address violations of MISRA C Rule 20.7
Posted by Nicola Vetrini 5 months, 1 week ago
MISRA C Rule 20.7 states: "Expressions resulting from the expansion
of macro parameters shall be enclosed in parentheses".

The helper macro bitmap_switch has parameters that cannot be parenthesized
in order to comply with the rule, as that would break its functionality.
Moreover, the risk of misuse due developer confusion is deemed not
substantial enough to warrant a more involved refactor, thus the macro
is deviated for this rule.

No functional change.

Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
---
 automation/eclair_analysis/ECLAIR/deviations.ecl | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/automation/eclair_analysis/ECLAIR/deviations.ecl b/automation/eclair_analysis/ECLAIR/deviations.ecl
index 447c1e6661d1..c2698e7074aa 100644
--- a/automation/eclair_analysis/ECLAIR/deviations.ecl
+++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
@@ -463,6 +463,14 @@ of this macro do not lead to developer confusion, and can thus be deviated."
 -config=MC3R1.R20.7,reports+={safe, "any_area(any_loc(any_exp(macro(^count_args_$))))"}
 -doc_end
 
+-doc_begin="The arguments of macro bitmap_switch macro can't be parenthesized as
+the rule would require, without breaking the functionality of the macro. This is
+a specialized local helper macro only used within the bitmap.h header, so it is
+less likely to lead to developer confusion and it is deemed better to deviate it."
+-file_tag+={xen_bitmap_h, "^xen/include/xen/bitmap\\.h$"}
+-config=MC3R1.R20.7,reports+={safe, "any_area(any_loc(any_exp(macro(loc(file(xen_bitmap_h))&&^bitmap_switch$))))"}
+-doc_end
+
 -doc_begin="Uses of variadic macros that have one of their arguments defined as
 a macro and used within the body for both ordinary parameter expansion and as an
 operand to the # or ## operators have a behavior that is well-understood and
-- 
2.34.1
Re: [XEN PATCH v2 1/6][RESEND] automation/eclair: address violations of MISRA C Rule 20.7
Posted by Stefano Stabellini 5 months ago
On Mon, 17 Jun 2024, Nicola Vetrini wrote:
> MISRA C Rule 20.7 states: "Expressions resulting from the expansion
> of macro parameters shall be enclosed in parentheses".
> 
> The helper macro bitmap_switch has parameters that cannot be parenthesized
> in order to comply with the rule, as that would break its functionality.
> Moreover, the risk of misuse due developer confusion is deemed not
> substantial enough to warrant a more involved refactor, thus the macro
> is deviated for this rule.
> 
> No functional change.
> 
> Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>

I would have preferred a SAF tag instead but it can be done later

Acked-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
>  automation/eclair_analysis/ECLAIR/deviations.ecl | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/automation/eclair_analysis/ECLAIR/deviations.ecl b/automation/eclair_analysis/ECLAIR/deviations.ecl
> index 447c1e6661d1..c2698e7074aa 100644
> --- a/automation/eclair_analysis/ECLAIR/deviations.ecl
> +++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
> @@ -463,6 +463,14 @@ of this macro do not lead to developer confusion, and can thus be deviated."
>  -config=MC3R1.R20.7,reports+={safe, "any_area(any_loc(any_exp(macro(^count_args_$))))"}
>  -doc_end
>  
> +-doc_begin="The arguments of macro bitmap_switch macro can't be parenthesized as
> +the rule would require, without breaking the functionality of the macro. This is
> +a specialized local helper macro only used within the bitmap.h header, so it is
> +less likely to lead to developer confusion and it is deemed better to deviate it."
> +-file_tag+={xen_bitmap_h, "^xen/include/xen/bitmap\\.h$"}
> +-config=MC3R1.R20.7,reports+={safe, "any_area(any_loc(any_exp(macro(loc(file(xen_bitmap_h))&&^bitmap_switch$))))"}
> +-doc_end
> +
>  -doc_begin="Uses of variadic macros that have one of their arguments defined as
>  a macro and used within the body for both ordinary parameter expansion and as an
>  operand to the # or ## operators have a behavior that is well-understood and
> -- 
> 2.34.1
>
Re: [XEN PATCH v2 1/6][RESEND] automation/eclair: address violations of MISRA C Rule 20.7
Posted by Stefano Stabellini 5 months, 1 week ago
On Mon, 16 Jun 2024, Nicola Vetrini wrote:
> MISRA C Rule 20.7 states: "Expressions resulting from the expansion
> of macro parameters shall be enclosed in parentheses".
> 
> The helper macro bitmap_switch has parameters that cannot be parenthesized
> in order to comply with the rule, as that would break its functionality.
> Moreover, the risk of misuse due developer confusion is deemed not
> substantial enough to warrant a more involved refactor, thus the macro
> is deviated for this rule.
> 
> No functional change.
> 
> Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>

If possible, I would prefer we used a SAF in-code comment deviation. If
that doesn't work for any reason this patch is fine and I'd ack it.


> ---
>  automation/eclair_analysis/ECLAIR/deviations.ecl | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/automation/eclair_analysis/ECLAIR/deviations.ecl b/automation/eclair_analysis/ECLAIR/deviations.ecl
> index 447c1e6661d1..c2698e7074aa 100644
> --- a/automation/eclair_analysis/ECLAIR/deviations.ecl
> +++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
> @@ -463,6 +463,14 @@ of this macro do not lead to developer confusion, and can thus be deviated."
>  -config=MC3R1.R20.7,reports+={safe, "any_area(any_loc(any_exp(macro(^count_args_$))))"}
>  -doc_end
>  
> +-doc_begin="The arguments of macro bitmap_switch macro can't be parenthesized as
> +the rule would require, without breaking the functionality of the macro. This is
> +a specialized local helper macro only used within the bitmap.h header, so it is
> +less likely to lead to developer confusion and it is deemed better to deviate it."
> +-file_tag+={xen_bitmap_h, "^xen/include/xen/bitmap\\.h$"}
> +-config=MC3R1.R20.7,reports+={safe, "any_area(any_loc(any_exp(macro(loc(file(xen_bitmap_h))&&^bitmap_switch$))))"}
> +-doc_end
> +
>  -doc_begin="Uses of variadic macros that have one of their arguments defined as
>  a macro and used within the body for both ordinary parameter expansion and as an
>  operand to the # or ## operators have a behavior that is well-understood and
> -- 
> 2.34.1
>
Re: [XEN PATCH v2 1/6][RESEND] automation/eclair: address violations of MISRA C Rule 20.7
Posted by Nicola Vetrini 5 months ago
On 2024-06-21 02:18, Stefano Stabellini wrote:
> On Mon, 16 Jun 2024, Nicola Vetrini wrote:
>> MISRA C Rule 20.7 states: "Expressions resulting from the expansion
>> of macro parameters shall be enclosed in parentheses".
>> 
>> The helper macro bitmap_switch has parameters that cannot be 
>> parenthesized
>> in order to comply with the rule, as that would break its 
>> functionality.
>> Moreover, the risk of misuse due developer confusion is deemed not
>> substantial enough to warrant a more involved refactor, thus the macro
>> is deviated for this rule.
>> 
>> No functional change.
>> 
>> Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
> 
> If possible, I would prefer we used a SAF in-code comment deviation. If
> that doesn't work for any reason this patch is fine and I'd ack it.
> 

Would that be an improvement for safety in your opinion?

> 
>> ---
>>  automation/eclair_analysis/ECLAIR/deviations.ecl | 8 ++++++++
>>  1 file changed, 8 insertions(+)
>> 
>> diff --git a/automation/eclair_analysis/ECLAIR/deviations.ecl 
>> b/automation/eclair_analysis/ECLAIR/deviations.ecl
>> index 447c1e6661d1..c2698e7074aa 100644
>> --- a/automation/eclair_analysis/ECLAIR/deviations.ecl
>> +++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
>> @@ -463,6 +463,14 @@ of this macro do not lead to developer confusion, 
>> and can thus be deviated."
>>  -config=MC3R1.R20.7,reports+={safe, 
>> "any_area(any_loc(any_exp(macro(^count_args_$))))"}
>>  -doc_end
>> 
>> +-doc_begin="The arguments of macro bitmap_switch macro can't be 
>> parenthesized as
>> +the rule would require, without breaking the functionality of the 
>> macro. This is
>> +a specialized local helper macro only used within the bitmap.h 
>> header, so it is
>> +less likely to lead to developer confusion and it is deemed better to 
>> deviate it."
>> +-file_tag+={xen_bitmap_h, "^xen/include/xen/bitmap\\.h$"}
>> +-config=MC3R1.R20.7,reports+={safe, 
>> "any_area(any_loc(any_exp(macro(loc(file(xen_bitmap_h))&&^bitmap_switch$))))"}
>> +-doc_end
>> +
>>  -doc_begin="Uses of variadic macros that have one of their arguments 
>> defined as
>>  a macro and used within the body for both ordinary parameter 
>> expansion and as an
>>  operand to the # or ## operators have a behavior that is 
>> well-understood and
>> --
>> 2.34.1
>> 

-- 
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)
Re: [XEN PATCH v2 1/6][RESEND] automation/eclair: address violations of MISRA C Rule 20.7
Posted by Stefano Stabellini 5 months ago
On Tue, 25 Jun 2024, Nicola Vetrini wrote:
> On 2024-06-21 02:18, Stefano Stabellini wrote:
> > On Mon, 16 Jun 2024, Nicola Vetrini wrote:
> > > MISRA C Rule 20.7 states: "Expressions resulting from the expansion
> > > of macro parameters shall be enclosed in parentheses".
> > > 
> > > The helper macro bitmap_switch has parameters that cannot be parenthesized
> > > in order to comply with the rule, as that would break its functionality.
> > > Moreover, the risk of misuse due developer confusion is deemed not
> > > substantial enough to warrant a more involved refactor, thus the macro
> > > is deviated for this rule.
> > > 
> > > No functional change.
> > > 
> > > Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
> > 
> > If possible, I would prefer we used a SAF in-code comment deviation. If
> > that doesn't work for any reason this patch is fine and I'd ack it.
> > 
> 
> Would that be an improvement for safety in your opinion?

Not for safety, as they both achieve the same result, but for
maintainability. The deviations under deviations.ecl are project-wide so
in my opinion should only be used for project-wide global deviations
from a rule. Specific deviations for functions should be done with SAF.
Another reason for this is that deviations under deviations.ecl need to
be manually ported to any other static analyzer one by one while SAF is
meant to support multiple automatically.

More importantly if we change deviations.ecl, deviations.rst should also
be changed the same way. I would say that this is required at a minimum
as deviations.ecl and deviations.rst should be in sync.

 
> > > ---
> > >  automation/eclair_analysis/ECLAIR/deviations.ecl | 8 ++++++++
> > >  1 file changed, 8 insertions(+)
> > > 
> > > diff --git a/automation/eclair_analysis/ECLAIR/deviations.ecl
> > > b/automation/eclair_analysis/ECLAIR/deviations.ecl
> > > index 447c1e6661d1..c2698e7074aa 100644
> > > --- a/automation/eclair_analysis/ECLAIR/deviations.ecl
> > > +++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
> > > @@ -463,6 +463,14 @@ of this macro do not lead to developer confusion, and
> > > can thus be deviated."
> > >  -config=MC3R1.R20.7,reports+={safe,
> > > "any_area(any_loc(any_exp(macro(^count_args_$))))"}
> > >  -doc_end
> > > 
> > > +-doc_begin="The arguments of macro bitmap_switch macro can't be
> > > parenthesized as
> > > +the rule would require, without breaking the functionality of the macro.
> > > This is
> > > +a specialized local helper macro only used within the bitmap.h header, so
> > > it is
> > > +less likely to lead to developer confusion and it is deemed better to
> > > deviate it."
> > > +-file_tag+={xen_bitmap_h, "^xen/include/xen/bitmap\\.h$"}
> > > +-config=MC3R1.R20.7,reports+={safe,
> > > "any_area(any_loc(any_exp(macro(loc(file(xen_bitmap_h))&&^bitmap_switch$))))"}
> > > +-doc_end
> > > +
> > >  -doc_begin="Uses of variadic macros that have one of their arguments
> > > defined as
> > >  a macro and used within the body for both ordinary parameter expansion
> > > and as an
> > >  operand to the # or ## operators have a behavior that is well-understood
> > > and
> > > --
> > > 2.34.1
> > > 
> 
> -- 
> Nicola Vetrini, BSc
> Software Engineer, BUGSENG srl (https://bugseng.com)
>