Am 27.09.2024 um 08:10 hat Michael Tokarev geschrieben:
> In order to correctly print executable name in various
> error messages, pass argv[0] to error_exit() function.
> This way, error messages will refer to actual executable
> name, which may be different from 'qemu-img'.
>
> For subcommands, pass original command name from the
> qemu-img argv[0], plus the subcommand name, as its own
> argv[0] element, so error messages can be more useful.
> Also don't require at least 3 options on the command
> line: it makes no sense with options before subcommand.
>
> Introduce tryhelp() function which just prints
>
> try 'command-name --help' for more info
>
> and exits. When tryhelp() is called from within a subcommand
> handler, the message will look like:
>
> try 'command-name subcommand --help' for more info
>
> qemu-img uses getopt_long() with ':' as the first char in
> optstring parameter, which means it doesn't print error
> messages but return ':' or '?' instead, and qemu-img uses
> unrecognized_option() or missing_argument() function to
> print error messages. But it doesn't quite work:
>
> $ ./qemu-img -xx
> qemu-img: unrecognized option './qemu-img'
>
> so the aim is to let getopt_long() to print regular error
> messages instead (removing ':' prefix from optstring) and
> remove handling of '?' and ':' "options" entirely. With
> concatenated argv[0] and the subcommand, it all finally
> does the right thing in all cases. This will be done in
> subsequent changes command by command, with main() done
> last.
>
> unrecognized_option() and missing_argument() functions
> prototypes aren't changed by this patch, since they're
> called from many places and will be removed a few patches
> later. Only artifical "qemu-img" argv0 is provided in
> there for now.
>
> Signed-off-by: Michael Tokarev <m...@tls.msk.ru>
> ---
> qemu-img.c | 80 +++++++++++++++++++++++++++++-------------------------
> 1 file changed, 43 insertions(+), 37 deletions(-)
>
> diff --git a/qemu-img.c b/qemu-img.c
> index fe22986931..130188e287 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -101,8 +101,15 @@ static void format_print(void *opaque, const char *name)
> printf(" %s", name);
> }
>
> -static G_NORETURN G_GNUC_PRINTF(1, 2)
> -void error_exit(const char *fmt, ...)
> +static G_NORETURN
> +void tryhelp(const char *argv0)
> +{
> + error_printf("Try '%s --help' for more info\n", argv0);
> + exit(EXIT_FAILURE);
> +}
> +
> +static G_NORETURN G_GNUC_PRINTF(2, 3)
> +void error_exit(const char *argv0, const char *fmt, ...)
> {
> va_list ap;
>
> @@ -110,20 +117,19 @@ void error_exit(const char *fmt, ...)
> error_vreport(fmt, ap);
> va_end(ap);
>
> - error_printf("Try 'qemu-img --help' for more information\n");
> - exit(EXIT_FAILURE);
> + tryhelp(argv0);
> }
No reason to change "information" into the more colloquial "info". I
would leave it as it is.
Apart from that:
Reviewed-by: Kevin Wolf <kw...@redhat.com>
