| Message ID | 255ae80cc8b95f33daa7534c9552c571391cf689.1731490650.git.alessandro.zucchelli@bugseng.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | xen: add deviations for MISRA C 2012 Rule R5.4 | expand |
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.
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
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>
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.
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(+)