[libvirt] [PATCH 4/8] virsh: add vrshCommandParser abstraction
add vrshCommandParser and make vshCommandParse() accepts different parsers. the current code for parse command string is integrated as vshCommandStringParse(). Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com> --- virsh.c | 91 +++++++++++++++++++++++++++++++--------------------------------- 1 file changed, 45 insertions(+), 46 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index f97ee42..27321a5 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -10158,23 +10158,29 @@ vshCommandRun(vshControl *ctl, const vshCmd *cmd) return ret; } +#define VSH_TK_ERROR -1 +#define VSH_TK_ARG 0 +#define VSH_TK_SUBCMD_END 1 +#define VSH_TK_END 2 + +typedef struct __vshCommandParser { + int (*getNextArg)(vshControl *, struct __vshCommandParser *, char **); + char *pos; +} vshCommandParser; + +static int vshCommandParse(vshControl *ctl, vshCommandParser *parser); + /* --------------- * Command string parsing * --------------- */ -#define VSH_TK_ERROR -1 -#define VSH_TK_NONE 0 -#define VSH_TK_DATA 1 -#define VSH_TK_END 2 static int -vshCommandGetToken(vshControl *ctl, char *str, char **end, char **res) +vshCommandStringGetArg(vshControl *ctl, vshCommandParser *parser, char **res) { bool double_quote = false; int sz = 0; - char *p = str, *q; - - *end = NULL; + char *p = parser->pos, *q; while (p && *p && (*p == ' ' || *p == '\t')) p++; @@ -10182,8 +10188,8 @@ vshCommandGetToken(vshControl *ctl, char *str, char **end, char **res) if (p == NULL || *p == '\0') return VSH_TK_END; if (*p == ';') { - *end = ++p; /* = \0 or begin of next command */ - return VSH_TK_END; + parser->pos = ++p; /* = \0 or begin of next command */ + return VSH_TK_SUBCMD_END; } q = p; @@ -10218,14 +10224,25 @@ copy: } *(*res + sz) = '\0'; - *end = p; - return VSH_TK_DATA; + parser->pos = p; + return VSH_TK_ARG; +} + +static int vshCommandStringParse(vshControl *ctl, char *cmdstr) +{ + vshCommandParser parser; + + if (cmdstr == NULL || *cmdstr == '\0') + return FALSE; + + parser.pos = cmdstr; + parser.getNextArg = vshCommandStringGetArg; + return vshCommandParse(ctl, &parser); } static int -vshCommandParse(vshControl *ctl, char *cmdstr) +vshCommandParse(vshControl *ctl, vshCommandParser *parser) { - char *str; char *tkdata = NULL; vshCmd *clast = NULL; vshCmdOpt *first = NULL; @@ -10235,44 +10252,27 @@ vshCommandParse(vshControl *ctl, char *cmdstr) ctl->cmd = NULL; } - if (cmdstr == NULL || *cmdstr == '\0') - return FALSE; - - str = cmdstr; - while (str && *str) { + for (;;) { vshCmdOpt *last = NULL; const vshCmdDef *cmd = NULL; - int tk = VSH_TK_NONE; + int tk; int data_ct = 0; first = NULL; - while (tk != VSH_TK_END) { - char *end = NULL; + for (;;) { const vshCmdOptDef *opt = NULL; tkdata = NULL; + tk = parser->getNextArg(ctl, parser, &tkdata); - /* get token */ - tk = vshCommandGetToken(ctl, str, &end, &tkdata); - - str = end; - - if (tk == VSH_TK_END) { - VIR_FREE(tkdata); - break; - } if (tk == VSH_TK_ERROR) goto syntaxError; + if (tk != VSH_TK_ARG) + break; if (cmd == NULL) { /* first token must be command name */ - if (tk != VSH_TK_DATA) { - vshError(ctl, - _("unexpected token (command name): '%s'"), - tkdata); - goto syntaxError; - } if (!(cmd = vshCmddefSearch(tkdata))) { vshError(ctl, _("unknown command: '%s'"), tkdata); goto syntaxError; /* ... or ignore this command only? */ @@ -10299,11 +10299,10 @@ vshCommandParse(vshControl *ctl, char *cmdstr) if (optstr) tkdata = optstr; else - tk = vshCommandGetToken(ctl, str, &end, &tkdata); - str = end; + tk = parser->getNextArg(ctl, parser, &tkdata); if (tk == VSH_TK_ERROR) goto syntaxError; - if (tk != VSH_TK_DATA) { + if (tk != VSH_TK_ARG) { vshError(ctl, _("expected syntax: --%s <%s>"), opt->name, @@ -10320,7 +10319,7 @@ vshCommandParse(vshControl *ctl, char *cmdstr) goto syntaxError; } } - } else if (tk == VSH_TK_DATA) { + } else { if (!(opt = vshCmddefGetData(cmd, data_ct++))) { vshError(ctl, _("unexpected data '%s'"), tkdata); goto syntaxError; @@ -10347,8 +10346,6 @@ vshCommandParse(vshControl *ctl, char *cmdstr) opt->type != VSH_OT_BOOL ? _("optdata") : _("bool"), opt->type != VSH_OT_BOOL ? arg->data : _("(none)")); } - if (!str) - break; } /* command parsed -- allocate new struct for the command */ @@ -10370,6 +10367,9 @@ vshCommandParse(vshControl *ctl, char *cmdstr) clast->next = c; clast = c; } + + if (tk == VSH_TK_END) + break; } return TRUE; @@ -10385,7 +10385,6 @@ vshCommandParse(vshControl *ctl, char *cmdstr) return FALSE; } - /* --------------- * Misc utils * --------------- @@ -11086,7 +11085,7 @@ vshParseArgv(vshControl *ctl, int argc, char **argv) strncat(cmdstr, " ", sz--); } vshDebug(ctl, 2, "command: \"%s\"\n", cmdstr); - ret = vshCommandParse(ctl, cmdstr); + ret = vshCommandStringParse(ctl, cmdstr); VIR_FREE(cmdstr); return ret; @@ -11165,7 +11164,7 @@ main(int argc, char **argv) #if USE_READLINE add_history(ctl->cmdstr); #endif - if (vshCommandParse(ctl, ctl->cmdstr)) + if (vshCommandStringParse(ctl, ctl->cmdstr)) vshCommandRun(ctl, ctl->cmd); } VIR_FREE(ctl->cmdstr);
On 10/12/2010 01:13 AM, Lai Jiangshan wrote:
add vrshCommandParser and make vshCommandParse() accepts different
s/vrsh/vsh/; s/accepts/accept/
parsers.
the current code for parse command string is integrated as vshCommandStringParse().
Signed-off-by: Lai Jiangshan<laijs@cn.fujitsu.com> --- virsh.c | 91 +++++++++++++++++++++++++++++++--------------------------------- 1 file changed, 45 insertions(+), 46 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index f97ee42..27321a5 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -10158,23 +10158,29 @@ vshCommandRun(vshControl *ctl, const vshCmd *cmd) return ret; }
+#define VSH_TK_ERROR -1 +#define VSH_TK_ARG 0 +#define VSH_TK_SUBCMD_END 1 +#define VSH_TK_END 2
Hmm, in addition to floating this earlier, you also lost _NONE and added _SUBCMD_END. The enum I suggested in patch 3 is sounding more appealing, so I'll squash it in now.
+ +typedef struct __vshCommandParser { + int (*getNextArg)(vshControl *, struct __vshCommandParser *, char **); + char *pos; +} vshCommandParser; + +static int vshCommandParse(vshControl *ctl, vshCommandParser *parser);
I'm a fan of avoiding forward static declarations if the file can be rearranged topologically.
+ /* --------------- * Command string parsing * --------------- */
This logically belongs before enums related to command parsing.
-#define VSH_TK_ERROR -1 -#define VSH_TK_NONE 0 -#define VSH_TK_DATA 1 -#define VSH_TK_END 2
static int -vshCommandGetToken(vshControl *ctl, char *str, char **end, char **res) +vshCommandStringGetArg(vshControl *ctl, vshCommandParser *parser, char **res) { bool double_quote = false; int sz = 0; - char *p = str, *q; - - *end = NULL; + char *p = parser->pos, *q;
Rebasing this on top of my edits to patch 1 is getting interesting.
while (p&& *p&& (*p == ' ' || *p == '\t'))
You no longer need to check if p is NULL,
+static int vshCommandStringParse(vshControl *ctl, char *cmdstr) +{ + vshCommandParser parser; + + if (cmdstr == NULL || *cmdstr == '\0') + return FALSE; + + parser.pos = cmdstr;
...since this guarantees it is non-NULL.
+ for (;;) {
$ git grep 'for.*;;' | wc -l 14 $ git grep 'while.*true' | wc -l 6 $ git grep 'while.*1' | wc -l 70 Guess which one I'm changing this to, for consistency :) ACK, with those nits fixed. Here's what I squashed in: diff --git i/tools/virsh.c w/tools/virsh.c index 56985a4..d9f72f3 100644 --- i/tools/virsh.c +++ w/tools/virsh.c @@ -10174,38 +10174,40 @@ vshCommandRun(vshControl *ctl, const vshCmd *cmd) return ret; } -#define VSH_TK_ERROR -1 -#define VSH_TK_ARG 0 -#define VSH_TK_SUBCMD_END 1 -#define VSH_TK_END 2 +/* --------------- + * Command string parsing + * --------------- + */ + +typedef enum { + VSH_TK_ERROR, /* Failed to parse a token */ + VSH_TK_ARG, /* Arbitrary argument, might be option or empty */ + VSH_TK_SUBCMD_END, /* Separation between commands */ + VSH_TK_END /* No more commands */ +} vshCommandToken; typedef struct __vshCommandParser { - int (*getNextArg)(vshControl *, struct __vshCommandParser *, char **); + vshCommandToken (*getNextArg)(vshControl *, struct __vshCommandParser *, + char **); char *pos; } vshCommandParser; static int vshCommandParse(vshControl *ctl, vshCommandParser *parser); -/* --------------- - * Command string parsing - * --------------- - */ - -static int ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) +static vshCommandToken ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) vshCommandStringGetArg(vshControl *ctl, vshCommandParser *parser, char **res) { bool double_quote = false; int sz = 0; char *p = parser->pos; - char *q = vshStrdup(ctl, str); + char *q = vshStrdup(ctl, p); - *end = NULL; *res = q; - while (p && *p && (*p == ' ' || *p == '\t')) + while (*p && (*p == ' ' || *p == '\t')) p++; - if (p == NULL || *p == '\0') + if (*p == '\0') return VSH_TK_END; if (*p == ';') { parser->pos = ++p; /* = \0 or begin of next command */ @@ -10261,15 +10262,15 @@ vshCommandParse(vshControl *ctl, vshCommandParser *parser) ctl->cmd = NULL; } - for (;;) { + while (1) { vshCmdOpt *last = NULL; const vshCmdDef *cmd = NULL; - int tk; + vshCommandToken tk; int data_ct = 0; first = NULL; - for (;;) { + while (1) { const vshCmdOptDef *opt = NULL; tkdata = NULL; -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
* tools/virsh.c (vshCommandParse): Float up, to avoid the need for a forward declaration. ---
+static int vshCommandParse(vshControl *ctl, vshCommandParser *parser);
I'm a fan of avoiding forward static declarations if the file can be rearranged topologically.
Like this. Should be a no-op change; applies after my patch followup to 8/8. tools/virsh.c | 220 ++++++++++++++++++++++++++++----------------------------- 1 files changed, 109 insertions(+), 111 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 0169493..4929f71 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -10322,7 +10322,7 @@ vshCommandRun(vshControl *ctl, const vshCmd *cmd) } /* --------------- - * Command string parsing + * Command parsing * --------------- */ @@ -10343,116 +10343,6 @@ typedef struct __vshCommandParser { char **arg_end; } vshCommandParser; -static int vshCommandParse(vshControl *ctl, vshCommandParser *parser); - -/* --------------- - * Command argv parsing - * --------------- - */ - -static vshCommandToken ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) -vshCommandArgvGetArg(vshControl *ctl, vshCommandParser *parser, char **res) -{ - if (parser->arg_pos == parser->arg_end) { - *res = NULL; - return VSH_TK_END; - } - - *res = vshStrdup(ctl, *parser->arg_pos); - parser->arg_pos++; - return VSH_TK_ARG; -} - -static int vshCommandArgvParse(vshControl *ctl, int nargs, char **argv) -{ - vshCommandParser parser; - - if (nargs <= 0) - return FALSE; - - parser.arg_pos = argv; - parser.arg_end = argv + nargs; - parser.getNextArg = vshCommandArgvGetArg; - return vshCommandParse(ctl, &parser); -} - -/* --------------- - * Command string parsing - * --------------- - */ - -static vshCommandToken ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) -vshCommandStringGetArg(vshControl *ctl, vshCommandParser *parser, char **res) -{ - bool single_quote = false; - bool double_quote = false; - int sz = 0; - char *p = parser->pos; - char *q = vshStrdup(ctl, p); - - *res = q; - - while (*p && (*p == ' ' || *p == '\t')) - p++; - - if (*p == '\0') - return VSH_TK_END; - if (*p == ';') { - parser->pos = ++p; /* = \0 or begin of next command */ - return VSH_TK_SUBCMD_END; - } - - while (*p) { - /* end of token is blank space or ';' */ - if (!double_quote && !single_quote && - (*p == ' ' || *p == '\t' || *p == ';')) - break; - - if (!double_quote && *p == '\'') { /* single quote */ - single_quote = !single_quote; - p++; - continue; - } else if (!single_quote && *p == '\\') { /* escape */ - /* - * The same as the bash, a \ in "" is an escaper, - * but a \ in '' is not an escaper. - */ - p++; - if (*p == '\0') { - vshError(ctl, "%s", _("dangling \\")); - return VSH_TK_ERROR; - } - } else if (!single_quote && *p == '"') { /* double quote */ - double_quote = !double_quote; - p++; - continue; - } - - *q++ = *p++; - sz++; - } - if (double_quote) { - vshError(ctl, "%s", _("missing \"")); - return VSH_TK_ERROR; - } - - *q = '\0'; - parser->pos = p; - return VSH_TK_ARG; -} - -static int vshCommandStringParse(vshControl *ctl, char *cmdstr) -{ - vshCommandParser parser; - - if (cmdstr == NULL || *cmdstr == '\0') - return FALSE; - - parser.pos = cmdstr; - parser.getNextArg = vshCommandStringGetArg; - return vshCommandParse(ctl, &parser); -} - static int vshCommandParse(vshControl *ctl, vshCommandParser *parser) { @@ -10606,6 +10496,114 @@ get_data: return FALSE; } +/* -------------------- + * Command argv parsing + * -------------------- + */ + +static vshCommandToken ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) +vshCommandArgvGetArg(vshControl *ctl, vshCommandParser *parser, char **res) +{ + if (parser->arg_pos == parser->arg_end) { + *res = NULL; + return VSH_TK_END; + } + + *res = vshStrdup(ctl, *parser->arg_pos); + parser->arg_pos++; + return VSH_TK_ARG; +} + +static int vshCommandArgvParse(vshControl *ctl, int nargs, char **argv) +{ + vshCommandParser parser; + + if (nargs <= 0) + return FALSE; + + parser.arg_pos = argv; + parser.arg_end = argv + nargs; + parser.getNextArg = vshCommandArgvGetArg; + return vshCommandParse(ctl, &parser); +} + +/* ---------------------- + * Command string parsing + * ---------------------- + */ + +static vshCommandToken ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) +vshCommandStringGetArg(vshControl *ctl, vshCommandParser *parser, char **res) +{ + bool single_quote = false; + bool double_quote = false; + int sz = 0; + char *p = parser->pos; + char *q = vshStrdup(ctl, p); + + *res = q; + + while (*p && (*p == ' ' || *p == '\t')) + p++; + + if (*p == '\0') + return VSH_TK_END; + if (*p == ';') { + parser->pos = ++p; /* = \0 or begin of next command */ + return VSH_TK_SUBCMD_END; + } + + while (*p) { + /* end of token is blank space or ';' */ + if (!double_quote && !single_quote && + (*p == ' ' || *p == '\t' || *p == ';')) + break; + + if (!double_quote && *p == '\'') { /* single quote */ + single_quote = !single_quote; + p++; + continue; + } else if (!single_quote && *p == '\\') { /* escape */ + /* + * The same as the bash, a \ in "" is an escaper, + * but a \ in '' is not an escaper. + */ + p++; + if (*p == '\0') { + vshError(ctl, "%s", _("dangling \\")); + return VSH_TK_ERROR; + } + } else if (!single_quote && *p == '"') { /* double quote */ + double_quote = !double_quote; + p++; + continue; + } + + *q++ = *p++; + sz++; + } + if (double_quote) { + vshError(ctl, "%s", _("missing \"")); + return VSH_TK_ERROR; + } + + *q = '\0'; + parser->pos = p; + return VSH_TK_ARG; +} + +static int vshCommandStringParse(vshControl *ctl, char *cmdstr) +{ + vshCommandParser parser; + + if (cmdstr == NULL || *cmdstr == '\0') + return FALSE; + + parser.pos = cmdstr; + parser.getNextArg = vshCommandStringGetArg; + return vshCommandParse(ctl, &parser); +} + /* --------------- * Misc utils * --------------- -- 1.7.2.3
participants (2)
-
Eric Blake -
Lai Jiangshan