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
