Peter Maydell <peter.mayd...@linaro.org> writes:
> For command line options which permit '?' meaning 'please list the
> permitted values', add support for 'help' as a synonym, by abstracting
> the check out into a helper function.
>
> Update the documentation to use 'help' rather than '?', since '?'
> is a shell metacharacter and thus prone to fail confusingly if there
> is a single character filename in the current working directory and
> the '?' has not been escaped. It's therefore better to steer users
> towards 'help', though '?' is retained for backwards compatibility.
>
> We do not, however, update the output of the system emulator's -help
> (or any documentation autogenerated from the qemu-options.hx which
> is the source of the -help text) because libvirt parses our -help
> output and will break. At a later date when QEMU provides a better
> interface so libvirt can avoid having to do this, we can update the
> -help text too.
>
> Signed-off-by: Peter Maydell <peter.mayd...@linaro.org>
> ---
> Since we've agreed not to change the -help output for QEMU 1.2
> this is basically just the v1 patch with the qemu-options.hx
> changes dropped.
Put a big fat TODO comment there?
> bsd-user change still only eyeballed, not compiled.
>
> arch_init.c | 4 ++--
> blockdev.c | 10 +++++-----
> bsd-user/main.c | 6 +++---
> hw/mips_jazz.c | 2 +-
> hw/qdev-monitor.c | 2 +-
> hw/watchdog.c | 2 +-
> linux-user/main.c | 4 ++--
> net.c | 3 ++-
> qemu-common.h | 16 ++++++++++++++++
> qemu-doc.texi | 2 +-
> qemu-ga.c | 2 +-
> qemu-img.c | 4 ++--
> qemu-timer.c | 2 +-
> vl.c | 4 ++--
> 14 files changed, 40 insertions(+), 23 deletions(-)
>
> diff --git a/arch_init.c b/arch_init.c
> index 26f30ef..60823ba 100644
> --- a/arch_init.c
> +++ b/arch_init.c
> @@ -680,7 +680,7 @@ void select_soundhw(const char *optarg)
> {
> struct soundhw *c;
>
> - if (*optarg == '?') {
> + if (is_help_option(optarg)) {
> show_valid_cards:
"-soundhw ?junk" now goes through the "bad card name" path instead of
the "help" path. Fine with me.
>
> printf("Valid sound card names (comma separated):\n");
> @@ -688,7 +688,7 @@ void select_soundhw(const char *optarg)
> printf ("%-11s %s\n", c->name, c->descr);
> }
> printf("\n-soundhw all will enable all of the above\n");
> - exit(*optarg != '?');
> + exit(!is_help_option(optarg));
Including the exit code. Still fine with me.
> }
> else {
> size_t l;
> diff --git a/blockdev.c b/blockdev.c
> index 3d75015..8669142 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -398,11 +398,11 @@ DriveInfo *drive_init(QemuOpts *opts, int
> default_to_scsi)
> #endif
>
> if ((buf = qemu_opt_get(opts, "format")) != NULL) {
> - if (strcmp(buf, "?") == 0) {
> - error_printf("Supported formats:");
> - bdrv_iterate_format(bdrv_format_print, NULL);
> - error_printf("\n");
> - return NULL;
> + if (is_help_option(buf)) {
> + error_printf("Supported formats:");
> + bdrv_iterate_format(bdrv_format_print, NULL);
> + error_printf("\n");
> + return NULL;
> }
> drv = bdrv_find_whitelisted_format(buf);
> if (!drv) {
> diff --git a/bsd-user/main.c b/bsd-user/main.c
> index cd33d65..0aad752 100644
> --- a/bsd-user/main.c
> +++ b/bsd-user/main.c
> @@ -681,7 +681,7 @@ static void usage(void)
> "-g port wait gdb connection to port\n"
> "-L path set the elf interpreter prefix (default=%s)\n"
> "-s size set the stack size in bytes (default=%ld)\n"
> - "-cpu model select CPU (-cpu ? for list)\n"
> + "-cpu model select CPU (-cpu help for list)\n"
> "-drop-ld-preload drop LD_PRELOAD for target process\n"
> "-E var=value sets/modifies targets environment
> variable(s)\n"
> "-U var unsets targets environment variable(s)\n"
> @@ -825,8 +825,8 @@ int main(int argc, char **argv)
> qemu_uname_release = argv[optind++];
> } else if (!strcmp(r, "cpu")) {
> cpu_model = argv[optind++];
> - if (strcmp(cpu_model, "?") == 0) {
> -/* XXX: implement xxx_cpu_list for targets that still miss it */
> + if (is_help_option(cpu_model)) {
> + /* XXX: implement xxx_cpu_list for targets that still miss
> it */
I wouldn't reindent this line.
> #if defined(cpu_list)
> cpu_list(stdout, &fprintf);
> #endif
> diff --git a/hw/mips_jazz.c b/hw/mips_jazz.c
> index bf1b799..db927f1 100644
> --- a/hw/mips_jazz.c
> +++ b/hw/mips_jazz.c
> @@ -239,7 +239,7 @@ static void mips_jazz_init(MemoryRegion *address_space,
> dp83932_init(nd, 0x80001000, 2, get_system_memory(), rc4030[4],
> rc4030_opaque, rc4030_dma_memory_rw);
> break;
> - } else if (strcmp(nd->model, "?") == 0) {
> + } else if (is_help_option(nd->model)) {
> fprintf(stderr, "qemu: Supported NICs: dp83932\n");
> exit(1);
> } else {
> diff --git a/hw/qdev-monitor.c b/hw/qdev-monitor.c
> index 7915b45..d777a20 100644
> --- a/hw/qdev-monitor.c
> +++ b/hw/qdev-monitor.c
> @@ -138,7 +138,7 @@ int qdev_device_help(QemuOpts *opts)
> ObjectClass *klass;
>
> driver = qemu_opt_get(opts, "driver");
> - if (driver && !strcmp(driver, "?")) {
> + if (driver && is_help_option(driver)) {
> bool show_no_user = false;
> object_class_foreach(qdev_print_devinfo, TYPE_DEVICE, false,
> &show_no_user);
> return 1;
}
if (!driver || !qemu_opt_get(opts, "?")) {
return 0;
}
I'm afraid you missed this one. Please test both
-device \?
-device e1000,\?
as well as
-device i6300esb
-device i6300esb,addr=9
-device i6300esb,romfile=\?
The last one is expected to fail (failed to find romfile "?").
You can use another device model, of course, i6300esb is just what I
used.
> diff --git a/hw/watchdog.c b/hw/watchdog.c
> index a42124d..b52aced 100644
> --- a/hw/watchdog.c
> +++ b/hw/watchdog.c
> @@ -55,7 +55,7 @@ int select_watchdog(const char *p)
> QemuOpts *opts;
>
> /* -watchdog ? lists available devices and exits cleanly. */
> - if (strcmp(p, "?") == 0) {
> + if (is_help_option(p)) {
> QLIST_FOREACH(model, &watchdog_list, entry) {
> fprintf(stderr, "\t%s\t%s\n",
> model->wdt_name, model->wdt_description);
> diff --git a/linux-user/main.c b/linux-user/main.c
> index a0ab8e8..25eaa11 100644
> --- a/linux-user/main.c
> +++ b/linux-user/main.c
> @@ -3140,7 +3140,7 @@ static void handle_arg_uname(const char *arg)
> static void handle_arg_cpu(const char *arg)
> {
> cpu_model = strdup(arg);
> - if (cpu_model == NULL || strcmp(cpu_model, "?") == 0) {
> + if (cpu_model == NULL || is_help_option(cpu_model)) {
> /* XXX: implement xxx_cpu_list for targets that still miss it */
> #if defined(cpu_list_id)
> cpu_list_id(stdout, &fprintf, "");
> @@ -3231,7 +3231,7 @@ struct qemu_argument arg_table[] = {
> {"s", "QEMU_STACK_SIZE", true, handle_arg_stack_size,
> "size", "set the stack size to 'size' bytes"},
> {"cpu", "QEMU_CPU", true, handle_arg_cpu,
> - "model", "select CPU (-cpu ? for list)"},
> + "model", "select CPU (-cpu help for list)"},
> {"E", "QEMU_SET_ENV", true, handle_arg_set_env,
> "var=value", "sets targets environment variable (see below)"},
> {"U", "QEMU_UNSET_ENV", true, handle_arg_unset_env,
> diff --git a/net.c b/net.c
> index dbca77b..32ca50e 100644
> --- a/net.c
> +++ b/net.c
> @@ -691,8 +691,9 @@ int qemu_show_nic_models(const char *arg, const char
> *const *models)
> {
> int i;
>
> - if (!arg || strcmp(arg, "?"))
> + if (!arg || !is_help_option(arg)) {
> return 0;
> + }
>
> fprintf(stderr, "qemu: Supported NIC models: ");
> for (i = 0 ; models[i]; i++)
> diff --git a/qemu-common.h b/qemu-common.h
> index d26ff39..ee75e9f 100644
> --- a/qemu-common.h
> +++ b/qemu-common.h
> @@ -136,6 +136,22 @@ int qemu_main(int argc, char **argv, char **envp);
> void qemu_get_timedate(struct tm *tm, int offset);
> int qemu_timedate_diff(struct tm *tm);
>
> +/**
> + * is_help_option:
> + * @s: string to test
> + *
> + * Check whether @s is one of the standard strings which indicate
> + * that the user is asking for a list of the valid values for a
> + * command option like -cpu or -M. The current accepted strings
> + * are 'help' and '?'.
Good opportunity to document that '?' is deprecated.
> + *
> + * Returns: true if @s is a request for a list.
> + */
> +static inline bool is_help_option(const char *s)
> +{
> + return !strcmp(s, "?") || !strcmp(s, "help");
> +}
> +
> /* cutils.c */
> void pstrcpy(char *buf, int buf_size, const char *str);
> void strpadcpy(char *buf, int buf_size, const char *str, char pad);
> diff --git a/qemu-doc.texi b/qemu-doc.texi
> index 84dad19..a41448a 100644
> --- a/qemu-doc.texi
> +++ b/qemu-doc.texi
> @@ -2390,7 +2390,7 @@ Set the x86 elf interpreter prefix
> (default=/usr/local/qemu-i386)
> @item -s size
> Set the x86 stack size in bytes (default=524288)
> @item -cpu model
> -Select CPU model (-cpu ? for list and additional feature selection)
> +Select CPU model (-cpu help for list and additional feature selection)
> @item -ignore-environment
> Start with an empty environment. Without this option,
> the initial environment is a copy of the caller's environment.
> diff --git a/qemu-ga.c b/qemu-ga.c
> index 8199da7..f1a39ec 100644
> --- a/qemu-ga.c
> +++ b/qemu-ga.c
> @@ -736,7 +736,7 @@ int main(int argc, char **argv)
> break;
> case 'b': {
> char **list_head, **list;
> - if (*optarg == '?') {
> + if (is_help_option(optarg)) {
> list_head = list = qmp_get_command_list();
> while (*list != NULL) {
> printf("%s\n", *list);
Another case of '?junk', and I still don't care :)
> diff --git a/qemu-img.c b/qemu-img.c
> index 80cfb9b..b866f80 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -350,7 +350,7 @@ static int img_create(int argc, char **argv)
> img_size = (uint64_t)sval;
> }
>
> - if (options && !strcmp(options, "?")) {
> + if (options && is_help_option(options)) {
> ret = print_block_option_help(filename, fmt);
> goto out;
> }
> @@ -744,7 +744,7 @@ static int img_convert(int argc, char **argv)
> /* Initialize before goto out */
> qemu_progress_init(progress, 2.0);
>
> - if (options && !strcmp(options, "?")) {
> + if (options && is_help_option(options)) {
> ret = print_block_option_help(out_filename, out_fmt);
> goto out;
> }
> diff --git a/qemu-timer.c b/qemu-timer.c
> index de98977..062fdf2 100644
> --- a/qemu-timer.c
> +++ b/qemu-timer.c
> @@ -183,7 +183,7 @@ void configure_alarms(char const *opt)
> char *name;
> struct qemu_alarm_timer tmp;
>
> - if (!strcmp(opt, "?")) {
> + if (is_help_option(opt)) {
> show_available_alarms();
> exit(0);
> }
> diff --git a/vl.c b/vl.c
> index 9fea320..1fd1114 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2086,7 +2086,7 @@ static QEMUMachine *machine_parse(const char *name)
> printf("%-20s %s%s\n", m->name, m->desc,
> m->is_default ? " (default)" : "");
> }
> - exit(!name || *name != '?');
> + exit(!name || !is_help_option(name));
> }
>
Yet another ?junk change.
> static int tcg_init(void)
> @@ -3216,7 +3216,7 @@ int main(int argc, char **argv, char **envp)
> */
> cpudef_init();
>
> - if (cpu_model && *cpu_model == '?') {
> + if (cpu_model && is_help_option(cpu_model)) {
> list_cpus(stdout, &fprintf, cpu_model);
> exit(0);
> }
Yet another ?junk change.
