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