1. Patchew
  2. Xen
  3. View series

[XEN PATCH] automation/eclair_analysis: fix MISRA Rule 20.7 regression in self-tests.h

Nicola Vetrini posted 1 patch 2 months, 1 week ago
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/4e59a8b7d97f34a824922013ffe5e44f70e6abaf.1725801931.git.nicola.vetrini@bugseng.com
automation/eclair_analysis/ECLAIR/deviations.ecl | 6 ++++++
docs/misra/deviations.rst                        | 8 ++++++++
2 files changed, 14 insertions(+)
[XEN PATCH] automation/eclair_analysis: fix MISRA Rule 20.7 regression in self-tests.h
Posted by Nicola Vetrini 2 months, 1 week ago 
Prior to bd1664db7b7d ("xen/bitops: Introduce a multiple_bits_set() helper")
the definition of {COMPILE,RUNTIME}_CHECK was fully compliant with respect
to MISRA C Rule 20.7:

"Expressions resulting from the expansion of macro parameters shall be
enclosed in parentheses."

However, to allow testing function-like macros, parentheses on the "fn"
parameter were removed and thus new violations of the rule have been
introduced. Given the usefulness of this functionality,
it is deemed ok to deviate these two macros for this rule, because
their scope of (direct) usage is limited to just the file where they
are defined, and the possibility of misuses is unlikely.

No functional change.

Fixes: bd1664db7b7d ("xen/bitops: Introduce a multiple_bits_set() helper")
Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
---
 automation/eclair_analysis/ECLAIR/deviations.ecl | 6 ++++++
 docs/misra/deviations.rst                        | 8 ++++++++
 2 files changed, 14 insertions(+)

diff --git a/automation/eclair_analysis/ECLAIR/deviations.ecl b/automation/eclair_analysis/ECLAIR/deviations.ecl
index 9051f4160282..ed80ac795851 100644
--- a/automation/eclair_analysis/ECLAIR/deviations.ecl
+++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
@@ -581,6 +581,12 @@ 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 argument \"fn\" in macros {COMPILE,RUNTIME}_CHECK is not parenthesized
+on purpose, to be able to test function-like macros. Given the specialized and limited
+use of this macro, it is deemed ok to deviate them."
+-config=MC3R1.R20.7,reports+={deliberate, "any_area(any_loc(any_exp(macro(^(COMPILE_CHECK|RUNTIME_CHECK)$))))"}
+-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
diff --git a/docs/misra/deviations.rst b/docs/misra/deviations.rst
index b66c271c4e7c..b020144a3254 100644
--- a/docs/misra/deviations.rst
+++ b/docs/misra/deviations.rst
@@ -533,6 +533,14 @@ Deviations related to MISRA C:2012 Rules:
        to incur in the risk of being misused or lead to developer confusion, and
        refactoring it to add parentheses breaks its functionality.
      - Tagged as `safe` for ECLAIR.
+
+   * - R20.7
+     - The macros `{COMPILE,RUNTIME}_CHECK` defined in
+       `xen/include/xen/self-tests.h` are allowed not to parenthesize their
+       arguments, to allow function-like macros to be tested as well as
+       functions. Given the specialized use of these macros and their limited
+       usage scope, omitting parentheses is deemed unlikely to cause issues.
+     - Tagged as `deliberate` for ECLAIR.

    * - R20.12
      - Variadic macros that use token pasting often employ the gcc extension
--
2.43.0
Re: [XEN PATCH] automation/eclair_analysis: fix MISRA Rule 20.7 regression in self-tests.h
Posted by Andrew Cooper 2 months, 1 week ago 
On 08/09/2024 2:27 pm, Nicola Vetrini wrote:
> Prior to bd1664db7b7d ("xen/bitops: Introduce a multiple_bits_set() helper")
> the definition of {COMPILE,RUNTIME}_CHECK was fully compliant with respect
> to MISRA C Rule 20.7:
>
> "Expressions resulting from the expansion of macro parameters shall be
> enclosed in parentheses."
>
> However, to allow testing function-like macros, parentheses on the "fn"
> parameter were removed and thus new violations of the rule have been
> introduced. Given the usefulness of this functionality,
> it is deemed ok to deviate these two macros for this rule, because
> their scope of (direct) usage is limited to just the file where they
> are defined, and the possibility of misuses is unlikely.
>
> No functional change.
>
> Fixes: bd1664db7b7d ("xen/bitops: Introduce a multiple_bits_set() helper")
> Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>

Sorry for breaking this.  I did check that Eclair was green with the
patch in place, assuming that this rule was already clean.

Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>

