Re: [Qemu-devel] [PATCH 4/5] qemu-img: move common options parsing before commands processing

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1717 bytes --]

On 06/02/2016 12:35 PM, Denis V. Lunev wrote:
> This is necessary to enable creation of common qemu-img options which will
> be specified before command.
> 
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> CC: Eric Blake <eblake@redhat.com>
> CC: Paolo Bonzini <pbonzini@redhat.com>
> CC: Stefan Hajnoczi <stefanha@redhat.com>
> CC: Kevin Wolf <kwolf@redhat.com>
> ---
>  qemu-img.c | 31 +++++++++++++++++++------------
>  1 file changed, 19 insertions(+), 12 deletions(-)
> 

> -    /* find the command */
> -    for (cmd = img_cmds; cmd->name != NULL; cmd++) {
> -        if (!strcmp(cmdname, cmd->name)) {
> -            return cmd->handler(argc - 1, argv + 1);
> +    while ((c = getopt_long(argc, argv, "+h", long_options, NULL)) != -1) {
> +        switch (c) {
> +        case 'h':
> +            help();
> +            return 0;
> +        case 'v':

Umm, how is 'v' supposed to be hit if it is not passed in the short
option string summary?  That is, 'qemu-img --version' will work, but
'qemu-img -v' won't; while both 'qemu-img --help' and 'qemu-img -h' work.

> +            printf(QEMU_IMG_VERSION);
> +            return 0;
>          }
>      }
>  
> -    c = getopt_long(argc, argv, "h", long_options, NULL);
> +    cmdname = argv[optind];
>  
> -    if (c == 'h') {
> -        help();
> -    }
> -    if (c == 'v') {

On the other hand, it seems to be a pre-existing bug.  Still, it's worth
fixing while you're touching this.

In addition to moving the option processing, you should also update the
documentation (both --help and man page).

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]