| Message ID | 1269340078-16446-3-git-send-email-armbru@redhat.com |
|---|---|
| State | New |
| Headers | show |
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. > + qdict_put(qdict, key, qbool_from_int(val)); > + } > + break; > case '-': > { > const char *tmp = p; > @@ -4322,6 +4347,12 @@ static int check_arg(const CmdArgs *cmd_args, QDict *args) > return -1; > } > break; > + case 'b': > + if (qobject_type(value) != QTYPE_QBOOL) { > + qerror_report(QERR_INVALID_PARAMETER_TYPE, name, "bool"); > + return -1; > + } > + break; > case '-': > if (qobject_type(value) != QTYPE_QINT && > qobject_type(value) != QTYPE_QBOOL) {
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. [...]
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.
Luiz Capitulino <lcapitulino@redhat.com> writes: > 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. Brain fart. I'll respin. Thanks for catching this!
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; + } + qdict_put(qdict, key, qbool_from_int(val)); + } + break; case '-': { const char *tmp = p; @@ -4322,6 +4347,12 @@ static int check_arg(const CmdArgs *cmd_args, QDict *args) return -1; } break; + case 'b': + if (qobject_type(value) != QTYPE_QBOOL) { + qerror_report(QERR_INVALID_PARAMETER_TYPE, name, "bool"); + return -1; + } + break; case '-': if (qobject_type(value) != QTYPE_QINT && qobject_type(value) != QTYPE_QBOOL) {
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(-)