[libvirt] [PATCH 3/8] virsh: better handling the boolean option

in old code the following commands are equivalent: virsh # dumpxml --update-cpu=vm1 virsh # dumpxml --update-cpu vm1 because the old code split the option argument into 2 parts: --update-cpu=vm1 is split into update-cpu and vm1, and update-cpu is a boolean option, so the parser takes vm1 as another argument, very strange. after this patch applied, the first one will become illegal. To achieve this, we don't parse/check options when parsing command sting, but check options when parsing a command argument. And the argument is not split when parsing command sting. Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com> --- diff --git a/tools/virsh.c b/tools/virsh.c index 1e95023..f97ee42 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -10164,14 +10164,12 @@ vshCommandRun(vshControl *ctl, const vshCmd *cmd) */ #define VSH_TK_ERROR -1 #define VSH_TK_NONE 0 -#define VSH_TK_OPTION 1 -#define VSH_TK_DATA 2 -#define VSH_TK_END 3 +#define VSH_TK_DATA 1 +#define VSH_TK_END 2 static int vshCommandGetToken(vshControl *ctl, char *str, char **end, char **res) { - int tk = VSH_TK_NONE; bool double_quote = false; int sz = 0; char *p = str, *q; @@ -10196,22 +10194,6 @@ copy: if (!double_quote && (*p == ' ' || *p == '\t' || *p == ';')) break; - /* end of option name could be '=' */ - if (tk == VSH_TK_OPTION && *p == '=') { - p++; /* skip '=' */ - break; - } - - if (tk == VSH_TK_NONE) { - if (*p == '-' && *(p + 1) == '-' && *(p + 2) - && c_isalnum(*(p + 2))) { - tk = VSH_TK_OPTION; - p += 2; - } else { - tk = VSH_TK_DATA; - } - } - if (*p == '"') { double_quote = !double_quote; p++; @@ -10237,7 +10219,7 @@ copy: *(*res + sz) = '\0'; *end = p; - return tk; + return VSH_TK_DATA; } static int @@ -10296,18 +10278,28 @@ vshCommandParse(vshControl *ctl, char *cmdstr) goto syntaxError; /* ... or ignore this command only? */ } VIR_FREE(tkdata); - } else if (tk == VSH_TK_OPTION) { - if (!(opt = vshCmddefGetOption(cmd, tkdata))) { + } else if (*tkdata == '-' && *(tkdata + 1) == '-' && *(tkdata + 2) + && c_isalnum(*(tkdata + 2))) { + char *optstr = strchr(tkdata + 2, '='); + if (optstr) { + *optstr = '\0'; /* conver the '=' to '\0' */ + optstr = vshStrdup(ctl, optstr + 1); + } + if (!(opt = vshCmddefGetOption(cmd, tkdata + 2))) { vshError(ctl, _("command '%s' doesn't support option --%s"), - cmd->name, tkdata); + cmd->name, tkdata + 2); + VIR_FREE(optstr); goto syntaxError; } - VIR_FREE(tkdata); /* option name */ + VIR_FREE(tkdata); if (opt->type != VSH_OT_BOOL) { /* option data */ - tk = vshCommandGetToken(ctl, str, &end, &tkdata); + if (optstr) + tkdata = optstr; + else + tk = vshCommandGetToken(ctl, str, &end, &tkdata); str = end; if (tk == VSH_TK_ERROR) goto syntaxError; @@ -10319,6 +10311,14 @@ vshCommandParse(vshControl *ctl, char *cmdstr) VSH_OT_INT ? _("number") : _("string")); goto syntaxError; } + } else { + tkdata = NULL; + if (optstr) { + vshError(ctl, _("invalid '=' after option --%s"), + opt->name); + VIR_FREE(optstr); + goto syntaxError; + } } } else if (tk == VSH_TK_DATA) { if (!(opt = vshCmddefGetData(cmd, data_ct++))) { @@ -10344,8 +10344,8 @@ vshCommandParse(vshControl *ctl, char *cmdstr) vshDebug(ctl, 4, "%s: %s(%s): %s\n", cmd->name, opt->name, - tk == VSH_TK_OPTION ? _("OPTION") : _("DATA"), - arg->data); + opt->type != VSH_OT_BOOL ? _("optdata") : _("bool"), + opt->type != VSH_OT_BOOL ? arg->data : _("(none)")); } if (!str) break;

On 10/12/2010 01:13 AM, Lai Jiangshan wrote:
in old code the following commands are equivalent: virsh # dumpxml --update-cpu=vm1 virsh # dumpxml --update-cpu vm1 because the old code split the option argument into 2 parts: --update-cpu=vm1 is split into update-cpu and vm1, and update-cpu is a boolean option, so the parser takes vm1 as another argument, very strange.
after this patch applied, the first one will become illegal.
To achieve this, we don't parse/check options when parsing command sting, but check options when parsing a command argument. And the argument is not split when parsing command sting.
Signed-off-by: Lai Jiangshan<laijs@cn.fujitsu.com> +++ b/tools/virsh.c @@ -10164,14 +10164,12 @@ vshCommandRun(vshControl *ctl, const vshCmd *cmd) */ #define VSH_TK_ERROR -1 #define VSH_TK_NONE 0 -#define VSH_TK_OPTION 1 -#define VSH_TK_DATA 2 -#define VSH_TK_END 3 +#define VSH_TK_DATA 1 +#define VSH_TK_END 2
Maybe it would be nice to make these an enum rather than #defines, but that would be a separate cleanup patch, no impact to today's review.
@@ -10296,18 +10278,28 @@ vshCommandParse(vshControl *ctl, char *cmdstr) goto syntaxError; /* ... or ignore this command only? */ } VIR_FREE(tkdata); - } else if (tk == VSH_TK_OPTION) { - if (!(opt = vshCmddefGetOption(cmd, tkdata))) { + } else if (*tkdata == '-'&& *(tkdata + 1) == '-'&& *(tkdata + 2) +&& c_isalnum(*(tkdata + 2))) { + char *optstr = strchr(tkdata + 2, '='); + if (optstr) { + *optstr = '\0'; /* conver the '=' to '\0' */
s/conver/convert/ ACK with that nit fixed. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
participants (2)
-
Eric Blake
-
Lai Jiangshan