[Qemu-devel] Re: [PATCH 2/4] monitor: New argument type 'b'

[Luiz Capitulino <lcapitulino@redhat.com>]

On Thu, 25 Mar 2010 18:28:43 +0100
Markus Armbruster <armbru@redhat.com> wrote:

> Luiz Capitulino <lcapitulino@redhat.com> writes:
> 
> > On Tue, 23 Mar 2010 11:27:56 +0100
> > Markus Armbruster <armbru@redhat.com> wrote:
> >
> >> This is a boolean value.  Human monitor accepts "on" or "off".
> >> Consistent with option parsing (see parse_option_bool()).
> >> 
> >> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> >> ---
> >>  monitor.c |   31 +++++++++++++++++++++++++++++++
> >>  1 files changed, 31 insertions(+), 0 deletions(-)
> >> 
> >> diff --git a/monitor.c b/monitor.c
> >> index 3ce9a4e..47b68a2 100644
> >> --- a/monitor.c
> >> +++ b/monitor.c
> >> @@ -85,6 +85,8 @@
> >>   *
> >>   * '?'          optional type (for all types, except '/')
> >>   * '.'          other form of optional type (for 'i' and 'l')
> >> + * 'b'          boolean
> >> + *              user mode accepts "on" or "off"
> >>   * '-'          optional parameter (eg. '-f')
> >>   *
> >>   */
> >> @@ -3841,6 +3843,29 @@ static const mon_cmd_t *monitor_parse_command(Monitor *mon,
> >>                  qdict_put(qdict, key, qfloat_from_double(val));
> >>              }
> >>              break;
> >> +        case 'b':
> >> +            {
> >> +                const char *beg;
> >> +                int val;
> >> +
> >> +                while (qemu_isspace(*p)) {
> >> +                    p++;
> >> +                }
> >> +                beg = p;
> >> +                while (qemu_isgraph(*p)) {
> >> +                    p++;
> >> +                }
> >> +                if (!strncmp(beg, "on", p - beg)) {
> >> +                    val = 1;
> >> +                } else if (!strncmp(beg, "off", p - beg)) {
> >> +                    val = 0;
> >> +                } else {
> >> +                    monitor_printf(mon, "Expected 'on' or 'off'\n");
> >> +                    goto fail;
> >> +                }
> >
> >  This will make 'on' be the default when no on/off is specified, is that
> > your intention? I'm wondering if this can cause problems when you add
> > optional support for it and mixes it with other arguments.
> 
> No.  Intended behavior: the argument must be either "on" or "off".  With
> "on", (KEY, true) is put into the dictionary, for "off" it's (KEY,
> false).
> 
> We get a third case for optional argument if we support that: KEY not in
> dictionary.  The handler decides how to interpret that.

 Ok, but strncmp() will return 0 if p - beg = 0, right? In this
case the current implementation will put true on the dict for a line like:

(qemu) set_link foo

 Which should return an error to the user then.