[PATCH 2/5] xen/common: vm_event: Fix MISRA C 2012 Rule 8.7 violation

Xenia Ragiadakou posted 5 patches 2 years, 5 months ago
There is a newer version of this series
[PATCH 2/5] xen/common: vm_event: Fix MISRA C 2012 Rule 8.7 violation
Posted by Xenia Ragiadakou 2 years, 5 months ago
The function vm_event_wake() is referenced only in vm_event.c.
Change the linkage of the function from external to internal by adding
the storage-class specifier static to the function definition.

This patch aims to resolve indirectly a MISRA C 2012 Rule 8.4 violation warning.

Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com>
---
 xen/common/vm_event.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/common/vm_event.c b/xen/common/vm_event.c
index 0b99a6ea72..ecf49c38a9 100644
--- a/xen/common/vm_event.c
+++ b/xen/common/vm_event.c
@@ -173,7 +173,7 @@ static void vm_event_wake_queued(struct domain *d, struct vm_event_domain *ved)
  * call vm_event_wake() again, ensuring that any blocked vCPUs will get
  * unpaused once all the queued vCPUs have made it through.
  */
-void vm_event_wake(struct domain *d, struct vm_event_domain *ved)
+static void vm_event_wake(struct domain *d, struct vm_event_domain *ved)
 {
     if ( !list_empty(&ved->wq.list) )
         vm_event_wake_queued(d, ved);
-- 
2.34.1
Re: [PATCH 2/5] xen/common: vm_event: Fix MISRA C 2012 Rule 8.7 violation
Posted by Stefano Stabellini 2 years, 5 months ago
On Mon, 27 Jun 2022, Xenia Ragiadakou wrote:
> The function vm_event_wake() is referenced only in vm_event.c.
> Change the linkage of the function from external to internal by adding
> the storage-class specifier static to the function definition.
> 
> This patch aims to resolve indirectly a MISRA C 2012 Rule 8.4 violation warning.
> 
> Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com>

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


> ---
>  xen/common/vm_event.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/xen/common/vm_event.c b/xen/common/vm_event.c
> index 0b99a6ea72..ecf49c38a9 100644
> --- a/xen/common/vm_event.c
> +++ b/xen/common/vm_event.c
> @@ -173,7 +173,7 @@ static void vm_event_wake_queued(struct domain *d, struct vm_event_domain *ved)
>   * call vm_event_wake() again, ensuring that any blocked vCPUs will get
>   * unpaused once all the queued vCPUs have made it through.
>   */
> -void vm_event_wake(struct domain *d, struct vm_event_domain *ved)
> +static void vm_event_wake(struct domain *d, struct vm_event_domain *ved)
>  {
>      if ( !list_empty(&ved->wq.list) )
>          vm_event_wake_queued(d, ved);
> -- 
> 2.34.1
> 
>
Re: [PATCH 2/5] xen/common: vm_event: Fix MISRA C 2012 Rule 8.7 violation
Posted by Jan Beulich 2 years, 5 months ago
On 26.06.2022 23:11, Xenia Ragiadakou wrote:
> The function vm_event_wake() is referenced only in vm_event.c.
> Change the linkage of the function from external to internal by adding
> the storage-class specifier static to the function definition.
> 
> This patch aims to resolve indirectly a MISRA C 2012 Rule 8.4 violation warning.

Actually also for patch 1 - this is slightly confusing, as the title
talks about 8.7. At first I suspected a typo. May I suggest to add
"also" (which I think could be easily done while committing)?

> Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>
Re: [PATCH 2/5] xen/common: vm_event: Fix MISRA C 2012 Rule 8.7 violation
Posted by xenia 2 years, 5 months ago
Hi Jan,

On 6/27/22 10:11, Jan Beulich wrote:

> On 26.06.2022 23:11, Xenia Ragiadakou wrote:
>> The function vm_event_wake() is referenced only in vm_event.c.
>> Change the linkage of the function from external to internal by adding
>> the storage-class specifier static to the function definition.
>>
>> This patch aims to resolve indirectly a MISRA C 2012 Rule 8.4 violation warning.
> Actually also for patch 1 - this is slightly confusing, as the title
> talks about 8.7. At first I suspected a typo. May I suggest to add
> "also" (which I think could be easily done while committing)?


This is actually a violation of MISRA C 2012 Rule 8.7 (Advisory), which 
states that functions referenced in only one translation unit should not 
be defined with external linkage.
This violation triggers a MISRA C 2012 Rule 8.4 violation warning, 
because the function is defined with external linkage without a visible 
declaration at the point of definition.
I thought that this does not make it a violation of MISRA C 2012 Rule 8.4.

>> Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com>
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
>
Re: [PATCH 2/5] xen/common: vm_event: Fix MISRA C 2012 Rule 8.7 violation
Posted by Jan Beulich 2 years, 5 months ago
On 27.06.2022 09:54, xenia wrote:
> On 6/27/22 10:11, Jan Beulich wrote:
>> On 26.06.2022 23:11, Xenia Ragiadakou wrote:
>>> The function vm_event_wake() is referenced only in vm_event.c.
>>> Change the linkage of the function from external to internal by adding
>>> the storage-class specifier static to the function definition.
>>>
>>> This patch aims to resolve indirectly a MISRA C 2012 Rule 8.4 violation warning.
>> Actually also for patch 1 - this is slightly confusing, as the title
>> talks about 8.7. At first I suspected a typo. May I suggest to add
>> "also" (which I think could be easily done while committing)?
> 
> 
> This is actually a violation of MISRA C 2012 Rule 8.7 (Advisory), which 
> states that functions referenced in only one translation unit should not 
> be defined with external linkage.
> This violation triggers a MISRA C 2012 Rule 8.4 violation warning, 
> because the function is defined with external linkage without a visible 
> declaration at the point of definition.
> I thought that this does not make it a violation of MISRA C 2012 Rule 8.4.

I think this is a violation of both rules. It would be a violation of
only 8.7 if the function had a declaration, but still wasn't used
outside its defining CU.

Jan
Re: [PATCH 2/5] xen/common: vm_event: Fix MISRA C 2012 Rule 8.7 violation
Posted by xenia 2 years, 5 months ago
Hi Jan,

On 6/27/22 11:43, Jan Beulich wrote:
> On 27.06.2022 09:54, xenia wrote:
>> On 6/27/22 10:11, Jan Beulich wrote:
>>> On 26.06.2022 23:11, Xenia Ragiadakou wrote:
>>>> The function vm_event_wake() is referenced only in vm_event.c.
>>>> Change the linkage of the function from external to internal by adding
>>>> the storage-class specifier static to the function definition.
>>>>
>>>> This patch aims to resolve indirectly a MISRA C 2012 Rule 8.4 violation warning.
>>> Actually also for patch 1 - this is slightly confusing, as the title
>>> talks about 8.7. At first I suspected a typo. May I suggest to add
>>> "also" (which I think could be easily done while committing)?
>>
>> This is actually a violation of MISRA C 2012 Rule 8.7 (Advisory), which
>> states that functions referenced in only one translation unit should not
>> be defined with external linkage.
>> This violation triggers a MISRA C 2012 Rule 8.4 violation warning,
>> because the function is defined with external linkage without a visible
>> declaration at the point of definition.
>> I thought that this does not make it a violation of MISRA C 2012 Rule 8.4.
> I think this is a violation of both rules. It would be a violation of
> only 8.7 if the function had a declaration, but still wasn't used
> outside its defining CU.

So you are suggesting to change the line
"This patch aims to resolve indirectly a MISRA C 2012 Rule 8.4 violation 
warning."
to
"Also, this patch aims to resolve indirectly a MISRA C 2012 Rule 8.4 
violation warning."

I will wait one more day in case there is some input for the last patch, 
and I will send a v2 with the above changed.

Xenia