Re: [PATCH] x86: Address MISRA Rule 13.6

[Federico Serafini <federico.serafini@xxxxxxxxxxx>]

On 02/04/24 18:09, Andrew Cooper wrote:
> On 02/04/2024 5:06 pm, Jan Beulich wrote:
> > On 02.04.2024 17:54, Andrew Cooper wrote:
> > > On 02/04/2024 4:46 pm, Jan Beulich wrote:
> > > > On 02.04.2024 17:43, Andrew Cooper wrote:
> > > > > MISRA Rule 13.6 doesn't like having an expression in a sizeof() which
> > > > > potentially has side effects.
> > > > >
> > > > > Address several violations by pulling the expression out into a local
> > > > > variable.
> > > > >
> > > > > No functional change.
> > > > >
> > > > > Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> > > > Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
> > > > with one caveat:
> > > >
> > > > > --- a/xen/arch/x86/irq.c
> > > > > +++ b/xen/arch/x86/irq.c
> > > > > @@ -1150,8 +1150,9 @@ static void cf_check irq_guest_eoi_timer_fn(void *data)
> > > > >       {
> > > > >           struct domain *d = action->guest[i];
> > > > >           unsigned int pirq = domain_irq_to_pirq(d, irq);
> > > > > +        struct pirq *pirq_info = pirq_info(d, pirq);
> > > > Misra won't like the var's name matching the macro's. Can we go with just
> > > > "info"?
> > > Ah - missed that.
> > >
> > > I can name it to just info, but I considered "struct pirq *info" to be a
> > > little odd.
> > I agree, but what do you do with another "pirq" already there.
> >
> > Or wait, what about
> >
> >          struct pirq *pirq = pirq_info(d, domain_irq_to_pirq(d, irq));
> >
> > ?
> That should work.  I'll switch to this locally, and wait for the
> feedback on whether the patch works for 13.6.
I confirm that both versions of the patch address some violations of
13.6.

--
Federico Serafini, M.Sc.

Software Engineer, BUGSENG (http://bugseng.com)