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 whole argv[] array, so argv[0] is
the executable name, not subcommand name. In order to
do that, avoid resetting optind but continue with the
next option. Also don't require at least 3 options on
the command line: it makes no sense with options before
subcommand.
Before invoking a subcommand, replace argv[0] to include
the subcommand name.
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 <mjt@tls.msk.ru>
---
qemu-img.c | 75 +++++++++++++++++++++++++++---------------------------
1 file changed, 38 insertions(+), 37 deletions(-)
diff --git a/qemu-img.c b/qemu-img.c
index df425b2517..44dbf5be4f 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);
}
static G_NORETURN
void missing_argument(const char *option)
{
- error_exit("missing argument for option '%s'", option);
+ error_exit("qemu-img", "missing argument for option '%s'", option);
}
static G_NORETURN
void unrecognized_option(const char *option)
{
- error_exit("unrecognized option '%s'", option);
+ error_exit("qemu-img", "unrecognized option '%s'", option);
}
/* Please keep in synch with docs/tools/qemu-img.rst */
@@ -576,7 +582,7 @@ static int img_create(int argc, char **argv)
}
if (optind >= argc) {
- error_exit("Expecting image file name");
+ error_exit(argv[0], "Expecting image file name");
}
optind++;
@@ -588,7 +594,7 @@ static int img_create(int argc, char **argv)
}
}
if (optind != argc) {
- error_exit("Unexpected argument: %s", argv[optind]);
+ error_exit(argv[0], "Unexpected argument: %s", argv[optind]);
}
bdrv_img_create(filename, fmt, base_filename, base_fmt,
@@ -770,7 +776,7 @@ static int img_check(int argc, char **argv)
} else if (!strcmp(optarg, "all")) {
fix = BDRV_FIX_LEAKS | BDRV_FIX_ERRORS;
} else {
- error_exit("Unknown option value for -r "
+ error_exit(argv[0], "Unknown option value for -r "
"(expecting 'leaks' or 'all'): %s", optarg);
}
break;
@@ -795,7 +801,7 @@ static int img_check(int argc, char **argv)
}
}
if (optind != argc - 1) {
- error_exit("Expecting one image file name");
+ error_exit(argv[0], "Expecting one image file name");
}
filename = argv[optind++];
@@ -1025,7 +1031,7 @@ static int img_commit(int argc, char **argv)
}
if (optind != argc - 1) {
- error_exit("Expecting one image file name");
+ error_exit(argv[0], "Expecting one image file name");
}
filename = argv[optind++];
@@ -1446,7 +1452,7 @@ static int img_compare(int argc, char **argv)
if (optind != argc - 2) {
- error_exit("Expecting two image file names");
+ error_exit(argv[0], "Expecting two image file names");
}
filename1 = argv[optind++];
filename2 = argv[optind++];
@@ -3056,7 +3062,7 @@ static int img_info(int argc, char **argv)
}
}
if (optind != argc - 1) {
- error_exit("Expecting one image file name");
+ error_exit(argv[0], "Expecting one image file name");
}
filename = argv[optind++];
@@ -3296,7 +3302,7 @@ static int img_map(int argc, char **argv)
}
}
if (optind != argc - 1) {
- error_exit("Expecting one image file name");
+ error_exit(argv[0], "Expecting one image file name");
}
filename = argv[optind];
@@ -3411,7 +3417,7 @@ static int img_snapshot(int argc, char **argv)
return 0;
case 'l':
if (action) {
- error_exit("Cannot mix '-l', '-a', '-c', '-d'");
+ error_exit(argv[0], "Cannot mix '-l', '-a', '-c', '-d'");
return 0;
}
action = SNAPSHOT_LIST;
@@ -3419,7 +3425,7 @@ static int img_snapshot(int argc, char **argv)
break;
case 'a':
if (action) {
- error_exit("Cannot mix '-l', '-a', '-c', '-d'");
+ error_exit(argv[0], "Cannot mix '-l', '-a', '-c', '-d'");
return 0;
}
action = SNAPSHOT_APPLY;
@@ -3427,7 +3433,7 @@ static int img_snapshot(int argc, char **argv)
break;
case 'c':
if (action) {
- error_exit("Cannot mix '-l', '-a', '-c', '-d'");
+ error_exit(argv[0], "Cannot mix '-l', '-a', '-c', '-d'");
return 0;
}
action = SNAPSHOT_CREATE;
@@ -3435,7 +3441,7 @@ static int img_snapshot(int argc, char **argv)
break;
case 'd':
if (action) {
- error_exit("Cannot mix '-l', '-a', '-c', '-d'");
+ error_exit(argv[0], "Cannot mix '-l', '-a', '-c', '-d'");
return 0;
}
action = SNAPSHOT_DELETE;
@@ -3457,7 +3463,7 @@ static int img_snapshot(int argc, char **argv)
}
if (optind != argc - 1) {
- error_exit("Expecting one image file name");
+ error_exit(argv[0], "Expecting one image file name");
}
filename = argv[optind++];
@@ -3624,10 +3630,11 @@ static int img_rebase(int argc, char **argv)
}
if (optind != argc - 1) {
- error_exit("Expecting one image file name");
+ error_exit(argv[0], "Expecting one image file name");
}
if (!unsafe && !out_baseimg) {
- error_exit("Must specify backing file (-b) or use unsafe mode (-u)");
+ error_exit(argv[0],
+ "Must specify backing file (-b) or use unsafe mode (-u)");
}
filename = argv[optind++];
@@ -4051,7 +4058,7 @@ static int img_resize(int argc, char **argv)
/* Remove size from argv manually so that negative numbers are not treated
* as options by getopt. */
if (argc < 3) {
- error_exit("Not enough arguments");
+ error_exit(argv[0], "Not enough arguments");
return 1;
}
@@ -4109,7 +4116,7 @@ static int img_resize(int argc, char **argv)
}
}
if (optind != argc - 1) {
- error_exit("Expecting image file name and size");
+ error_exit(argv[0], "Expecting image file name and size");
}
filename = argv[optind++];
@@ -4306,7 +4313,7 @@ static int img_amend(int argc, char **argv)
}
if (!options) {
- error_exit("Must specify options (-o)");
+ error_exit(argv[0], "Must specify options (-o)");
}
if (quiet) {
@@ -4668,7 +4675,7 @@ static int img_bench(int argc, char **argv)
}
if (optind != argc - 1) {
- error_exit("Expecting one image file name");
+ error_exit(argv[0], "Expecting one image file name");
}
filename = argv[argc - 1];
@@ -5556,9 +5563,6 @@ int main(int argc, char **argv)
module_call_init(MODULE_INIT_QOM);
bdrv_init();
- if (argc < 2) {
- error_exit("Not enough arguments");
- }
qemu_add_opts(&qemu_source_opts);
qemu_add_opts(&qemu_trace_opts);
@@ -5583,15 +5587,11 @@ int main(int argc, char **argv)
}
}
- cmdname = argv[optind];
-
- /* reset getopt_long scanning */
- argc -= optind;
- if (argc < 1) {
- return 0;
+ if (optind >= argc) {
+ error_exit(argv[0], "Not enough arguments");
}
- argv += optind;
- qemu_reset_optind();
+
+ cmdname = argv[optind++];
if (!trace_init_backends()) {
exit(1);
@@ -5602,10 +5602,11 @@ int main(int argc, char **argv)
/* find the command */
for (cmd = img_cmds; cmd->name != NULL; cmd++) {
if (!strcmp(cmdname, cmd->name)) {
+ argv[0] = g_strdup_printf("%s %s", argv[0], cmdname);
return cmd->handler(argc, argv);
}
}
/* not found */
- error_exit("Command not found: %s", cmdname);
+ error_exit(argv[0], "Command not found: %s", cmdname);
}
--
2.39.2On Thu, Feb 22, 2024 at 12:15:45AM +0300, Michael Tokarev wrote:
> 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 whole argv[] array, so argv[0] is
> the executable name, not subcommand name. In order to
> do that, avoid resetting optind but continue with the
> next option. Also don't require at least 3 options on
> the command line: it makes no sense with options before
> subcommand.
>
> Before invoking a subcommand, replace argv[0] to include
> the subcommand name.
>
> 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 <mjt@tls.msk.ru>
> ---
> qemu-img.c | 75 +++++++++++++++++++++++++++---------------------------
> 1 file changed, 38 insertions(+), 37 deletions(-)
I'm not sure how, but this change seems to have broken the iotests.
Just one example:
$ (cd tests/qemu-iotests/ && ./check -qcow2 249)
QEMU -- "/var/home/berrange/src/virt/qemu/build/qemu-system-x86_64" -nodefaults -display none -accel qtest
QEMU_IMG -- "/var/home/berrange/src/virt/qemu/build/qemu-img"
QEMU_IO -- "/var/home/berrange/src/virt/qemu/build/qemu-io" --cache writeback --aio threads -f qcow2
QEMU_NBD -- "/var/home/berrange/src/virt/qemu/build/qemu-nbd"
IMGFMT -- qcow2
IMGPROTO -- file
PLATFORM -- Linux/x86_64 toolbox 6.6.12-200.fc39.x86_64
TEST_DIR -- /var/home/berrange/src/virt/qemu/build/tests/qemu-iotests/scratch
SOCK_DIR -- /tmp/qemu-iotests-0t8h94bu
GDB_OPTIONS --
VALGRIND_QEMU --
PRINT_QEMU_OUTPUT --
249 fail [15:39:25] [15:39:25] 0.2s (last: 0.4s) failed, exit status 1
--- /var/home/berrange/src/virt/qemu/tests/qemu-iotests/249.out
+++ /var/home/berrange/src/virt/qemu/build/tests/qemu-iotests/scratch/qcow2-file-249/249.out.bad
@@ -1,47 +1,7 @@
QA output created by 249
Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=1048576
-Formatting 'TEST_DIR/t.IMGFMT.int', fmt=IMGFMT size=1048576 backing_file=TEST_DIR/t.IMGFMT.base backing_fmt=IMGFMT
-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576 backing_file=TEST_DIR/t.IMGFMT.int backing_fmt=IMGFMT
-{ 'execute': 'qmp_capabilities' }
-{"return": {}}
...snip....
-*** done
+qemu-img: Invalid image size specified. You may use k, M, G, T, P or E suffixes for
+qemu-img: kilobytes, megabytes, gigabytes, terabytes, petabytes and exabytes.
+qemu-img: Invalid image size specified. You may use k, M, G, T, P or E suffixes for
+qemu-img: kilobytes, megabytes, gigabytes, terabytes, petabytes and exabytes.
+Timeout waiting for capabilities on handle 0
Failures: 249
Failed 1 of 1 iotests
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
26.02.2024 18:40, Daniel P. Berrangé : .. > I'm not sure how, but this change seems to have broken the iotests. > Just one example: Heh. Thank you for trying that. I wanted to do that but forgot. The reason is most likely the argv/argc handling (lack of optind reset). In the later change it is fixed but at that stage it's broken. I'll take a look later today. /mjt
26.02.2024 18:43, Michael Tokarev wrote: > The reason is most likely the argv/argc handling (lack of optind reset). > In the later change it is fixed but at that stage it's broken. Nope. GNU getopt_long really needs resetting the state. Or else it keeps return_in_order/permute/etc setting from the previous init call. So this patch needs tweaking, - the reset must be kept, and argv[0] init should be done a bit differently. An easy change. /mjt
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 <mjt@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 df425b2517..d73a5d8fdb 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);
}
static G_NORETURN
void missing_argument(const char *option)
{
- error_exit("missing argument for option '%s'", option);
+ error_exit("qemu-img", "missing argument for option '%s'", option);
}
static G_NORETURN
void unrecognized_option(const char *option)
{
- error_exit("unrecognized option '%s'", option);
+ error_exit("qemu-img", "unrecognized option '%s'", option);
}
/* Please keep in synch with docs/tools/qemu-img.rst */
@@ -576,7 +582,7 @@ static int img_create(int argc, char **argv)
}
if (optind >= argc) {
- error_exit("Expecting image file name");
+ error_exit(argv[0], "Expecting image file name");
}
optind++;
@@ -588,7 +594,7 @@ static int img_create(int argc, char **argv)
}
}
if (optind != argc) {
- error_exit("Unexpected argument: %s", argv[optind]);
+ error_exit(argv[0], "Unexpected argument: %s", argv[optind]);
}
bdrv_img_create(filename, fmt, base_filename, base_fmt,
@@ -770,7 +776,7 @@ static int img_check(int argc, char **argv)
} else if (!strcmp(optarg, "all")) {
fix = BDRV_FIX_LEAKS | BDRV_FIX_ERRORS;
} else {
- error_exit("Unknown option value for -r "
+ error_exit(argv[0], "Unknown option value for -r "
"(expecting 'leaks' or 'all'): %s", optarg);
}
break;
@@ -795,7 +801,7 @@ static int img_check(int argc, char **argv)
}
}
if (optind != argc - 1) {
- error_exit("Expecting one image file name");
+ error_exit(argv[0], "Expecting one image file name");
}
filename = argv[optind++];
@@ -1025,7 +1031,7 @@ static int img_commit(int argc, char **argv)
}
if (optind != argc - 1) {
- error_exit("Expecting one image file name");
+ error_exit(argv[0], "Expecting one image file name");
}
filename = argv[optind++];
@@ -1446,7 +1452,7 @@ static int img_compare(int argc, char **argv)
if (optind != argc - 2) {
- error_exit("Expecting two image file names");
+ error_exit(argv[0], "Expecting two image file names");
}
filename1 = argv[optind++];
filename2 = argv[optind++];
@@ -3056,7 +3062,7 @@ static int img_info(int argc, char **argv)
}
}
if (optind != argc - 1) {
- error_exit("Expecting one image file name");
+ error_exit(argv[0], "Expecting one image file name");
}
filename = argv[optind++];
@@ -3296,7 +3302,7 @@ static int img_map(int argc, char **argv)
}
}
if (optind != argc - 1) {
- error_exit("Expecting one image file name");
+ error_exit(argv[0], "Expecting one image file name");
}
filename = argv[optind];
@@ -3411,7 +3417,7 @@ static int img_snapshot(int argc, char **argv)
return 0;
case 'l':
if (action) {
- error_exit("Cannot mix '-l', '-a', '-c', '-d'");
+ error_exit(argv[0], "Cannot mix '-l', '-a', '-c', '-d'");
return 0;
}
action = SNAPSHOT_LIST;
@@ -3419,7 +3425,7 @@ static int img_snapshot(int argc, char **argv)
break;
case 'a':
if (action) {
- error_exit("Cannot mix '-l', '-a', '-c', '-d'");
+ error_exit(argv[0], "Cannot mix '-l', '-a', '-c', '-d'");
return 0;
}
action = SNAPSHOT_APPLY;
@@ -3427,7 +3433,7 @@ static int img_snapshot(int argc, char **argv)
break;
case 'c':
if (action) {
- error_exit("Cannot mix '-l', '-a', '-c', '-d'");
+ error_exit(argv[0], "Cannot mix '-l', '-a', '-c', '-d'");
return 0;
}
action = SNAPSHOT_CREATE;
@@ -3435,7 +3441,7 @@ static int img_snapshot(int argc, char **argv)
break;
case 'd':
if (action) {
- error_exit("Cannot mix '-l', '-a', '-c', '-d'");
+ error_exit(argv[0], "Cannot mix '-l', '-a', '-c', '-d'");
return 0;
}
action = SNAPSHOT_DELETE;
@@ -3457,7 +3463,7 @@ static int img_snapshot(int argc, char **argv)
}
if (optind != argc - 1) {
- error_exit("Expecting one image file name");
+ error_exit(argv[0], "Expecting one image file name");
}
filename = argv[optind++];
@@ -3624,10 +3630,11 @@ static int img_rebase(int argc, char **argv)
}
if (optind != argc - 1) {
- error_exit("Expecting one image file name");
+ error_exit(argv[0], "Expecting one image file name");
}
if (!unsafe && !out_baseimg) {
- error_exit("Must specify backing file (-b) or use unsafe mode (-u)");
+ error_exit(argv[0],
+ "Must specify backing file (-b) or use unsafe mode (-u)");
}
filename = argv[optind++];
@@ -4051,7 +4058,7 @@ static int img_resize(int argc, char **argv)
/* Remove size from argv manually so that negative numbers are not treated
* as options by getopt. */
if (argc < 3) {
- error_exit("Not enough arguments");
+ error_exit(argv[0], "Not enough arguments");
return 1;
}
@@ -4109,7 +4116,7 @@ static int img_resize(int argc, char **argv)
}
}
if (optind != argc - 1) {
- error_exit("Expecting image file name and size");
+ error_exit(argv[0], "Expecting image file name and size");
}
filename = argv[optind++];
@@ -4306,7 +4313,7 @@ static int img_amend(int argc, char **argv)
}
if (!options) {
- error_exit("Must specify options (-o)");
+ error_exit(argv[0], "Must specify options (-o)");
}
if (quiet) {
@@ -4668,7 +4675,7 @@ static int img_bench(int argc, char **argv)
}
if (optind != argc - 1) {
- error_exit("Expecting one image file name");
+ error_exit(argv[0], "Expecting one image file name");
}
filename = argv[argc - 1];
@@ -5556,9 +5563,6 @@ int main(int argc, char **argv)
module_call_init(MODULE_INIT_QOM);
bdrv_init();
- if (argc < 2) {
- error_exit("Not enough arguments");
- }
qemu_add_opts(&qemu_source_opts);
qemu_add_opts(&qemu_trace_opts);
@@ -5583,15 +5587,11 @@ int main(int argc, char **argv)
}
}
- cmdname = argv[optind];
-
- /* reset getopt_long scanning */
- argc -= optind;
- if (argc < 1) {
- return 0;
+ if (optind >= argc) {
+ error_exit(argv[0], "Not enough arguments");
}
- argv += optind;
- qemu_reset_optind();
+
+ cmdname = argv[optind];
if (!trace_init_backends()) {
exit(1);
@@ -5602,10 +5602,16 @@ int main(int argc, char **argv)
/* find the command */
for (cmd = img_cmds; cmd->name != NULL; cmd++) {
if (!strcmp(cmdname, cmd->name)) {
+ g_autofree char *argv0 = g_strdup_printf("%s %s", argv[0], cmdname);
+ /* reset options and getopt processing (incl return order) */
+ argv += optind;
+ argc -= optind;
+ qemu_reset_optind();
+ argv[0] = argv0;
return cmd->handler(argc, argv);
}
}
/* not found */
- error_exit("Command not found: %s", cmdname);
+ error_exit(argv[0], "Command not found: %s", cmdname);
}
--
2.39.2On Thu, Feb 22, 2024 at 12:15:45AM +0300, Michael Tokarev wrote:
> 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 whole argv[] array, so argv[0] is
> the executable name, not subcommand name. In order to
> do that, avoid resetting optind but continue with the
> next option. Also don't require at least 3 options on
> the command line: it makes no sense with options before
> subcommand.
>
> Before invoking a subcommand, replace argv[0] to include
> the subcommand name.
>
> 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 <mjt@tls.msk.ru>
> ---
> qemu-img.c | 75 +++++++++++++++++++++++++++---------------------------
> 1 file changed, 38 insertions(+), 37 deletions(-)
> @@ -5602,10 +5602,11 @@ int main(int argc, char **argv)
> /* find the command */
> for (cmd = img_cmds; cmd->name != NULL; cmd++) {
> if (!strcmp(cmdname, cmd->name)) {
> + argv[0] = g_strdup_printf("%s %s", argv[0], cmdname);
> return cmd->handler(argc, argv);
This is going to result in valgrind warning that argv[0] is leaked.
How about:
g_autofree char *cmdargv0 = g_strdup_printf("%s %s", argv[0], cmdname);
argv[0] = cmdargv0;
return cmd->handler(argc, argv);
> }
> }
>
> /* not found */
> - error_exit("Command not found: %s", cmdname);
> + error_exit(argv[0], "Command not found: %s", cmdname);
> }
> --
> 2.39.2
>
>
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
© 2016 - 2024 Red Hat, Inc.