On Wed, Jul 27, 2022 at 11:26:15AM +0100, Peter Maydell wrote:
> On Tue, 26 Jul 2022 at 23:30, Richard Henderson
> <richard.hender...@linaro.org> wrote:
> >
> > On 7/26/22 09:32, Peter Maydell wrote:
> > > Coverity complains that in functions like pci_set_word_by_mask()
> > > we might end up shifting by more than 31 bits. This is true,
> > > but only if the caller passes in a zero mask. Help Coverity out
> > > by asserting that the mask argument is valid.
> > >
> > > Fixes: CID 1487168
> > >
> > > Signed-off-by: Peter Maydell <peter.mayd...@linaro.org>
> > > ---
> > > Note that only 1 of these 4 functions is used, and that only
> > > in 2 places in the codebase. In both cases the mask argument
> > > is a compile-time constant.
> > > ---
> > > include/hw/pci/pci.h | 20 ++++++++++++++++----
> > > 1 file changed, 16 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> > > index c79144bc5ef..0392b947b8b 100644
> > > --- a/include/hw/pci/pci.h
> > > +++ b/include/hw/pci/pci.h
> > > @@ -688,7 +688,10 @@ static inline void
> > > pci_set_byte_by_mask(uint8_t *config, uint8_t mask, uint8_t reg)
> > > {
> > > uint8_t val = pci_get_byte(config);
> > > - uint8_t rval = reg << ctz32(mask);
> > > + uint8_t rval;
> > > +
> > > + assert(mask & 0xff);
> >
> > Why the and, especially considering the uint8_t type?
>
> Oops, yes. I think I was mixing up prototypes and thought the
> mask was passed as a 32-bit value in both these functions.
>
> -- PMM
Did you intend to send v2 of this without the &?
--
MST
