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

login
register
mail settings
Submitter Peter Maydell
Date Aug. 1, 2012, 4:13 p.m.
Message ID <1343837599-9597-1-git-send-email-peter.maydell@linaro.org>
Download mbox | patch
Permalink /patch/174513/
State New
Headers show

Comments

Peter Maydell - Aug. 1, 2012, 4:13 p.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.

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.maydell@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.

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(-)
Markus Armbruster - Aug. 2, 2012, 9:03 a.m.
Peter Maydell <peter.maydell@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.maydell@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.
Peter Maydell - Aug. 2, 2012, 9:10 a.m.
On 2 August 2012 10:03, Markus Armbruster <armbru@redhat.com> wrote:
>> -    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.

Yeah, I assumed that treating '?junk' like '?' was basically a bug
due to lazy coding, but I should call it out in the commit message
as a behaviour change I guess.

>> @@ -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.

I have a feeling checkpatch complained or I probably wouldn't have.
I'll put it back though.

>
>        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 "?").

OK.

>> + * 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.

Agreed.

-- PMM

Patch

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:
 
         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));
     }
     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 */
 #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 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 '?'.
+ *
+ * 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);
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));
 }
 
 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);
     }