Re: [PATCH 2/2] migration: Define BLK_MIG_BLOCK_SIZE as unsigned long long

["Dr. David Alan Gilbert" <dgilbert@redhat.com>]

* Peter Maydell (peter.maydell@linaro.org) wrote:
> On Thu, 21 Jul 2022 at 13:07, Dr. David Alan Gilbert
> <dgilbert@redhat.com> wrote:
> >
> > * Peter Maydell (peter.maydell@linaro.org) wrote:
> > > When we use BLK_MIG_BLOCK_SIZE in expressions like
> > > block_mig_state.submitted * BLK_MIG_BLOCK_SIZE, this multiplication
> > > is done as 32 bits, because both operands are 32 bits.  Coverity
> > > complains about possible overflows because we then accumulate that
> > > into a 64 bit variable.
> > >
> > > Define BLK_MIG_BLOCK_SIZE as unsigned long long using the ULL suffix.
> > > The only two current uses of it with this problem are both in
> > > block_save_pending(), so we could just cast to uint64_t there, but
> > > using the ULL suffix is simpler and ensures that we don't
> > > accidentally introduce new variants of the same issue in future.
> > >
> > > Resolves: Coverity CID 1487136, 1487175
> > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> > > ---
> > > I haven't tried to analyse the code to see if the multiplications
> > > could ever actually end up overflowing, but I would assume
> > > probably not.
> > >
> > >  migration/block.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/migration/block.c b/migration/block.c
> > > index 9e5aae58982..3577c815a94 100644
> > > --- a/migration/block.c
> > > +++ b/migration/block.c
> > > @@ -28,7 +28,7 @@
> > >  #include "sysemu/block-backend.h"
> > >  #include "trace.h"
> > >
> > > -#define BLK_MIG_BLOCK_SIZE           (1 << 20)
> > > +#define BLK_MIG_BLOCK_SIZE           (1ULL << 20)
> >
> > Is it a problem that this is passed to bdrv_create_dirty_bitmap that
> > takes a uint32_t ?
> 
> Shouldn't be -- the constant value still fits within 32 bits.

Hmm OK, lets keep an eye out for build problems on any odd combos

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

> -- PMM
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK