Re: [Qemu-devel] [PATCH] qemu-img: Improve error messages

[Eric Blake]

On 04/21/2014 12:23 AM, Fam Zheng wrote:
> Previously, when there is an user error in argv parsing, qemu-img prints

s/an user/a user/

(The rule of thumb for selecting which article to use for a leading 'u'
is pronunciation: anything starting with "you" uses "a", anything with
"uh" uses "an" [e.g. a unicorn under an umbrella].  Similar confusion
for 'h': hard h uses "a", silent h uses "an" [e.g. an hour and a half])

> 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>
> ---
>  qemu-img.c | 71 
> +++++++++++++++++++++++++++++++++++++++-----------------------
>  1 file changed, 45 insertions(+), 26 deletions(-)

>  
> +static void GCC_FMT_ATTR(1, 2) error_exit(const char *fmt, ...)

Should you also mark this QEMU_NORETURN?

> +{
> +    va_list ap;
> +
> +    error_printf("qemu-img: ");
> +
> +    va_start(ap, fmt);
> +    error_vprintf(fmt, ap);
> +    va_end(ap);
> +
> +    error_printf("\nTry 'qemu-img --help' for more information\n");
> +    exit(1);

Worth using EXIT_FAILURE?

> @@ -129,7 +143,7 @@ static void help(void)
>      printf("%s\nSupported formats:", help_msg);
>      bdrv_iterate_format(format_print, NULL);
>      printf("\n");
> -    exit(1);
> +    exit(0);

Worth using EXIT_SUCCESS?

> @@ -2046,10 +2061,10 @@ static int img_map(int argc, char **argv)
>              break;
>          }
>      }
> -    if (optind >= argc) {
> -        help();
> +    if (optind != argc - 1) {
> +        error_exit("Expecting one image file name");
>      }
> -    filename = argv[optind++];
> +    filename = argv[optind];

I had to look at context to see that the increment of optind was indeed
dead code worth removing.

> @@ -2788,6 +2807,6 @@ int main(int argc, char **argv)
>      }
>  
>      /* not found */
> -    help();
> +    error_exit("Command not found: %s", cmdname);
>      return 0;

This return is now dead code; using QEMU_NORETURN would have helped the
compiler flag it.

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

signature.asc
Description: OpenPGP digital signature