18.04.2014 18:29, Eric Blake wrote:
> On 04/18/2014 12:23 AM, Fam Zheng wrote:
>> Signed-off-by: Fam Zheng <f...@redhat.com>
>> ---
>> qemu-img.c | 68
>> +++++++++++++++++++++++++++++++-------------------------------
>> 1 file changed, 34 insertions(+), 34 deletions(-)
>>
>
>> /* Please keep in synch with qemu-img.texi */
>> -static void help(void)
>> +static void help(bool error)
>
> This doesn't intuitively tell me whether 'true' is success (0 status) or
> failure (non-zero status).
>
>> {
>> const char *help_msg =
>> "qemu-img version " QEMU_VERSION ", Copyright (c) 2004-2008
>> Fabrice Bellard\n"
>> @@ -129,7 +129,7 @@ static void help(void)
>> printf("%s\nSupported formats:", help_msg);
>> bdrv_iterate_format(format_print, NULL);
>> printf("\n");
>> - exit(1);
>> + exit(error ? 1 : 0);
>
> Oh - true for failure. I'd MUCH rather see:
As I already pointed out in a comment for similar patch for ./configure,
maybe it is better to _not_ print help output in case of error, because
this way, the most important error message scrolls up the screen replaced
by the help text. Instead, it might be better to say something like,
run ./qemu-img --help for list of available options
and exit non-zero in case of some option error instead of printing
whole help text. This way, there's no need to pass anything to the
help() function.
Thanks,
/mjt
