[PATCH] xen: add deviations for MISRA C 2012 Rule R5.4

[PATCH] xen: add deviations for MISRA C 2012 Rule R5.4

[Alessandro Zucchelli]

This addresses violations of MISRA C:2012 Rule 5.4 which states as
following: Macro identifiers shall be distinct.

This deviation aims to address violations of Rule 5.4 regarding
identifiers XLAT_hvm_altp2m_set_mem_access_multi_HNDL_pfn_list and
XLAT_hvm_altp2m_set_mem_access_multi_HNDL_access_list, and identifiers
declared in header file include/asm/guest/hyperv-tlfs.h.

No functional change.

Signed-off-by: Alessandro Zucchelli <alessandro.zucchelli@bugseng.com>
---
 automation/eclair_analysis/ECLAIR/deviations.ecl | 9 +++++++++
 docs/misra/deviations.rst                        | 8 ++++++++
 2 files changed, 17 insertions(+)

diff --git a/automation/eclair_analysis/ECLAIR/deviations.ecl b/automation/eclair_analysis/ECLAIR/deviations.ecl
index 2f58f29203..9e780e4465 100644
--- a/automation/eclair_analysis/ECLAIR/deviations.ecl
+++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
@@ -98,6 +98,15 @@ it defines would (in the common case) be already defined. Peer reviewed by the c
 -config=MC3R1.R5.3,reports+={safe, "any_area(any_loc(any_exp(macro(^read_debugreg$))&&any_exp(macro(^write_debugreg$))))"}
 -doc_end
 
+-doc_begin="Identifiers declared in the following header file should not be changed, therefore they are excluded from compliance with this rule."
+-config=MC3R1.R5.4,reports+={safe, "any_area(any_loc(file(^xen/arch/x86/include/asm/guest/hyperv-tlfs\\.h$)))"}
+-doc_end
+
+-doc_begin="The following macro identifiers should not be changed, therefore they are excluded from compliance with this rule."
+-config=MC3R1.R5.4,ignored_macros+=^XLAT_hvm_altp2m_set_mem_access_multi_HNDL_pfn_list$
+-config=MC3R1.R5.4,ignored_macros+=^XLAT_hvm_altp2m_set_mem_access_multi_HNDL_access_list$
+-doc_end
+
 -doc_begin="Macros expanding to their own identifier (e.g., \"#define x x\") are deliberate."
 -config=MC3R1.R5.5,reports+={deliberate, "all_area(macro(same_id_body())||!macro(!same_id_body()))"}
 -doc_end
diff --git a/docs/misra/deviations.rst b/docs/misra/deviations.rst
index 15a993d050..2ce1c8e58a 100644
--- a/docs/misra/deviations.rst
+++ b/docs/misra/deviations.rst
@@ -109,6 +109,14 @@ Deviations related to MISRA C:2012 Rules:
          - __emulate_2op and __emulate_2op_nobyte
          - read_debugreg and write_debugreg
 
+   * - R5.4
+     - Macros XLAT_hvm_altp2m_set_mem_access_multi_HNDL_pfn_list and
+       XLAT_hvm_altp2m_set_mem_access_multi_HNDL_access_list should not be
+       changed, and are therefore ignored by the ECLAIR.
+       Identifiers in header file xen/arch/x86/include/asm/guest/hyperv-tlfs.halder
+       shall not be changed.
+     - Tagged as `safe` for ECLAIR.
+
    * - R5.5
      - Macros expanding to their own name are allowed.
      - Tagged as `deliberate` for ECLAIR.
-- 
2.43.0

Re: [PATCH] xen: add deviations for MISRA C 2012 Rule R5.4

[Simone Ballarin]

