Re: [Qemu-devel] [PATCH 1/3] Consolidate printing of block driver options

[Stefan Hajnoczi]

On Thu, Dec 2, 2010 at 5:46 PM,  <address@hidden> wrote:
> @@ -188,6 +188,32 @@ static int read_password(char *buf, int buf_size)
>  }
>  #endif
>
> +static int print_block_option_help(const char *filename, const char *fmt)
> +{
> +    BlockDriver *drv, *proto_drv;
> +    QEMUOptionParameter *create_options = NULL;
> +
> +    /* Find driver and parse its options */
> +    drv = bdrv_find_format(fmt);
> +    if (!drv) {
> +        error("Unknown file format '%s'", fmt);
> +        return 1;
> +    }
> +
> +    proto_drv = bdrv_find_protocol(filename);
> +    if (!proto_drv) {
> +        error("Unknown protocol '%s'", filename);
> +        return 1;
> +    }
> +
> +    create_options = append_option_parameters(create_options,
> +                                              drv->create_options);
> +    create_options = append_option_parameters(create_options,
> +                                              proto_drv->create_options);
> +    print_option_help(create_options);

free_option_parameters(create_options);

> @@ -694,6 +720,11 @@ static int img_convert(int argc, char **argv)
>
>     out_filename = argv[argc - 1];
>
> +    if (options && !strcmp(options, "?")) {
> +        ret = print_block_option_help(out_filename, out_fmt);
> +        goto out2;
> +    }
> +
>     if (bs_n > 1 && out_baseimg) {
>         error("-B makes no sense when concatenating multiple input images");
>         return 1;
> @@ -749,10 +780,6 @@ static int img_convert(int argc, char **argv)
>                                               drv->create_options);
>     create_options = append_option_parameters(create_options,
>                                               proto_drv->create_options);
> -    if (options && !strcmp(options, "?")) {
> -        print_option_help(create_options);
> -        goto out;
> -    }
>
>     if (options) {
>         param = parse_option_parameters(options, create_options, param);
> @@ -984,6 +1011,7 @@ out:
>         }
>     }
>     free(bs);
> +out2:

Not needed, out is fine.  All those free functions are nops on NULL pointers.

Even simpler would be to just return early instead of jumping to a
label.  The other checks at the top of the function also do that.

Stefan