Re: [XEN PATCH v2 3/9] x86/irq: tidy switch statement and address MISRA violation

[Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>]

On 2024-04-09 02:14, Stefano Stabellini wrote:
> On Mon, 8 Apr 2024, Jan Beulich wrote:
> > On 05.04.2024 11:14, Nicola Vetrini wrote:
> > > Remove unneded blank lines between switch clauses.
> >
> > NAK for this part again.
> >
> > > --- a/xen/arch/x86/irq.c
> > > +++ b/xen/arch/x86/irq.c
> > > @@ -2882,7 +2882,7 @@ int allocate_and_map_gsi_pirq(struct domain *d, int 
> > index, int *pirq_p)
> > >  int allocate_and_map_msi_pirq(struct domain *d, int index, int *pirq_p,
> > >                                int type, struct msi_info *msi)
> > >  {
> > > -    int irq, pirq, ret;
> > > +    int irq = -1, pirq, ret;
> > >
> > >      ASSERT_PDEV_LIST_IS_READ_LOCKED(d);
> > >
> > > @@ -2892,12 +2892,10 @@ int allocate_and_map_msi_pirq(struct domain *d, int 
> > index, int *pirq_p,
> > >          if ( !msi->table_base )
> > >              msi->entry_nr = 1;
> > >          irq = index;
> > > -        if ( irq == -1 )
> > > -        {
> > > +        fallthrough;
> > >      case MAP_PIRQ_TYPE_MULTI_MSI:
> > > +        if( type == MAP_PIRQ_TYPE_MULTI_MSI || irq == -1 )
> > >              irq = create_irq(NUMA_NO_NODE, true);
> >
> > It may seem small, but this extra comparison already is duplication 
> > I'd rather
> > see avoided. At the very least though you want to clarify in the 
> > description
> > whether the compiler manages to eliminate it again.
> It could just be:
>
>     if ( irq == -1 )
>
> because in the MAP_PIRQ_TYPE_MULTI_MSI case we know the irq will be -1.
> No duplication needed.
Ok then. Didn't know whether I could avoid the check to get this 
structuring.
--
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)