Patchwork Support 'help' as a synonym for '?' in command line options

login
register
mail settings
Submitter Peter Maydell
Date July 9, 2012, 11:52 a.m.
Message ID <1341834742-28654-1-git-send-email-peter.maydell@linaro.org>
Download mbox | patch
Permalink /patch/169798/
State New
Headers show

Comments

Peter Maydell - July 9, 2012, 11:52 a.m.
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.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
Patch is based on grepping the source for '?' and "?"; I think I've
caught everything that needed changing but it's possible I missed
something.

NB: the bsd-user patch isn't compile tested since I don't have a BSD box.

 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-options.hx   |   32 ++++++++++++++++----------------
 qemu-timer.c      |    2 +-
 vl.c              |    4 ++--
 15 files changed, 56 insertions(+), 39 deletions(-)
Eric Blake - July 9, 2012, 12:07 p.m.
On 07/09/2012 05:52 AM, Peter Maydell wrote:
> 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.

I like the idea, but this may cause grief for libvirt, which basically does:

const char *help; // = output of 'qemu -h'
if (strstr(help, "-device driver,?")) {
    /* Cram together all device-related queries into one invocation;
     * the output format makes it possible to distinguish what we
     * need.  With qemu 0.13.0 and later, unrecognized '-device
     * bogus,?' cause an error in isolation, but are silently ignored
     * in combination with '-device ?'.  Upstream qemu 0.12.x doesn't
     * understand '-device name,?', and always exits with status 1 for
     * the simpler '-device ?', so this function is really only useful
     * if -help includes "device driver,?".  */
    cmd = virCommandNewArgList(qemu,
                               "-device", "?",
                               "-device", "pci-assign,?",
                               "-device", "virtio-blk-pci,?",
                               "-device", "virtio-net-pci,?",
                               "-device", "scsi-disk,?",
                               NULL);
}

That is, we are filtering based on the explicit presence of a literal
'?' in the help output to determine whether we can further filter based
on '-device device,?' queries without confusing qemu or libvirt;
changing the 'help' output means that old libvirt with new qemu won't
run the extra queries (as if it had been targeting qemu 0.12.x), even
though the older spelling of the query would still work.

If that is not a concern (that is, if you are willing to state that use
of newer qemu is intended to be coupled with newer libvirt that has been
taught to use 'help' instead of '?'), then this patch is probably fine.
 The alternative is to update 'qemu -h' output to mention both forms,
rather than completely eradicating the ? form from documentation.
Peter Maydell - July 9, 2012, 12:10 p.m.
On 9 July 2012 13:07, Eric Blake <eblake@redhat.com> wrote:
> That is, we are filtering based on the explicit presence of a literal
> '?' in the help output to determine whether we can further filter based
> on '-device device,?' queries without confusing qemu or libvirt;
> changing the 'help' output means that old libvirt with new qemu won't
> run the extra queries (as if it had been targeting qemu 0.12.x), even
> though the older spelling of the query would still work.
>
> If that is not a concern (that is, if you are willing to state that use
> of newer qemu is intended to be coupled with newer libvirt that has been
> taught to use 'help' instead of '?'), then this patch is probably fine.

My personal position would be that people who parse -help
output deserve to get bitten, but I don't get to make that
call :-)

-- PMM
Eric Blake - July 9, 2012, 12:19 p.m.
On 07/09/2012 06:10 AM, Peter Maydell wrote:
> On 9 July 2012 13:07, Eric Blake <eblake@redhat.com> wrote:
>> That is, we are filtering based on the explicit presence of a literal
>> '?' in the help output to determine whether we can further filter based
>> on '-device device,?' queries without confusing qemu or libvirt;
>> changing the 'help' output means that old libvirt with new qemu won't
>> run the extra queries (as if it had been targeting qemu 0.12.x), even
>> though the older spelling of the query would still work.
>>
>> If that is not a concern (that is, if you are willing to state that use
>> of newer qemu is intended to be coupled with newer libvirt that has been
>> taught to use 'help' instead of '?'), then this patch is probably fine.
> 
> My personal position would be that people who parse -help
> output deserve to get bitten, but I don't get to make that
> call :-)

