[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [XEN PATCH v2] automation/eclair: add deviations for MISRA C:2012 Rule 8.3



Hi,

On 26/10/2023 15:04, Federico Serafini wrote:
On 26/10/23 15:54, Julien Grall wrote:
Hi,

On 26/10/2023 13:13, Federico Serafini wrote:
On 26/10/23 12:25, Julien Grall wrote:
Hi,

On 26/10/2023 11:04, Federico Serafini wrote:
Update ECLAIR configuration to deviate Rule 8.3 ("All declarations of
an object or function shall use the same names and type qualifiers")
for the following functions: guest_walk_tables_[0-9]+_levels().
Update file docs/misra/deviations.rst accordingly.
No functional change.

Signed-off-by: Federico Serafini <federico.serafini@xxxxxxxxxxx>
---
Changes in v2:
   - removed set_px_pminfo() from the scope of the deviation;
   - fixed tag of the commit.
---
  automation/eclair_analysis/ECLAIR/deviations.ecl | 4 ++++
  docs/misra/deviations.rst                        | 6 ++++++
  2 files changed, 10 insertions(+)

diff --git a/automation/eclair_analysis/ECLAIR/deviations.ecl b/automation/eclair_analysis/ECLAIR/deviations.ecl
index d8170106b4..b99dfdafd6 100644
--- a/automation/eclair_analysis/ECLAIR/deviations.ecl
+++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
@@ -204,6 +204,10 @@ const-qualified."
-config=MC3R1.R8.3,reports+={deliberate,"any_area(any_loc(file(adopted_mpparse_r8_3)))&&any_area(any_loc(file(^xen/arch/x86/include/asm/mpspec\\.h$)))"}
  -doc_end
+-doc_begin="For functions guest_walk_tables_[0-9]+_levels(), parameter names of definitions deliberately differ from the ones used in the corresponding declarations."
+-config=MC3R1.R8.3,declarations={deliberate,"^guest_walk_tables_[0-9]+_levels\\(const
 struct vcpu\\*, struct p2m_domain\\*, unsigned long, walk_t\\*, uint32_t, gfn_t, mfn_t, 
void\\*\\)$"}
+-doc_end
+
  -doc_begin="The following variables are compiled in multiple translation units
  belonging to different executables and therefore are safe."
  -config=MC3R1.R8.6,declarations+={safe, "name(current_stack_pointer||bsearch||sort)"}
diff --git a/docs/misra/deviations.rst b/docs/misra/deviations.rst
index 8511a18925..9423b5cd6b 100644
--- a/docs/misra/deviations.rst
+++ b/docs/misra/deviations.rst
@@ -121,6 +121,12 @@ Deviations related to MISRA C:2012 Rules:
           - xen/common/unxz.c
           - xen/common/unzstd.c
+   * - R8.3
+     - In some cases, parameter names used in the function definition
+       deliberately differ from the ones used in the corresponding declaration.

It would be helpful to provide a bit more reasoning in your commit message why this was desired. At least for Arm and common code, I would not want anyone to do that because it adds more confusion.

+     - Tagged as `deliberate` for ECLAIR. Such functions are:
+         - guest_walk_tables_[0-9]+_levels()

I think you want to be a bit mores specific. Other arch may have such function in the function and we don't want to deviate them from the start.

Cheers,


Alright, thanks for the observation.

Actually, I cannot find the original discussion. Do you have link? I am interested to read the reasoning and how many maintainers expressed there view.

Cheers,


The discussion started here:
https://lists.xenproject.org/archives/html/xen-devel/2023-07/msg00122.html

Then, I asked for further suggestions:
https://lists.xenproject.org/archives/html/xen-devel/2023-07/msg00855.html

Thanks! So only Jan really provided feedback here. I don't think this is a good idea to deviate in this case. If we really want to keep in sync and use 'walk' for the name, then we could add a comment after. Something like:

uint32_t walk /* pfec */

Cheers,

--
Julien Grall



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.