Re: [Qemu-devel] [PATCH v10 07/10] hbitmap: serialization

[Markus Armbruster <armbru@redhat.com>]

John Snow <jsnow@redhat.com> writes:

> From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>
> Functions to serialize / deserialize(restore) HBitmap. HBitmap should be
> saved to linear sequence of bits independently of endianness and bitmap
> array element (unsigned long) size. Therefore Little Endian is chosen.
>
> These functions are appropriate for dirty bitmap migration, restoring
> the bitmap in several steps is available. To save performance, every
> step writes only the last level of the bitmap. All other levels are
> restored by hbitmap_deserialize_finish() as a last step of restoring.
> So, HBitmap is inconsistent while restoring.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> [Fix left shift operand to 1UL; add "finish" parameter. - Fam]
> Signed-off-by: Fam Zheng <famz@redhat.com>
>
> Signed-off-by: John Snow <jsnow@redhat.com>
[...]
> diff --git a/util/hbitmap.c b/util/hbitmap.c
> index f303975..5d1a21c 100644
> --- a/util/hbitmap.c
> +++ b/util/hbitmap.c
> @@ -397,6 +397,143 @@ bool hbitmap_get(const HBitmap *hb, uint64_t item)
>      return (hb->levels[HBITMAP_LEVELS - 1][pos >> BITS_PER_LEVEL] & bit) != 0;
>  }
>  
> +uint64_t hbitmap_serialization_granularity(const HBitmap *hb)
> +{
> +    /* Require at least 64 bit granularity to be safe on both 64 bit and 32 bit
> +     * hosts. */
> +    return 64 << hb->granularity;
> +}

Coverity reports:

    *** CID 1365378:  Integer handling issues  (OVERFLOW_BEFORE_WIDEN)
    /util/hbitmap.c: 404 in hbitmap_serialization_granularity()
    398     }
    399     
    400     uint64_t hbitmap_serialization_granularity(const HBitmap *hb)
    401     {
    402         /* Require at least 64 bit granularity to be safe on both 64 bit and 32 bit
    403          * hosts. */
    >>>     CID 1365378:  Integer handling issues  (OVERFLOW_BEFORE_WIDEN)
    >>>     Potentially overflowing expression "0x40 << hb->granularity" with type "int" (32 bits, signed) is evaluated using 32-bit arithmetic, and then used in a context that expects an expression of type "uint64_t" (64 bits, unsigned).
    404         return 64 << hb->granularity;
    405     }
    406     
    407     /* Start should be aligned to serialization granularity, chunk size should be
    408      * aligned to serialization granularity too, except for last chunk.
    409      */

Use (uint64_t)64 << hb->granularity for an unsigned 64 bit shift.

[...]