Re: [XEN PATCH] arm64/vfp: address MISRA C:2012 Dir 4.3

[Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>]

On 23/08/2023 22:30, Stefano Stabellini wrote:
> On Wed, 23 Aug 2023, Julien Grall wrote:
> > Hi Nicola,
> >
> > On 23/08/2023 17:09, Nicola Vetrini wrote:
> > > On 23/08/2023 16:59, Julien Grall wrote:
> > > > Hi,
> > > >
> > > > On 23/08/2023 15:27, Nicola Vetrini wrote:
> > > > > Directive 4.3 prescribes the following:
> > > > > "Assembly language shall be encapsulated and isolated",
> > > > > on the grounds of improved readability and ease of maintenance.
> > > > > The Directive is violated in this case by asm code in between C code.
> > > > >
> > > > > A macro is the chosen encapsulation mechanism.
> > > >
> > > > I would rather prefer if we use a static inline.
> > >
> > > Just to prevent an possible back and forth on a similar patch:
> > > is it ok to adopt the same approach with the inline asm in
> > > xen/arch/arm/arm64/lib/bitops.c in the definition of the macros
> > > 'bitop' and 'testop'?
> >
> > So, in the VFP I agree that moving the assembly part outside of 
> > vfp_*_state()
> > makes sense even without MISRA. But I don't agree with moving the 
> > assembly
> > code out as the C function is tightly coupled with the assembly code.
> >
> > So this would please MISRA but IHMO would make the code more difficult 
> > to
> > understand. So I think we should deviate for the bitops.
> >
> > Bertrand, Stefano, what do you think?
> I agree. I think bitops.c is already encapsulated and introducing
> additional macros or functions is likely to make the code worse. testop
> and bitop are the encapsulating functions, as you can see there is very
> little else other than the asm volatile and a do/while loop.
Ok, thanks.

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