1. Patchew
  2. Xen
  3. View series

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

Federico Serafini posted 1 patch 6 months ago
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/1c146f28cb19607ddd6741de4f7de051894a3381.1698314415.git.federico.serafini@bugseng.com
automation/eclair_analysis/ECLAIR/deviations.ecl | 4 ++++
docs/misra/deviations.rst                        | 6 ++++++
2 files changed, 10 insertions(+)
[XEN PATCH v2] automation/eclair: add deviations for MISRA C:2012 Rule 8.3
Posted by Federico Serafini 6 months ago 
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@bugseng.com>
---
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.
+     - Tagged as `deliberate` for ECLAIR. Such functions are:
+         - guest_walk_tables_[0-9]+_levels()
+
    * - R8.4
      - The definitions present in the files 'asm-offsets.c' for any architecture
        are used to generate definitions for asm modules, and are not called by
-- 
2.34.1
Re: [XEN PATCH v2] automation/eclair: add deviations for MISRA C:2012 Rule 8.3
Posted by Julien Grall 6 months ago 
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@bugseng.com>
> ---
> 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,

-- 
Julien Grall
Re: [XEN PATCH v2] automation/eclair: add deviations for MISRA C:2012 Rule 8.3
Posted by Federico Serafini 6 months ago 
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@bugseng.com>
>> ---
>> 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.

-- 
Federico Serafini, M.Sc.

Software Engineer, BUGSENG (http://bugseng.com)
Re: [XEN PATCH v2] automation/eclair: add deviations for MISRA C:2012 Rule 8.3
Posted by Julien Grall 6 months ago 
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@bugseng.com>
>>> ---
>>> 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,

-- 
Julien Grall
Re: [XEN PATCH v2] automation/eclair: add deviations for MISRA C:2012 Rule 8.3
Posted by Federico Serafini 6 months ago 
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@bugseng.com>
>>>> ---
>>>> 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

-- 
Federico Serafini, M.Sc.

Software Engineer, BUGSENG (http://bugseng.com)
Re: [XEN PATCH v2] automation/eclair: add deviations for MISRA C:2012 Rule 8.3
Posted by Julien Grall 6 months ago 
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@bugseng.com>
>>>>> ---
>>>>> 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
Re: [XEN PATCH v2] automation/eclair: add deviations for MISRA C:2012 Rule 8.3
Posted by Stefano Stabellini 6 months ago 
+Roger

See below

On Thu, 26 Oct 2023, Julien Grall wrote:
> 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@bugseng.com>
> > > > > > ---
> > > > > > 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 */
Re: [XEN PATCH v2] automation/eclair: add deviations for MISRA C:2012 Rule 8.3
Posted by Federico Serafini 5 months, 2 weeks ago 
On 27/10/23 00:55, Stefano Stabellini wrote:
> +Roger
> 
> See below
> 
> On Thu, 26 Oct 2023, Julien Grall wrote:
>> 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@bugseng.com>
>>>>>>> ---
>>>>>>> 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 */

What do you think about "pfec_walk" as parameter name?

-- 
Federico Serafini, M.Sc.

Software Engineer, BUGSENG (http://bugseng.com)
Re: [XEN PATCH v2] automation/eclair: add deviations for MISRA C:2012 Rule 8.3
Posted by Stefano Stabellini 5 months, 1 week ago 
On Fri, 17 Nov 2023, Federico Serafini wrote:
> On 27/10/23 00:55, Stefano Stabellini wrote:
> > +Roger
> > 
> > See below
> > 
> > On Thu, 26 Oct 2023, Julien Grall wrote:
> > > 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@bugseng.com>
> > > > > > > > ---
> > > > > > > > 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 */
> 
> What do you think about "pfec_walk" as parameter name?

I am OK with that

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.