qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH] Support 'help' as a synonym for '?' in command


From: Peter Maydell
Subject: Re: [Qemu-devel] [PATCH] Support 'help' as a synonym for '?' in command line options
Date: Wed, 25 Jul 2012 15:53:02 +0100

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 <address@hidden> 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 <address@hidden>
> ---
> 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 address@hidden,address@hidden,...]]
>  @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 address@hidden:@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}[,@address@hidden,...]]
>  @findex -device
>  Add device @var{driver}.  @address@hidden sets driver
>  properties.  Valid properties depend on the driver.  To get help on
> -possible drivers and properties, use @code{-device ?} and
> address@hidden @var{driver},?}.
> +possible drivers and properties, use @code{-device help} and
> address@hidden @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
>



reply via email to

[Prev in Thread] Current Thread [Next in Thread]