Re: [XEN PATCH 4/8] xen/arm: address MISRA C:2012 Rule 8.4

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Wed, 9 Aug 2023, Jan Beulich wrote:
> On 09.08.2023 13:02, Nicola Vetrini wrote:
> > 'xen/hypercall.h' is included in 'xen/arch/arm/setup.c' to allow
> > the declaration of 'arch_get_xen_caps' to be visible when
> > defining the function.
> > 
> > The header 'xen/delay.h' is included in 'xen/arch/arm/time.c'
> > to allow the declaration of 'udelay' to be visible.
> > 
> > At the same time, a declaration for 'get_sec' is added in
> > 'xen/include/xen/time.h' to be available for every call site
> > (in particular cper.h).
> > 
> > Signed-off-by: Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>
> > ---
> >  xen/arch/arm/setup.c   | 1 +
> >  xen/arch/arm/time.c    | 1 +
> >  xen/include/xen/cper.h | 3 +--
> >  xen/include/xen/time.h | 1 +
> >  4 files changed, 4 insertions(+), 2 deletions(-)
> 
> I would have almost put this off as Arm-only, but then saw this diffstat.
> How come you consider cper.h Arm-related? This is used only by APEI source
> files, which in turn are used only by x86 afaics. So I think you want to
> split off the movement of the get_sec() declaration.
> 
> As to title and description (perhaps affecting more than just this patch):
> Failing to have declarations in scope where definitions appear is an at
> least latent bug. We fix these as we find them anyway, so Misra is
> secondary here. Hence I'd like to suggest that you declare any such
> change as an actual bugfix, ideally including a Fixes: tag. It is of
> course fine to mention that this then also addresses Misra rule 8.4.

As you split the patches moving cper.h out, and fixing the commit
message, please add my Reviewed-by for the setup.c/time.c/time.h
changes.