|
[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.
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |