| Message ID | d06ee9f139392045fb8d927ff3a0c38fdc5080c6.1701270983.git.nicola.vetrini@bugseng.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | address some violations of MISRA C Rule 8.4 | expand |
On 29.11.2023 16:24, Nicola Vetrini wrote: > The comment referred to the declaration for do_mca, which > now is part of hypercall-defs.h, therefore the comment is stale. If the comments were stale, the #include-s should also be able to disappear? > --- a/xen/arch/x86/cpu/mcheck/mce.c > +++ b/xen/arch/x86/cpu/mcheck/mce.c > @@ -14,7 +14,7 @@ > #include <xen/cpumask.h> > #include <xen/event.h> > #include <xen/guest_access.h> > -#include <xen/hypercall.h> /* for do_mca */ > +#include <xen/hypercall.h> > #include <xen/cpu.h> Here specifically I think the comment isn't stale, as xen/hypercall.h includes xen/hypercall-defs.h. > --- a/xen/arch/x86/include/asm/hypercall.h > +++ b/xen/arch/x86/include/asm/hypercall.h > @@ -12,7 +12,7 @@ > #include <xen/types.h> > #include <public/physdev.h> > #include <public/event_channel.h> > -#include <public/arch-x86/xen-mca.h> /* for do_mca */ > +#include <public/arch-x86/xen-mca.h> > #include <asm/paging.h> Here otoh I'm not even sure this public header (or the others) is (are) really needed. Jan
On 2023-11-30 17:41, Jan Beulich wrote: > On 29.11.2023 16:24, Nicola Vetrini wrote: >> The comment referred to the declaration for do_mca, which >> now is part of hypercall-defs.h, therefore the comment is stale. > > If the comments were stale, the #include-s should also be able to > disappear? > >> --- a/xen/arch/x86/cpu/mcheck/mce.c >> +++ b/xen/arch/x86/cpu/mcheck/mce.c >> @@ -14,7 +14,7 @@ >> #include <xen/cpumask.h> >> #include <xen/event.h> >> #include <xen/guest_access.h> >> -#include <xen/hypercall.h> /* for do_mca */ >> +#include <xen/hypercall.h> >> #include <xen/cpu.h> > > Here specifically I think the comment isn't stale, as xen/hypercall.h > includes xen/hypercall-defs.h. > Ok, I see your point >> --- a/xen/arch/x86/include/asm/hypercall.h >> +++ b/xen/arch/x86/include/asm/hypercall.h >> @@ -12,7 +12,7 @@ >> #include <xen/types.h> >> #include <public/physdev.h> >> #include <public/event_channel.h> >> -#include <public/arch-x86/xen-mca.h> /* for do_mca */ >> +#include <public/arch-x86/xen-mca.h> >> #include <asm/paging.h> > > Here otoh I'm not even sure this public header (or the others) is (are) > really needed. > I confirm this. It build even without this header.
On 2023-12-01 17:57, Nicola Vetrini wrote: > On 2023-11-30 17:41, Jan Beulich wrote: >> On 29.11.2023 16:24, Nicola Vetrini wrote: >>> The comment referred to the declaration for do_mca, which >>> now is part of hypercall-defs.h, therefore the comment is stale. >> >> If the comments were stale, the #include-s should also be able to >> disappear? >>> --- a/xen/arch/x86/include/asm/hypercall.h >>> +++ b/xen/arch/x86/include/asm/hypercall.h >>> @@ -12,7 +12,7 @@ >>> #include <xen/types.h> >>> #include <public/physdev.h> >>> #include <public/event_channel.h> >>> -#include <public/arch-x86/xen-mca.h> /* for do_mca */ >>> +#include <public/arch-x86/xen-mca.h> >>> #include <asm/paging.h> >> >> Here otoh I'm not even sure this public header (or the others) is >> (are) >> really needed. >> > > I confirm this. It build even without this header. It does appear to be needed after all. I did two differential pipeline runs, and some jobs fail to compile when I remove the header (e.g., [1]). Looking trough the build log, it's not entirely clear what is the relationship, but it seems related to some use of this struct defined in xen-mca.h: typedef struct xen_mc xen_mc_t; DEFINE_XEN_GUEST_HANDLE(xen_mc_t); [1] https://gitlab.com/xen-project/people/bugseng/xen/-/jobs/5675760184
On 04.12.2023 17:26, Nicola Vetrini wrote: > On 2023-12-01 17:57, Nicola Vetrini wrote: >> On 2023-11-30 17:41, Jan Beulich wrote: >>> On 29.11.2023 16:24, Nicola Vetrini wrote: >>>> The comment referred to the declaration for do_mca, which >>>> now is part of hypercall-defs.h, therefore the comment is stale. >>> >>> If the comments were stale, the #include-s should also be able to >>> disappear? > >>>> --- a/xen/arch/x86/include/asm/hypercall.h >>>> +++ b/xen/arch/x86/include/asm/hypercall.h >>>> @@ -12,7 +12,7 @@ >>>> #include <xen/types.h> >>>> #include <public/physdev.h> >>>> #include <public/event_channel.h> >>>> -#include <public/arch-x86/xen-mca.h> /* for do_mca */ >>>> +#include <public/arch-x86/xen-mca.h> >>>> #include <asm/paging.h> >>> >>> Here otoh I'm not even sure this public header (or the others) is >>> (are) >>> really needed. >>> >> >> I confirm this. It build even without this header. > > It does appear to be needed after all. I did two differential pipeline > runs, and some jobs fail to compile when I remove the header (e.g., > [1]). Looking trough the build log, it's not entirely clear what is the > relationship, but it seems related to some use of this struct defined in > xen-mca.h: > > typedef struct xen_mc xen_mc_t; > DEFINE_XEN_GUEST_HANDLE(xen_mc_t); That do_mca()'s parameter type, so in a way the comment is still correct then. Jan
On 2023-12-04 17:40, Jan Beulich wrote: > On 04.12.2023 17:26, Nicola Vetrini wrote: >> On 2023-12-01 17:57, Nicola Vetrini wrote: >>> On 2023-11-30 17:41, Jan Beulich wrote: >>>> On 29.11.2023 16:24, Nicola Vetrini wrote: >>>>> The comment referred to the declaration for do_mca, which >>>>> now is part of hypercall-defs.h, therefore the comment is stale. >>>> >>>> If the comments were stale, the #include-s should also be able to >>>> disappear? >> >>>>> --- a/xen/arch/x86/include/asm/hypercall.h >>>>> +++ b/xen/arch/x86/include/asm/hypercall.h >>>>> @@ -12,7 +12,7 @@ >>>>> #include <xen/types.h> >>>>> #include <public/physdev.h> >>>>> #include <public/event_channel.h> >>>>> -#include <public/arch-x86/xen-mca.h> /* for do_mca */ >>>>> +#include <public/arch-x86/xen-mca.h> >>>>> #include <asm/paging.h> >>>> >>>> Here otoh I'm not even sure this public header (or the others) is >>>> (are) >>>> really needed. >>>> >>> >>> I confirm this. It build even without this header. >> >> It does appear to be needed after all. I did two differential pipeline >> runs, and some jobs fail to compile when I remove the header (e.g., >> [1]). Looking trough the build log, it's not entirely clear what is >> the >> relationship, but it seems related to some use of this struct defined >> in >> xen-mca.h: >> >> typedef struct xen_mc xen_mc_t; >> DEFINE_XEN_GUEST_HANDLE(xen_mc_t); > > That do_mca()'s parameter type, so in a way the comment is still > correct > then. > > Jan Yeah, this patch can be dropped.
diff --git a/xen/arch/x86/cpu/mcheck/mce.c b/xen/arch/x86/cpu/mcheck/mce.c index 779a458cd88f..53493c8e4778 100644 --- a/xen/arch/x86/cpu/mcheck/mce.c +++ b/xen/arch/x86/cpu/mcheck/mce.c @@ -14,7 +14,7 @@ #include <xen/cpumask.h> #include <xen/event.h> #include <xen/guest_access.h> -#include <xen/hypercall.h> /* for do_mca */ +#include <xen/hypercall.h> #include <xen/cpu.h> #include <asm/processor.h> diff --git a/xen/arch/x86/include/asm/hypercall.h b/xen/arch/x86/include/asm/hypercall.h index ec2edc771e9d..76658fff19ff 100644 --- a/xen/arch/x86/include/asm/hypercall.h +++ b/xen/arch/x86/include/asm/hypercall.h @@ -12,7 +12,7 @@ #include <xen/types.h> #include <public/physdev.h> #include <public/event_channel.h> -#include <public/arch-x86/xen-mca.h> /* for do_mca */ +#include <public/arch-x86/xen-mca.h> #include <asm/paging.h> #define __HYPERVISOR_paging_domctl_cont __HYPERVISOR_arch_1
The comment referred to the declaration for do_mca, which now is part of hypercall-defs.h, therefore the comment is stale. No functional change. Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com> --- xen/arch/x86/cpu/mcheck/mce.c | 2 +- xen/arch/x86/include/asm/hypercall.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)