Obviously, newer libvirt can just proceed to try '-device device,help'
rather than attempting to parse -h output in the first place (that is,
making this change will not cause undue grief to libvirt 0.10.0, because
it's a very short patch to teach libvirt.git to parse this new scheme,
even without relying on -h output).  That is, my concern is not about
what future libvirt will be fixed to do, but about the existing
interaction of old libvirt with qemu (libvirt 0.9.13 + qemu 1.2 would
fail to take full advantage of the qemu 1.2 features because of the
change in the -h output that libvirt 0.9.13 was hard-coded to expect).

And yes, I agree that libvirt has been bitten more than once because it
tries to parse unstable -h output, but that is only because qemu has
never provided anything more stable and more machine-parseable than -h
output, even though the topic has come up several times in the past.
For example, we still don't have a way to run 'query-commands' to see
what the QMP monitor will support without first creating a dummy guest.
Daniel P. Berrange - July 9, 2012, 12:40 p.m.
On Mon, Jul 09, 2012 at 06:07:48AM -0600, Eric Blake wrote:
> On 07/09/2012 05:52 AM, Peter Maydell wrote:
> > 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.
> 
> I like the idea, but this may cause grief for libvirt, which basically does:
> 
> const char *help; // = output of 'qemu -h'
> if (strstr(help, "-device driver,?")) {
>     /* Cram together all device-related queries into one invocation;
>      * the output format makes it possible to distinguish what we
>      * need.  With qemu 0.13.0 and later, unrecognized '-device
>      * bogus,?' cause an error in isolation, but are silently ignored
>      * in combination with '-device ?'.  Upstream qemu 0.12.x doesn't
>      * understand '-device name,?', and always exits with status 1 for
>      * the simpler '-device ?', so this function is really only useful
>      * if -help includes "device driver,?".  */
>     cmd = virCommandNewArgList(qemu,
>                                "-device", "?",
>                                "-device", "pci-assign,?",
>                                "-device", "virtio-blk-pci,?",
>                                "-device", "virtio-net-pci,?",
>                                "-device", "scsi-disk,?",
>                                NULL);
> }
> 
> That is, we are filtering based on the explicit presence of a literal
> '?' in the help output to determine whether we can further filter based
> on '-device device,?' queries without confusing qemu or libvirt;
> changing the 'help' output means that old libvirt with new qemu won't
> run the extra queries (as if it had been targeting qemu 0.12.x), even
> though the older spelling of the query would still work.
> 
> If that is not a concern (that is, if you are willing to state that use
> of newer qemu is intended to be coupled with newer libvirt that has been
> taught to use 'help' instead of '?'), then this patch is probably fine.
>  The alternative is to update 'qemu -h' output to mention both forms,
> rather than completely eradicating the ? form from documentation.


Regardless of what QEMU does here, I think we should change libvirt's
handling of this arg. I think we can probably just skip that check, and
run 'qemu -device XXX,?' unconditionally and then handle any error that
we get back from old QEMU.

Regards,
Daniel
Markus Armbruster - July 9, 2012, 12:59 p.m.
Eric Blake <eblake@redhat.com> writes:

> On 07/09/2012 06:10 AM, Peter Maydell wrote:
>> On 9 July 2012 13:07, Eric Blake <eblake@redhat.com> wrote:
>>> That is, we are filtering based on the explicit presence of a literal
>>> '?' in the help output to determine whether we can further filter based
>>> on '-device device,?' queries without confusing qemu or libvirt;
>>> changing the 'help' output means that old libvirt with new qemu won't
>>> run the extra queries (as if it had been targeting qemu 0.12.x), even
>>> though the older spelling of the query would still work.
>>>
>>> If that is not a concern (that is, if you are willing to state that use
>>> of newer qemu is intended to be coupled with newer libvirt that has been
>>> taught to use 'help' instead of '?'), then this patch is probably fine.
>> 
>> My personal position would be that people who parse -help
>> output deserve to get bitten, but I don't get to make that
>> call :-)
>
> Obviously, newer libvirt can just proceed to try '-device device,help'
> rather than attempting to parse -h output in the first place (that is,
> making this change will not cause undue grief to libvirt 0.10.0, because
> it's a very short patch to teach libvirt.git to parse this new scheme,
> even without relying on -h output).  That is, my concern is not about
> what future libvirt will be fixed to do, but about the existing
> interaction of old libvirt with qemu (libvirt 0.9.13 + qemu 1.2 would
> fail to take full advantage of the qemu 1.2 features because of the
> change in the -h output that libvirt 0.9.13 was hard-coded to expect).
>
> And yes, I agree that libvirt has been bitten more than once because it
> tries to parse unstable -h output, but that is only because qemu has
> never provided anything more stable and more machine-parseable than -h
> output, even though the topic has come up several times in the past.
> For example, we still don't have a way to run 'query-commands' to see
> what the QMP monitor will support without first creating a dummy guest.