I'll try to keep this in mind when extending self-tests further.
Re: [XEN PATCH] automation/eclair_analysis: fix MISRA Rule 20.7 regression in self-tests.h
Posted by Jan Beulich 2 months, 1 week ago 
On 08.09.2024 15:27, Nicola Vetrini wrote:
> --- a/docs/misra/deviations.rst
> +++ b/docs/misra/deviations.rst
> @@ -533,6 +533,14 @@ Deviations related to MISRA C:2012 Rules:
>         to incur in the risk of being misused or lead to developer confusion, and
>         refactoring it to add parentheses breaks its functionality.
>       - Tagged as `safe` for ECLAIR.
> +
> +   * - R20.7
> +     - The macros `{COMPILE,RUNTIME}_CHECK` defined in
> +       `xen/include/xen/self-tests.h` are allowed not to parenthesize their
> +       arguments, to allow function-like macros to be tested as well as
> +       functions. Given the specialized use of these macros and their limited
> +       usage scope, omitting parentheses is deemed unlikely to cause issues.
> +     - Tagged as `deliberate` for ECLAIR.

As on earlier occasions my take here again is that this is going too far:
Beside the "fn" parameter, all other parameter uses should be properly
parenthesized. I have no idea whether this can be expressed for Eclair,
but at least the verbal deviation description should imo be no more lax
than necessary.

Jan
Re: [XEN PATCH] automation/eclair_analysis: fix MISRA Rule 20.7 regression in self-tests.h
Posted by Stefano Stabellini 2 months, 1 week ago 
Acked-by: Stefano Stabellini <sstabellini@kernel.org>

On Mon, 9 Sep 2024, Jan Beulich wrote:
> On 08.09.2024 15:27, Nicola Vetrini wrote:
> > --- a/docs/misra/deviations.rst
> > +++ b/docs/misra/deviations.rst
> > @@ -533,6 +533,14 @@ Deviations related to MISRA C:2012 Rules:
> >         to incur in the risk of being misused or lead to developer confusion, and
> >         refactoring it to add parentheses breaks its functionality.
> >       - Tagged as `safe` for ECLAIR.
> > +
> > +   * - R20.7
> > +     - The macros `{COMPILE,RUNTIME}_CHECK` defined in
> > +       `xen/include/xen/self-tests.h` are allowed not to parenthesize their
> > +       arguments, to allow function-like macros to be tested as well as
> > +       functions. Given the specialized use of these macros and their limited
> > +       usage scope, omitting parentheses is deemed unlikely to cause issues.
> > +     - Tagged as `deliberate` for ECLAIR.
> 
> As on earlier occasions my take here again is that this is going too far:
> Beside the "fn" parameter, all other parameter uses should be properly
> parenthesized. I have no idea whether this can be expressed for Eclair,
> but at least the verbal deviation description should imo be no more lax
> than necessary.

I can add a mention to the "fn" parameter on commit
Re: [XEN PATCH] automation/eclair_analysis: fix MISRA Rule 20.7 regression in self-tests.h
Posted by Nicola Vetrini 2 months, 1 week ago 
On 2024-09-10 06:39, Stefano Stabellini wrote:
> Acked-by: Stefano Stabellini <sstabellini@kernel.org>
> 
> On Mon, 9 Sep 2024, Jan Beulich wrote:
>> On 08.09.2024 15:27, Nicola Vetrini wrote:
>> > --- a/docs/misra/deviations.rst
>> > +++ b/docs/misra/deviations.rst
>> > @@ -533,6 +533,14 @@ Deviations related to MISRA C:2012 Rules:
>> >         to incur in the risk of being misused or lead to developer confusion, and
>> >         refactoring it to add parentheses breaks its functionality.
>> >       - Tagged as `safe` for ECLAIR.
>> > +
>> > +   * - R20.7
>> > +     - The macros `{COMPILE,RUNTIME}_CHECK` defined in
>> > +       `xen/include/xen/self-tests.h` are allowed not to parenthesize their
>> > +       arguments, to allow function-like macros to be tested as well as
>> > +       functions. Given the specialized use of these macros and their limited
>> > +       usage scope, omitting parentheses is deemed unlikely to cause issues.
>> > +     - Tagged as `deliberate` for ECLAIR.
>> 
>> As on earlier occasions my take here again is that this is going too 
>> far:
>> Beside the "fn" parameter, all other parameter uses should be properly
>> parenthesized. I have no idea whether this can be expressed for 
>> Eclair,
>> but at least the verbal deviation description should imo be no more 
>> lax
>> than necessary.
> 
> I can add a mention to the "fn" parameter on commit

I am ok with that. Unfortunately configuring the tool to only require 
certain parameters to be non-parenthesized is not possible at the 
moment, but it's something that may be considered in the future.

-- 
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.