Re: [Qemu-devel] [PULL 14/16] qemu-img: Improve error messages

[Fam Zheng]

On Fri, 04/25 14:18, Jeff Cody wrote:
> On Wed, Apr 23, 2014 at 12:04:49PM +0200, Kevin Wolf wrote:
> > From: Fam Zheng <address@hidden>
> > 
> > Previously, when there is a user error in argv parsing, qemu-img prints
> > help text and exits.
> > 
> > Add an error_exit function to print a helpful error message and a hint
> > to run 'qemu-img --help' for more information.
> > 
> > As a bonus, "qemu-img <cmd> --help" now has a more reasonable exit code
> > 0.
> > 
> > In the future the help text should be split by sub command, and only
> > print the information for the specified command.
> > 
> > Signed-off-by: Fam Zheng <address@hidden>
> > Signed-off-by: Kevin Wolf <address@hidden>
> > ---
> 
> This breaks 'qemu-img --help':
> 
> ./qemu-img --help
> qemu-img: Command not found: --help
> Try 'qemu-img --help' for more information
> 
> See below:
> 
> 
> <snip>
>   
> >      bs = bdrv_new_open("image", filename, fmt,
> > @@ -2781,8 +2799,9 @@ int main(int argc, char **argv)
> >  
> >      qemu_init_main_loop();
> >      bdrv_init();
> > -    if (argc < 2)
> > -        help();
> > +    if (argc < 2) {
> > +        error_exit("Not enough arguments");
> > +    }
> 
> As an aside, are we sure we want './qemu-img' with no arguments to not
> return the full help message?
> 
> >      cmdname = argv[1];
> >      argc--; argv++;
> >  
> > @@ -2794,6 +2813,5 @@ int main(int argc, char **argv)
> >      }
> >  
> >      /* not found */
> > -    help();
> > -    return 0;
> > +    error_exit("Command not found: %s", cmdname);
> 
> Looks like we just relied previously on the default 'not found' case
> for help() to provide the "--help" option.
> 

Oops! Thanks for noticing this and sending the fix.

Fam