Yes, several attempts to provide something real & useful have been shot
down in favor of something hypothetical & perfect.

Thus, libvirt has no choice but working with the (poor) tools we give
it: version number, try and see whether it fails, help output.

Try and see can be slow, and there have been cases where you can't
easily see whether it fails.

Version number breaks down for backports.

Help breaks down when the help text changes.

Perhaps the following could reduce breakage.  Say the feature to be
tested appeared in fully usable form in upstream version X.  If the
version number is at least X, assume we have the feature.  Else, if
detecting a backport of the feature is worth it, check the help output
for a feature signature.  That way, your feature test becomes immune to
future help changes.  Downside, it becomes vulnerable to features
vanishing in the future.  But QEMU shouldn't do that to you without
sufficient prior notice.
Peter Maydell - July 25, 2012, 2:53 p.m.
Ping? There doesn't seem to have been any decision about whether
changing the -help output was OK or not.

Patchwork url: http://patchwork.ozlabs.org/patch/169798/

-- PMM

On 9 July 2012 12:52, Peter Maydell <peter.maydell@linaro.org> wrote:
> 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.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> Patch is based on grepping the source for '?' and "?"; I think I've
> caught everything that needed changing but it's possible I missed
> something.
>
> NB: the bsd-user patch isn't compile tested since I don't have a BSD box.
>
>  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-options.hx   |   32 ++++++++++++++++----------------
>  qemu-timer.c      |    2 +-
>  vl.c              |    4 ++--
>  15 files changed, 56 insertions(+), 39 deletions(-)
>
> diff --git a/arch_init.c b/arch_init.c
> index a9e8b74..e8e6f80 100644
> --- a/arch_init.c
> +++ b/arch_init.c
> @@ -602,7 +602,7 @@ void select_soundhw(const char *optarg)
>  {
>      struct soundhw *c;
>
> -    if (*optarg == '?') {
> +    if (is_help_option(optarg)) {
>      show_valid_cards:
>
>          printf("Valid sound card names (comma separated):\n");
> @@ -610,7 +610,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));
>      }
>      else {
>          size_t l;
> diff --git a/blockdev.c b/blockdev.c
> index 9e0a72a..7810cd7 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 */
>  #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;
> 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 d0e0e4f..3f0ee33 100644
> --- a/linux-user/main.c
> +++ b/linux-user/main.c
> @@ -3053,7 +3053,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, "");
> @@ -3144,7 +3144,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 4aa416c..2e6b405 100644
> --- a/net.c
> +++ b/net.c
> @@ -688,8 +688,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 9d9e603..fa1edb8 100644
> --- a/qemu-common.h
> +++ b/qemu-common.h
> @@ -135,6 +135,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 '?'.
> + *
> + * 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);
>  char *pstrcat(char *buf, int buf_size, const char *s);
> diff --git a/qemu-doc.texi b/qemu-doc.texi
> index 0af0ff4..140b650 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);
> 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-options.hx b/qemu-options.hx
> index 8b66264..eba1ec2 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -29,7 +29,7 @@ ETEXI
>
>  DEF("machine", HAS_ARG, QEMU_OPTION_machine, \
>      "-machine [type=]name[,prop[=value][,...]]\n"
> -    "                selects emulated machine (-machine ? for list)\n"
> +    "                selects emulated machine (-machine help for list)\n"
>      "                property accel=accel1[:accel2[:...]] selects accelerator\n"
>      "                supported accelerators are kvm, xen, tcg (default: tcg)\n"
>      "                kernel_irqchip=on|off controls accelerated irqchip support\n"
> @@ -38,7 +38,7 @@ DEF("machine", HAS_ARG, QEMU_OPTION_machine, \
>  STEXI
>  @item -machine [type=]@var{name}[,prop=@var{value}[,...]]
>  @findex -machine
> -Select the emulated machine by @var{name}. Use @code{-machine ?} to list
> +Select the emulated machine by @var{name}. Use @code{-machine help} to list
>  available machines. Supported machine properties are:
>  @table @option
>  @item accel=@var{accels1}[:@var{accels2}[:...]]
> @@ -57,11 +57,11 @@ HXCOMM Deprecated by -machine
>  DEF("M", HAS_ARG, QEMU_OPTION_M, "", QEMU_ARCH_ALL)
>
>  DEF("cpu", HAS_ARG, QEMU_OPTION_cpu,
> -    "-cpu cpu        select CPU (-cpu ? for list)\n", QEMU_ARCH_ALL)
> +    "-cpu cpu        select CPU (-cpu help for list)\n", QEMU_ARCH_ALL)
>  STEXI
>  @item -cpu @var{model}
>  @findex -cpu
> -Select CPU model (-cpu ? for list and additional feature selection)
> +Select CPU model (-cpu help for list and additional feature selection)
>  ETEXI
>
>  DEF("smp", HAS_ARG, QEMU_OPTION_smp,
> @@ -445,12 +445,12 @@ ETEXI
>  DEF("soundhw", HAS_ARG, QEMU_OPTION_soundhw,
>      "-soundhw c1,... enable audio support\n"
>      "                and only specified sound cards (comma separated list)\n"
> -    "                use -soundhw ? to get the list of supported cards\n"
> +    "                use -soundhw help to get the list of supported cards\n"
>      "                use -soundhw all to enable all of them\n", QEMU_ARCH_ALL)
>  STEXI
>  @item -soundhw @var{card1}[,@var{card2},...] or -soundhw all
>  @findex -soundhw
> -Enable audio and selected sound hardware. Use ? to print all
> +Enable audio and selected sound hardware. Use 'help' to print all
>  available sound hardware.
>
>  @example
> @@ -459,7 +459,7 @@ qemu-system-i386 -soundhw es1370 disk.img
>  qemu-system-i386 -soundhw ac97 disk.img
>  qemu-system-i386 -soundhw hda disk.img
>  qemu-system-i386 -soundhw all disk.img
> -qemu-system-i386 -soundhw ?
> +qemu-system-i386 -soundhw help
>  @end example
>
>  Note that Linux's i810_audio OSS kernel (for AC97) module might
> @@ -548,16 +548,16 @@ DEF("device", HAS_ARG, QEMU_OPTION_device,
>      "-device driver[,prop[=value][,...]]\n"
>      "                add device (based on driver)\n"
>      "                prop=value,... sets driver properties\n"
> -    "                use -device ? to print all possible drivers\n"
> -    "                use -device driver,? to print all possible properties\n",
> +    "                use -device help to print all possible drivers\n"
> +    "                use -device driver,help to print all possible properties\n",
>      QEMU_ARCH_ALL)
>  STEXI
>  @item -device @var{driver}[,@var{prop}[=@var{value}][,...]]
>  @findex -device
>  Add device @var{driver}.  @var{prop}=@var{value} sets driver
>  properties.  Valid properties depend on the driver.  To get help on
> -possible drivers and properties, use @code{-device ?} and
> -@code{-device @var{driver},?}.
> +possible drivers and properties, use @code{-device help} and
> +@code{-device @var{driver},help}.
>  ETEXI
>
>  DEFHEADING()
> @@ -1315,7 +1315,7 @@ Valid values for @var{type} are
>  @code{virtio}, @code{i82551}, @code{i82557b}, @code{i82559er},
>  @code{ne2k_pci}, @code{ne2k_isa}, @code{pcnet}, @code{rtl8139},
>  @code{e1000}, @code{smc91c111}, @code{lance} and @code{mcf_fec}.
> -Not all devices are supported on all targets.  Use -net nic,model=?
> +Not all devices are supported on all targets.  Use -net nic,model=help
>  for a list of available devices for your target.
>
>  @item -net user[,@var{option}][,@var{option}][,...]
> @@ -2313,7 +2313,7 @@ Shorthand for -gdb tcp::1234, i.e. open a gdbserver on TCP port 1234
>  ETEXI
>
>  DEF("d", HAS_ARG, QEMU_OPTION_d, \
> -    "-d item1,...    output log to /tmp/qemu.log (use -d ? for a list of log items)\n",
> +    "-d item1,...    output log to /tmp/qemu.log (use -d help for a list of log items)\n",
>      QEMU_ARCH_ALL)
>  STEXI
>  @item -d
> @@ -2448,13 +2448,13 @@ ETEXI
>
>  DEF("clock", HAS_ARG, QEMU_OPTION_clock, \
>      "-clock          force the use of the given methods for timer alarm.\n" \
> -    "                To see what timers are available use -clock ?\n",
> +    "                To see what timers are available use -clock help\n",
>      QEMU_ARCH_ALL)
>  STEXI
>  @item -clock @var{method}
>  @findex -clock
>  Force the use of the given methods for timer alarm. To see what timers
> -are available use -clock ?.
> +are available use -clock help.
>  ETEXI
>
>  HXCOMM Options deprecated by -rtc
> @@ -2523,7 +2523,7 @@ watchdog with a single timer, or @code{i6300esb} (Intel 6300ESB I/O
>  controller hub) which is a much more featureful PCI-based dual-timer
>  watchdog.  Choose a model for which your guest has drivers.
>
> -Use @code{-watchdog ?} to list available hardware models.  Only one
> +Use @code{-watchdog help} to list available hardware models.  Only one
>  watchdog can be enabled for a guest.
>  ETEXI
>
> 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 1329c30..716fafc 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2087,7 +2087,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));
>  }
>
>  static int tcg_init(void)
> @@ -3217,7 +3217,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);
>      }
> --
> 1.7.1
>

