| Message ID | 20220705210218.483854-5-burzalodowa@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Fix MISRA C 2012 violations | expand |
Hi Xenia, On 05/07/2022 22:02, Xenia Ragiadakou wrote: > The function pv_console_evtchn() is defined in the header <xen/pv_console.h>. > If the header happens to be included by multiple files, this can result in > linker errors due to multiple definitions, > So, it is more appropriate to resolve this MISRA C 2012 Rule 8.4 violation > warning by making pv_console_evtchn() inline with internal linkage. There are multiple version of pv_console_evtchn(). The version below is only used when !CONFIG_XEN_GUEST. The header is also included multiple time within Xen. So I am a bit puzzled why we haven't seen this error before. Maybe this is unused when !CONFIG_XEN_GUEST? If yes, I would remove the call. If no, then I think the commit message should be clarified. Cheers,
Hi Julien, On 7/6/22 00:38, Julien Grall wrote: > Hi Xenia, > > On 05/07/2022 22:02, Xenia Ragiadakou wrote: >> The function pv_console_evtchn() is defined in the header >> <xen/pv_console.h>. >> If the header happens to be included by multiple files, this can >> result in >> linker errors due to multiple definitions, >> So, it is more appropriate to resolve this MISRA C 2012 Rule 8.4 >> violation >> warning by making pv_console_evtchn() inline with internal linkage. > > There are multiple version of pv_console_evtchn(). The version below is > only used when !CONFIG_XEN_GUEST. > > The header is also included multiple time within Xen. So I am a bit > puzzled why we haven't seen this error before. Maybe this is unused when > !CONFIG_XEN_GUEST? > > If yes, I would remove the call. If no, then I think the commit message > should be clarified. You are right. I had to clarify that this holds when !CONFIG_XEN_GUEST. So when !CONFIG_XEN_GUEST, the function pv_console_evtchn() is defined inside the header file and the header is included only by a single file. This is why the error has not been triggered. > > Cheers, >
On 06.07.2022 00:02, Xenia Ragiadakou wrote: > Hi Julien, > > On 7/6/22 00:38, Julien Grall wrote: >> Hi Xenia, >> >> On 05/07/2022 22:02, Xenia Ragiadakou wrote: >>> The function pv_console_evtchn() is defined in the header >>> <xen/pv_console.h>. >>> If the header happens to be included by multiple files, this can >>> result in >>> linker errors due to multiple definitions, >>> So, it is more appropriate to resolve this MISRA C 2012 Rule 8.4 >>> violation >>> warning by making pv_console_evtchn() inline with internal linkage. >> >> There are multiple version of pv_console_evtchn(). The version below is >> only used when !CONFIG_XEN_GUEST. >> >> The header is also included multiple time within Xen. So I am a bit >> puzzled why we haven't seen this error before. Maybe this is unused when >> !CONFIG_XEN_GUEST? >> >> If yes, I would remove the call. If no, then I think the commit message >> should be clarified. > > You are right. I had to clarify that this holds when !CONFIG_XEN_GUEST. > So when !CONFIG_XEN_GUEST, the function pv_console_evtchn() is defined > inside the header file and the header is included only by a single file. > This is why the error has not been triggered. Irrespective of that it is as Julien suspects: The function is only ever referenced when XEN_GUEST. Hence the better course of action indeed looks to be to ditch the unused stub; we don't even need DCE here for there to not be any references. Jan
On 7/6/22 10:18, Jan Beulich wrote: > On 06.07.2022 00:02, Xenia Ragiadakou wrote: >> Hi Julien, >> >> On 7/6/22 00:38, Julien Grall wrote: >>> Hi Xenia, >>> >>> On 05/07/2022 22:02, Xenia Ragiadakou wrote: >>>> The function pv_console_evtchn() is defined in the header >>>> <xen/pv_console.h>. >>>> If the header happens to be included by multiple files, this can >>>> result in >>>> linker errors due to multiple definitions, >>>> So, it is more appropriate to resolve this MISRA C 2012 Rule 8.4 >>>> violation >>>> warning by making pv_console_evtchn() inline with internal linkage. >>> >>> There are multiple version of pv_console_evtchn(). The version below is >>> only used when !CONFIG_XEN_GUEST. >>> >>> The header is also included multiple time within Xen. So I am a bit >>> puzzled why we haven't seen this error before. Maybe this is unused when >>> !CONFIG_XEN_GUEST? >>> >>> If yes, I would remove the call. If no, then I think the commit message >>> should be clarified. >> >> You are right. I had to clarify that this holds when !CONFIG_XEN_GUEST. >> So when !CONFIG_XEN_GUEST, the function pv_console_evtchn() is defined >> inside the header file and the header is included only by a single file. >> This is why the error has not been triggered. > > Irrespective of that it is as Julien suspects: The function is only ever > referenced when XEN_GUEST. Hence the better course of action indeed looks > to be to ditch the unused stub; we don't even need DCE here for there to > not be any references. Yes, if the function is not used at all when XEN_GUEST, then I think its definition is considered unreachable code (Rule 2.1) and needs to be removed.
diff --git a/xen/include/xen/pv_console.h b/xen/include/xen/pv_console.h index 4745f46f2d..a96a6368b1 100644 --- a/xen/include/xen/pv_console.h +++ b/xen/include/xen/pv_console.h @@ -19,7 +19,7 @@ static inline void pv_console_set_rx_handler(serial_rx_fn fn) { } static inline void pv_console_init_postirq(void) { } static inline void pv_console_puts(const char *buf, size_t nr) { } static inline size_t pv_console_rx(struct cpu_user_regs *regs) { return 0; } -evtchn_port_t pv_console_evtchn(void) +static inline evtchn_port_t pv_console_evtchn(void) { ASSERT_UNREACHABLE(); return 0;
The function pv_console_evtchn() is defined in the header <xen/pv_console.h>. If the header happens to be included by multiple files, this can result in linker errors due to multiple definitions, So, it is more appropriate to resolve this MISRA C 2012 Rule 8.4 violation warning by making pv_console_evtchn() inline with internal linkage. Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com> --- xen/include/xen/pv_console.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)