| Message ID | 20220626211131.678995-3-burzalodowa@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Fix MISRA C 2012 violations | expand |
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>
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> >
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
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
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 > >
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);
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(-)