On 2024-11-13 10:38, Alessandro Zucchelli wrote:
> This addresses violations of MISRA C:2012 Rule 5.4 which states as
> following: Macro identifiers shall be distinct.
> 
> This deviation aims to address violations of Rule 5.4 regarding
> identifiers XLAT_hvm_altp2m_set_mem_access_multi_HNDL_pfn_list and
> XLAT_hvm_altp2m_set_mem_access_multi_HNDL_access_list, and identifiers
> declared in header file include/asm/guest/hyperv-tlfs.h.
> 
> No functional change.
> 
> Signed-off-by: Alessandro Zucchelli <alessandro.zucchelli@bugseng.com>
> ---
>  automation/eclair_analysis/ECLAIR/deviations.ecl | 9 +++++++++
>  docs/misra/deviations.rst                        | 8 ++++++++
>  2 files changed, 17 insertions(+)
> 
> diff --git a/automation/eclair_analysis/ECLAIR/deviations.ecl 
> b/automation/eclair_analysis/ECLAIR/deviations.ecl
> index 2f58f29203..9e780e4465 100644
> --- a/automation/eclair_analysis/ECLAIR/deviations.ecl
> +++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
> @@ -98,6 +98,15 @@ it defines would (in the common case) be already 
> defined. Peer reviewed by the c
>  -config=MC3R1.R5.3,reports+={safe, 
> "any_area(any_loc(any_exp(macro(^read_debugreg$))&&any_exp(macro(^write_debugreg$))))"}
>  -doc_end
> 
> +-doc_begin="Identifiers declared in the following header file should 
> not be changed, therefore they are excluded from compliance with this 
> rule."
> +-config=MC3R1.R5.4,reports+={safe, 
> "any_area(any_loc(file(^xen/arch/x86/include/asm/guest/hyperv-tlfs\\.h$)))"}
> +-doc_end
> +
> +-doc_begin="The following macro identifiers should not be changed, 
> therefore they are excluded from compliance with this rule."
> +-config=MC3R1.R5.4,ignored_macros+=^XLAT_hvm_altp2m_set_mem_access_multi_HNDL_pfn_list$
> +-config=MC3R1.R5.4,ignored_macros+=^XLAT_hvm_altp2m_set_mem_access_multi_HNDL_access_list$
> +-doc_end
> +
>  -doc_begin="Macros expanding to their own identifier (e.g., \"#define 
> x x\") are deliberate."
>  -config=MC3R1.R5.5,reports+={deliberate, 
> "all_area(macro(same_id_body())||!macro(!same_id_body()))"}
>  -doc_end
> diff --git a/docs/misra/deviations.rst b/docs/misra/deviations.rst
> index 15a993d050..2ce1c8e58a 100644
> --- a/docs/misra/deviations.rst
> +++ b/docs/misra/deviations.rst
> @@ -109,6 +109,14 @@ Deviations related to MISRA C:2012 Rules:
>           - __emulate_2op and __emulate_2op_nobyte
>           - read_debugreg and write_debugreg
> 
> +   * - R5.4
> +     - Macros XLAT_hvm_altp2m_set_mem_access_multi_HNDL_pfn_list and
> +       XLAT_hvm_altp2m_set_mem_access_multi_HNDL_access_list should 
> not be
> +       changed, and are therefore ignored by the ECLAIR.
> +       Identifiers in header file 
> xen/arch/x86/include/asm/guest/hyperv-tlfs.halder
                                                                          
        ^ typo here
I think it is preferable to cite the actual source of those constants, 
but on the other hand, there is a link at the top of the cited header.

> +       shall not be changed.
> +     - Tagged as `safe` for ECLAIR.
> +
>     * - R5.5
>       - Macros expanding to their own name are allowed.
>       - Tagged as `deliberate` for ECLAIR.

Re: [PATCH] xen: add deviations for MISRA C 2012 Rule R5.4

[Jan Beulich]

On 13.11.2024 10:38, Alessandro Zucchelli wrote:
> This addresses violations of MISRA C:2012 Rule 5.4 which states as
> following: Macro identifiers shall be distinct.
> 
> This deviation aims to address violations of Rule 5.4 regarding
> identifiers XLAT_hvm_altp2m_set_mem_access_multi_HNDL_pfn_list and
> XLAT_hvm_altp2m_set_mem_access_multi_HNDL_access_list, and identifiers
> declared in header file include/asm/guest/hyperv-tlfs.h.

Please can you provide enough context? The two identifiers are quite
obviously distinct. Just not in the first 40 characters. A limit I had
to actually go look up, because it's entirely arbitrary.

Plus - what are we going to do if further such identifiers appear?
Exclude them one by one? That wouldn't really scale. Can we perhaps
make a wider exception, e.g. at least for all XLAT_*_HDNL_* ones? Then
again part of the problem here is that hvm_altp2m_set_mem_access_multi
is already excessively long, alone taking up 31 characters.

Jan

Re: [PATCH] xen: add deviations for MISRA C 2012 Rule R5.4

[Stefano Stabellini]

On Wed, 13 Nov 2024, Jan Beulich wrote:
> On 13.11.2024 10:38, Alessandro Zucchelli wrote:
> > This addresses violations of MISRA C:2012 Rule 5.4 which states as
> > following: Macro identifiers shall be distinct.
> > 
> > This deviation aims to address violations of Rule 5.4 regarding
> > identifiers XLAT_hvm_altp2m_set_mem_access_multi_HNDL_pfn_list and
> > XLAT_hvm_altp2m_set_mem_access_multi_HNDL_access_list, and identifiers
> > declared in header file include/asm/guest/hyperv-tlfs.h.
> 
> Please can you provide enough context? The two identifiers are quite
> obviously distinct. Just not in the first 40 characters. A limit I had
> to actually go look up, because it's entirely arbitrary.
> 
> Plus - what are we going to do if further such identifiers appear?
> Exclude them one by one? That wouldn't really scale. Can we perhaps
> make a wider exception, e.g. at least for all XLAT_*_HDNL_* ones? Then
> again part of the problem here is that hvm_altp2m_set_mem_access_multi
> is already excessively long, alone taking up 31 characters.

If the reason for the deviation is solely the length of the identifiers,
I believe we should increase the limit from 40 to 64 characters. If we
make this change, would it resolve the issue?  After that, can we mark
Rule 5.4 as clean?

Regardless, the recent MISRA regressions have demonstrated the need to
act quickly in marking as many rules as clean as possible to prevent
further regressions (new MISRA violations) from entering the codebase.
There are a few pending patches on the list to address only one or two
violations left so that we can mark a rule as clean. I believe we should
accept these patches even if they are not perfect, as the cost of
delaying action would be far greater for everyone involved. Andrew had
to spent a full day this week just to catch up on recent violations that
we would have easily caught if we had marked rules as clean.

Based on that, I think we should take this patch changing the deviation
to cover all XLAT_*_HDNL_*. The change can be made on commit.

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