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 Jiangshanla...@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_DATA2
#define VSH_TK_END3
-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 +
