[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



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.



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.