Patch

diff --git a/arch_init.c b/arch_init.c
index a9e8b74..e8e6f80 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -602,7 +602,7 @@  void select_soundhw(const char *optarg)
 {
     struct soundhw *c;
 
-    if (*optarg == '?') {
+    if (is_help_option(optarg)) {
     show_valid_cards:
 
         printf("Valid sound card names (comma separated):\n");
@@ -610,7 +610,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));
     }
     else {
         size_t l;
diff --git a/blockdev.c b/blockdev.c
index 9e0a72a..7810cd7 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 */
 #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;
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 d0e0e4f..3f0ee33 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -3053,7 +3053,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, "");
@@ -3144,7 +3144,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 4aa416c..2e6b405 100644
--- a/net.c
+++ b/net.c
@@ -688,8 +688,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 9d9e603..fa1edb8 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -135,6 +135,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 '?'.
+ *
+ * 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);
 char *pstrcat(char *buf, int buf_size, const char *s);
diff --git a/qemu-doc.texi b/qemu-doc.texi
index 0af0ff4..140b650 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);
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-options.hx b/qemu-options.hx
index 8b66264..eba1ec2 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -29,7 +29,7 @@  ETEXI
 
 DEF("machine", HAS_ARG, QEMU_OPTION_machine, \
     "-machine [type=]name[,prop[=value][,...]]\n"
-    "                selects emulated machine (-machine ? for list)\n"
+    "                selects emulated machine (-machine help for list)\n"
     "                property accel=accel1[:accel2[:...]] selects accelerator\n"
     "                supported accelerators are kvm, xen, tcg (default: tcg)\n"
     "                kernel_irqchip=on|off controls accelerated irqchip support\n"
@@ -38,7 +38,7 @@  DEF("machine", HAS_ARG, QEMU_OPTION_machine, \
 STEXI
 @item -machine [type=]@var{name}[,prop=@var{value}[,...]]
 @findex -machine
-Select the emulated machine by @var{name}. Use @code{-machine ?} to list
+Select the emulated machine by @var{name}. Use @code{-machine help} to list
 available machines. Supported machine properties are:
 @table @option
 @item accel=@var{accels1}[:@var{accels2}[:...]]
@@ -57,11 +57,11 @@  HXCOMM Deprecated by -machine
 DEF("M", HAS_ARG, QEMU_OPTION_M, "", QEMU_ARCH_ALL)
 
 DEF("cpu", HAS_ARG, QEMU_OPTION_cpu,
-    "-cpu cpu        select CPU (-cpu ? for list)\n", QEMU_ARCH_ALL)
+    "-cpu cpu        select CPU (-cpu help for list)\n", QEMU_ARCH_ALL)
 STEXI
 @item -cpu @var{model}
 @findex -cpu
-Select CPU model (-cpu ? for list and additional feature selection)
+Select CPU model (-cpu help for list and additional feature selection)
 ETEXI
 
 DEF("smp", HAS_ARG, QEMU_OPTION_smp,
@@ -445,12 +445,12 @@  ETEXI
 DEF("soundhw", HAS_ARG, QEMU_OPTION_soundhw,
     "-soundhw c1,... enable audio support\n"
     "                and only specified sound cards (comma separated list)\n"
-    "                use -soundhw ? to get the list of supported cards\n"
+    "                use -soundhw help to get the list of supported cards\n"
     "                use -soundhw all to enable all of them\n", QEMU_ARCH_ALL)
 STEXI
 @item -soundhw @var{card1}[,@var{card2},...] or -soundhw all
 @findex -soundhw
-Enable audio and selected sound hardware. Use ? to print all
+Enable audio and selected sound hardware. Use 'help' to print all
 available sound hardware.
 
 @example
@@ -459,7 +459,7 @@  qemu-system-i386 -soundhw es1370 disk.img
 qemu-system-i386 -soundhw ac97 disk.img
 qemu-system-i386 -soundhw hda disk.img
 qemu-system-i386 -soundhw all disk.img
-qemu-system-i386 -soundhw ?
+qemu-system-i386 -soundhw help
 @end example
 
 Note that Linux's i810_audio OSS kernel (for AC97) module might
@@ -548,16 +548,16 @@  DEF("device", HAS_ARG, QEMU_OPTION_device,
     "-device driver[,prop[=value][,...]]\n"
     "                add device (based on driver)\n"
     "                prop=value,... sets driver properties\n"
-    "                use -device ? to print all possible drivers\n"
-    "                use -device driver,? to print all possible properties\n",
+    "                use -device help to print all possible drivers\n"
+    "                use -device driver,help to print all possible properties\n",
     QEMU_ARCH_ALL)
 STEXI
 @item -device @var{driver}[,@var{prop}[=@var{value}][,...]]
 @findex -device
 Add device @var{driver}.  @var{prop}=@var{value} sets driver
 properties.  Valid properties depend on the driver.  To get help on
-possible drivers and properties, use @code{-device ?} and
-@code{-device @var{driver},?}.
+possible drivers and properties, use @code{-device help} and
+@code{-device @var{driver},help}.
 ETEXI
 
 DEFHEADING()
@@ -1315,7 +1315,7 @@  Valid values for @var{type} are
 @code{virtio}, @code{i82551}, @code{i82557b}, @code{i82559er},
 @code{ne2k_pci}, @code{ne2k_isa}, @code{pcnet}, @code{rtl8139},
 @code{e1000}, @code{smc91c111}, @code{lance} and @code{mcf_fec}.
-Not all devices are supported on all targets.  Use -net nic,model=?
+Not all devices are supported on all targets.  Use -net nic,model=help
 for a list of available devices for your target.
 
 @item -net user[,@var{option}][,@var{option}][,...]
@@ -2313,7 +2313,7 @@  Shorthand for -gdb tcp::1234, i.e. open a gdbserver on TCP port 1234
 ETEXI
 
 DEF("d", HAS_ARG, QEMU_OPTION_d, \
-    "-d item1,...    output log to /tmp/qemu.log (use -d ? for a list of log items)\n",
+    "-d item1,...    output log to /tmp/qemu.log (use -d help for a list of log items)\n",
     QEMU_ARCH_ALL)
 STEXI
 @item -d
@@ -2448,13 +2448,13 @@  ETEXI
 
 DEF("clock", HAS_ARG, QEMU_OPTION_clock, \
     "-clock          force the use of the given methods for timer alarm.\n" \
-    "                To see what timers are available use -clock ?\n",
+    "                To see what timers are available use -clock help\n",
     QEMU_ARCH_ALL)
 STEXI
 @item -clock @var{method}
 @findex -clock
 Force the use of the given methods for timer alarm. To see what timers
-are available use -clock ?.
+are available use -clock help.
 ETEXI
 
 HXCOMM Options deprecated by -rtc
@@ -2523,7 +2523,7 @@  watchdog with a single timer, or @code{i6300esb} (Intel 6300ESB I/O
 controller hub) which is a much more featureful PCI-based dual-timer
 watchdog.  Choose a model for which your guest has drivers.
 
-Use @code{-watchdog ?} to list available hardware models.  Only one
+Use @code{-watchdog help} to list available hardware models.  Only one
 watchdog can be enabled for a guest.
 ETEXI
 
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 1329c30..716fafc 100644
--- a/vl.c
+++ b/vl.c
@@ -2087,7 +2087,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));
 }
 
 static int tcg_init(void)
@@ -3217,7 +3217,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);
     }