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

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



On Tue, 2 Jun 2026, Jan Beulich wrote:
> On 27.05.2026 00:12, 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.
> 
> Well, okay, we have different opinions there. This reply of yours applies
> to the first paragraph of my earlier reply though, despite its placement.
> What about the aspect mentioned in the second paragraph?

You mean "then we ought to consider alternatives (like changing the
variables' types in the case here)" ?

Other alternatives could be OK, but also this patch as-is is OK to me.



 


Rackspace

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