Re: [XEN PATCH 1/7] xen: add declarations for variables where needed

[Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>]

On 03/10/2023 09:18, Roger Pau Monné wrote:
> On Tue, Oct 03, 2023 at 09:05:34AM +0200, Nicola Vetrini wrote:
> > On 02/10/2023 13:07, Roger Pau Monné wrote:
> > > On Mon, Oct 02, 2023 at 09:49:44AM +0200, Nicola Vetrini wrote:
> > > > Some variables with external linkage used in C code do not have
> > > > a visible declaration where they are defined. Providing such
> > > > declaration also resolves violations of MISRA C:2012 Rule 8.4.
> > > >
> > > > Signed-off-by: Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>
> > > > ---
> > > >  xen/arch/arm/include/asm/setup.h  |  3 +++
> > > >  xen/arch/arm/include/asm/smp.h    |  3 +++
> > > >  xen/arch/x86/cpu/mcheck/mce.c     |  6 +++---
> > > >  xen/arch/x86/include/asm/setup.h  |  3 +++
> > > >  xen/arch/x86/irq.c                |  2 +-
> > > >  xen/arch/x86/platform_hypercall.c |  3 ---
> > > >  xen/common/symbols.c              | 17 -----------------
> > > >  xen/include/xen/hypercall.h       |  3 +++
> > > >  xen/include/xen/symbols.h         | 18 ++++++++++++++++++
> > > >  9 files changed, 34 insertions(+), 24 deletions(-)
> > > >
> > > > diff --git a/xen/include/xen/hypercall.h b/xen/include/xen/hypercall.h
> > > > index f307dfb59760..12de5a69b5b1 100644
> > > > --- a/xen/include/xen/hypercall.h
> > > > +++ b/xen/include/xen/hypercall.h
> > > > @@ -24,6 +24,9 @@
> > > >  /* Needs to be after asm/hypercall.h. */
> > > >  #include <xen/hypercall-defs.h>
> > > >
> > > > +/* Declarations for items shared with the compat mode handler. */
> > > > +extern spinlock_t xenpf_lock;
> > >
> > > I'm confused about why this needs to be moved, AFAICT xenpf_lock is
> > > only used in platform_hypercall.c, and the declaration is
> > > unconditional, so there's no definition without declaration issue.
> > >
> >
> > The violation was on ARM code, because of a slight inconsistency:
> > xen/arch/arm/platform_hypercall.c:DEFINE_SPINLOCK(xenpf_lock);
> >
> > xen/arch/x86/platform_hypercall.c:extern spinlock_t xenpf_lock;
> > xen/arch/x86/platform_hypercall.c:DEFINE_SPINLOCK(xenpf_lock);
> >
> > therefore, by moving the extern declaration to the common header there
> > should be no issue.
> Just make xenpf_lock static on arm, that would be a better solution.
> The only need for the lock to be global is because of compat code, and
> arm doesn't seem to need it.
>
> Thanks, Roger.
I'll do that, thanks.

--
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)