Re: [PATCH] vPCI: resolve MISRA R10.1 boolean arithmetic type violation

["Roger Pau Monné" <roger.pau@citrix.com>]

On Tue, May 26, 2026 at 03:12:23PM -0700, Stefano Stabellini wrote:
> On Fri, 22 May 2026, Jan Beulich wrote:
> > (extending Cc list)
> > 
> > On 22.05.2026 08:13, Dmytro Prokopchuk1 wrote:
> > > --- a/xen/drivers/vpci/header.c
> > > +++ b/xen/drivers/vpci/header.c
> > > @@ -586,7 +586,7 @@ static void cf_check bar_write(
> > >          if ( val != (uint32_t)(bar->addr >> (hi ? 32 : 0)) )
> > >              gprintk(XENLOG_WARNING,
> > >                      "%pp: ignored BAR %zu write while mapped\n",
> > > -                    &pdev->sbdf, bar - pdev->vpci->header.bars + hi);
> > > +                    &pdev->sbdf, bar - pdev->vpci->header.bars + (hi ? 1 : 0));
> > >          return;
> > >      }
> > >  
> > > @@ -647,7 +647,7 @@ static void cf_check guest_mem_bar_write(const struct pci_dev *pdev,
> > >          if ( guest_addr != bar->guest_addr )
> > >              gprintk(XENLOG_WARNING,
> > >                      "%pp: ignored guest BAR %zu write while mapped\n",
> > > -                    &pdev->sbdf, bar - pdev->vpci->header.bars + hi);
> > > +                    &pdev->sbdf, bar - pdev->vpci->header.bars + (hi ? 1 : 0));
> > >          return;
> > >      }
> > >      bar->guest_addr = guest_addr;
> > 
> > Well. If I'm not mistaken we had discussed situations like this (long ago).
> > Imo the added verbosity gets in the way of readability. If we absolutely
> > cannot or don't want to deviate such constructs (of which I expect we have
> > more), then we ought to consider alternatives (like changing the variables'
> > types in the case here).
> > 
> > As to deviating: rules.rst, according to my reading, says that &, |, ^, or
> > shifts would be okay to use with a bool operand. What's wrong with also
> > permitting this for other operators?
> 
> In my opinion, if we are going to treat bool as its own type, it makes
> sense not to silently mix bools into arithmetic with int types. I also
> do not find this patch less readable -- I actually find it more
> readable, since it makes it more obvious that hi is a bool.

Another possibly less controversial option is doing the arithmetic
based on the register value directly: (reg - PCI_BASE_ADDRESS_0) / 4.

TBH that's likely faster than the dereferences done to get the base
address of the bars array.

Thanks, Roger.