Re: [Qemu-devel] [PULL 20/39] cow.c: replace QEMUOptionParameter with QemuOpts

[Peter Maydell]

On 16 June 2014 12:23, Stefan Hajnoczi <address@hidden> wrote:
> From: Chunyan Liu <address@hidden>
>
> Reviewed-by: Eric Blake <address@hidden>
> Reviewed-by: Stefan Hajnoczi <address@hidden>
> Signed-off-by: Dong Xu Wang <address@hidden>
> Signed-off-by: Chunyan Liu <address@hidden>
> Signed-off-by: Stefan Hajnoczi <address@hidden>
> ---
>  block/cow.c | 54 ++++++++++++++++++++++++++----------------------------
>  1 file changed, 26 insertions(+), 28 deletions(-)
>
> diff --git a/block/cow.c b/block/cow.c
> index 7e61024..af85753 100644
> --- a/block/cow.c
> +++ b/block/cow.c
> @@ -324,31 +324,24 @@ static void cow_close(BlockDriverState *bs)
>  {
>  }
>
> -static int cow_create(const char *filename, QEMUOptionParameter *options,
> -                      Error **errp)
> +static int cow_create(const char *filename, QemuOpts *opts, Error **errp)
>  {
>      struct cow_header_v2 cow_header;
>      struct stat st;
>      int64_t image_sectors = 0;
> -    const char *image_filename = NULL;
> +    char *image_filename = NULL;
>      Error *local_err = NULL;
>      int ret;
>      BlockDriverState *cow_bs;
>
>      /* Read out options */
> -    while (options && options->name) {
> -        if (!strcmp(options->name, BLOCK_OPT_SIZE)) {
> -            image_sectors = options->value.n / 512;
> -        } else if (!strcmp(options->name, BLOCK_OPT_BACKING_FILE)) {
> -            image_filename = options->value.s;
> -        }
> -        options++;
> -    }
> +    image_sectors = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0) / 512;
> +    image_filename = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FILE);
>
> -    ret = bdrv_create_file(filename, options, NULL, &local_err);
> +    ret = bdrv_create_file(filename, NULL, opts, &local_err);
>      if (ret < 0) {
>          error_propagate(errp, local_err);
> -        return ret;
> +        goto exit;
>      }
>
>      cow_bs = NULL;

This change means we now have a 'goto exit' before cow_bs
is initialized, and the exit: code path uses cow_bs:

>
>  exit:
> +    g_free(image_filename);
>      bdrv_unref(cow_bs);
>      return ret;
>  }

clang 3.4 correctly warns:

/home/petmay01/linaro/qemu-from-laptop/qemu/block/cow.c:342:9:
warning: variable 'cow_bs' is used uninitialized whenever 'if'
condition is true [-Wsometimes-uninitialized]
    if (ret < 0) {
        ^~~~~~~
/home/petmay01/linaro/qemu-from-laptop/qemu/block/cow.c:386:16: note:
uninitialized use occurs here
    bdrv_unref(cow_bs);
               ^~~~~~
/home/petmay01/linaro/qemu-from-laptop/qemu/block/cow.c:342:5: note:
remove the 'if' if its condition is always false
    if (ret < 0) {
    ^~~~~~~~~~~~~~
/home/petmay01/linaro/qemu-from-laptop/qemu/block/cow.c:335:29: note:
initialize the variable 'cow_bs' to silence this warning
    BlockDriverState *cow_bs;
                            ^
                             = NULL
1 warning generated.

Patch to follow.

thanks
-- PMM