Re: [Qemu-devel] [PULL 0/7] bitmap export over NBD

[Eric Blake]

On 06/21/2018 12:34 AM, address@hidden wrote:
> Hi,
>
> This series failed address@hidden build test. Please find the testing commands 
> and
> their output below. If you have Docker installed, you can probably reproduce it
> locally.

> /tmp/qemu-test/src/nbd/server.c: In function 'nbd_trip':
> /tmp/qemu-test/src/nbd/server.c:1979:19: error: 'end' may be used uninitialized 
> in this function [-Werror=maybe-uninitialized]
>       *length = end - offset;
>                 ~~~~^~~~~~~~
> /tmp/qemu-test/src/nbd/server.c:1940:30: note: 'end' was declared here
>       uint64_t begin = offset, end;
>                                ^~~

This gcc is too stupid to see that end is always initialized:

> +/*
> + * Populate @extents from a dirty bitmap. Unless @dont_fragment, the
> + * final extent may exceed the original @length. Store in @length the
> + * byte length encoded (which may be smaller or larger than the
> + * original), and return the number of extents used.
> + */
> +static unsigned int bitmap_to_extents(BdrvDirtyBitmap *bitmap, uint64_t offset,
> +                                      uint64_t *length, NBDExtent *extents,
> +                                      unsigned int nb_extents,
> +                                      bool dont_fragment)
> +{
> +    uint64_t begin = offset, end;
> +    uint64_t overall_end = offset + *length;

On input, *length is a non-zero 32-bit number (gcc can't see that)...

> +    unsigned int i = 0;
> +    BdrvDirtyBitmapIter *it;
> +    bool dirty;
> +
> +    bdrv_dirty_bitmap_lock(bitmap);
> +
> +    it = bdrv_dirty_iter_new(bitmap);
> +    dirty = bdrv_get_dirty_locked(NULL, bitmap, offset);
> +
> +    while (begin < overall_end && i < nb_extents) {

...so this loop always iterates at least once...

> +        if (dirty) {
> +            end = bdrv_dirty_bitmap_next_zero(bitmap, begin);
> +        } else {
> +            bdrv_set_dirty_iter(it, begin);
> +            end = bdrv_dirty_iter_next(it);
> +        }

...and end is always initialized...

> +        if (end == -1) {
> +            /* Cap to an aligned value < 4G beyond begin. */
> +            end = MIN(bdrv_dirty_bitmap_size(bitmap),
> +                      begin + 0x100000000ULL -
> +                      bdrv_dirty_bitmap_granularity(bitmap));
> +        }
> +        if (dont_fragment && end > overall_end) {
> +            end = overall_end;
> +        }
> +
> +        extents[i].length = cpu_to_be32(end - begin);
> +        extents[i].flags = cpu_to_be32(dirty ? NBD_STATE_DIRTY : 0);
> +        i++;
> +        begin = end;
> +        dirty = !dirty;
> +    }
> +
> +    bdrv_dirty_iter_free(it);
> +
> +    bdrv_dirty_bitmap_unlock(bitmap);
> +
> +    *length = end - offset;

...but gcc didn't know that.

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org