On 10/12/2010 01:13 AM, Lai Jiangshan wrote:
In origin code, double quote is only allowed at the begin or end "complicated argument" --some_opt="complicated string" (we split this argument into 2 parts, option and data, the data is "complicated string").
This patch makes it allow double quote at any position of an argument: complicated" argument" complicated" "argument --"some opt=complicated string"
This patch is also needed for the following patches, the following patches will not split option argument into 2 parts, so we have to allow double quote at any position of an argument.
Signed-off-by: Lai Jiangshan<laijs@cn.fujitsu.com>
I had to add your name to AUTHORS to make syntax-check happy.
--- diff --git a/tools/virsh.c b/tools/virsh.c index 57ea618..7b6f2b6 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -10172,9 +10172,9 @@ static int vshCommandGetToken(vshControl *ctl, char *str, char **end, char **res) { int tk = VSH_TK_NONE; - int quote = FALSE; + bool double_quote = false; int sz = 0; - char *p = str; + char *p = str, *q;
Maybe it's just me, but I tend to declare multiple pointers on separate lines. But no big deal.
- } else if (quote&& *p == '"') { - quote = FALSE; + } + + if (*p == '"') {
'make syntax-check' would have caught this TAB.
+ double_quote = !double_quote; p++; - break; /* end of "..." token */ + continue; } + + if (*res) + (*res)[sz] = *p;
That's a lot of indirection, for every byte of the loop. Wouldn't it be better to have a local temporary, and only assign to *res at the end?
p++; sz++; } - if (quote) { + if (double_quote) { vshError(ctl, "%s", _("missing \"")); return VSH_TK_ERROR; } @@ -10231,8 +10234,12 @@ vshCommandGetToken(vshControl *ctl, char *str, char **end, char **res) if (sz == 0) return VSH_TK_END;
- *res = vshMalloc(ctl, sz + 1); - memcpy(*res, tkstr, sz); + if (!*res) { + *res = vshMalloc(ctl, sz + 1); + sz = 0; + p = q; + goto copy; + }
Hmm, a backwards jump, which means we parse every string twice - once to figure out how long it is, and again to strip quotes. Yes, that avoids over-allocating, but are we really that tight on memory? I find it quicker to just strdup() up front, then edit in-place on a single pass. Hmm, one thing you _don't_ recognize is: -"-"option as an option. In the shell, quotes are stripped before arguments are recognized as a particular token. I think that's pretty easy to support - delay VSH_TK_OPTION checking until after we've stripped quotes, but I'll show it as a separate patch. Hmm, I also noticed that we're inconsistent on strdup/vshStrdup in this file; separate cleanup patch for that coming up soon. But, with those nits fixed, ACK. Here's what I'm squashing in to my local tree; I'll push it once I complete my review of your other 7 patches, as well as getting approval to my promised followup patches. diff --git i/AUTHORS w/AUTHORS index 09169f2..a8f96df 100644 --- i/AUTHORS +++ w/AUTHORS @@ -129,6 +129,7 @@ Patches have also been contributed by: diff --git i/tools/virsh.c w/tools/virsh.c index 0a28c99..4f70724 100644 --- i/tools/virsh.c +++ w/tools/virsh.c @@ -10124,16 +10124,18 @@ vshCommandRun(vshControl *ctl, const vshCmd *cmd) #define VSH_TK_DATA 2 #define VSH_TK_END 3 -static int +static int ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4) 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; + char *p = str; + char *q = vshStrdup(ctl, str); char *tkstr = NULL; *end = NULL; + *res = q; while (p && *p && (*p == ' ' || *p == '\t')) p++; @@ -10145,9 +10147,6 @@ vshCommandGetToken(vshControl *ctl, char *str, char **end, char **res) return VSH_TK_END; } - q = p; - *res = NULL; -copy: while (*p) { /* end of token is blank space or ';' */ if (!double_quote && (*p == ' ' || *p == '\t' || *p == ';')) @@ -10170,34 +10169,23 @@ copy: tkstr = p; /* begin of token */ } - if (*p == '"') { + if (*p == '"') { double_quote = !double_quote; p++; continue; } - if (*res) - (*res)[sz] = *p; - p++; + *q++ = *p++; sz++; } if (double_quote) { vshError(ctl, "%s", _("missing \"")); return VSH_TK_ERROR; } - if (tkstr == NULL || *tkstr == '\0' || p == NULL) - return VSH_TK_END; - if (sz == 0) + if (tkstr == NULL || *tkstr == '\0' || sz == 0) return VSH_TK_END; - if (!*res) { - *res = vshMalloc(ctl, sz + 1); - sz = 0; - p = q; - goto copy; - } - *(*res + sz) = '\0'; - + *q = '\0'; *end = p; return tk; } -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org