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

[Nicola Vetrini <nicola.vetrini@bugseng.com>]

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@bugseng.com>
>> > > ---
>> > >  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)