qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 5/7] qemu-img create: Support multiple -o option


From: Eric Blake
Subject: Re: [Qemu-devel] [PATCH 5/7] qemu-img create: Support multiple -o options
Date: Wed, 19 Feb 2014 09:46:06 -0700
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.3.0

On 02/19/2014 08:12 AM, Kevin Wolf wrote:
> Multiple -o options has the same meaning as having a single option with
> all settings in the order of their respective -o options.

Ah, this addresses (part of) the problem I raised about patch 1.  But it
feels a bit awkward to have the intermediate state.

>          case 'o':
>              if (is_help_option(optarg)) {
>                  options_help = true;
>              } else if (!options) {
> -                options = optarg;
> +                options = g_strdup(optarg);
>              } else {
> -                error_report("-o cannot be used multiple times. Please use a 
> "
> -                             "single -o option with comma-separated settings 
> "
> -                             "instead.");
> -                return 1;
> +                options = g_strdup_printf("%s,%s", options, optarg);

In addition to the memleak that Fam pointed out, I still think you have
the problem that you aren't detecting:

-o backing_file=/path/to/foo,help

as specifying options_help.  I think you instead need to collect ALL -o
options without special-casing options_help, then at the end, call
contains_help_option which looks for "help" or "?" among all the
comma-separated options.


>  
> +    g_free(options);
>      return 0;
> +
> +fail:
> +    g_free(options);
> +    return 1;
>  }

I'd also like if we started using EXIT_FAILURE instead of the magic
number 1 (but that's probably a separate cleanup).

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]