| Message ID | 1c146f28cb19607ddd6741de4f7de051894a3381.1698314415.git.federico.serafini@bugseng.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [XEN,v2] automation/eclair: add deviations for MISRA C:2012 Rule 8.3 | expand |
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,
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.
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,
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
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,
+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 */
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?
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
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
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(+)