Re: [libvirt] [PATCH 02/13] improve the iteration of VSH_OT_ARGV options

On Wed, May 25, 2011 at 05:37:44PM +0800, Lai Jiangshan wrote: > Signed-off-by: Lai Jiangshan <laijs(a)fujitsu.com&gt; > --- > tools/virsh.c | 47 ++++++++++++++++++++++++----------------------- > 1 files changed, 24 insertions(+), 23 deletions(-) > > diff --git a/tools/virsh.c b/tools/virsh.c > index c358580..2e27535 100644 > --- a/tools/virsh.c > +++ b/tools/virsh.c > @@ -277,7 +277,27 @@ static int vshCommandOptULongLong(const vshCmd *cmd, const char *name, > unsigned long long *value) > ATTRIBUTE_NONNULL(3) ATTRIBUTE_RETURN_CHECK; > static bool vshCommandOptBool(const vshCmd *cmd, const char *name); > -static char *vshCommandOptArgv(const vshCmd *cmd, int count); > + > +/* > + * Iterate all the argv arguments. > + * > + * Requires that a VSH_OT_ARGV option be last in the > + * list of supported options in CMD->def->opts. > + */ > +static inline const vshCmdOpt *__variable_arg(const vshCmdOpt *opt) > +{ > + while (opt) { > + if (opt->def && opt->def->type == VSH_OT_ARGV) > + break; > + opt = opt->next; > + } > + > + return opt; > +} > + > +#define for_each_variable_arg(cmd, opt) \ > + for (opt = __variable_arg(cmd->opts); opt; opt = __variable_arg(opt->next)) > + > > #define VSH_BYID (1 << 1) > #define VSH_BYUUID (1 << 2) > @@ -10059,6 +10079,7 @@ cmdEcho (vshControl *ctl ATTRIBUTE_UNUSED, const vshCmd *cmd) > bool shell = false; > bool xml = false; > int count = 0; > + const vshCmdOpt *opt; > char *arg; > virBuffer buf = VIR_BUFFER_INITIALIZER; > > @@ -10067,10 +10088,11 @@ cmdEcho (vshControl *ctl ATTRIBUTE_UNUSED, const vshCmd *cmd) > if (vshCommandOptBool(cmd, "xml")) > xml = true; > > - while ((arg = vshCommandOptArgv(cmd, count)) != NULL) { > + for_each_variable_arg(cmd, opt) { > bool close_quote = false; > char *q; > > + arg = opt->data; > if (count) > virBufferAddChar(&buf, ' '); > /* Add outer '' only if arg included shell metacharacters. */ > @@ -11484,27 +11506,6 @@ vshCommandOptBool(const vshCmd *cmd, const char *name) > return vshCommandOpt(cmd, name) != NULL; > } > > -/* > - * Returns the COUNT argv argument, or NULL after last argument. > - * > - * Requires that a VSH_OT_ARGV option with the name "" be last in the > - * list of supported options in CMD->def->opts. > - */ > -static char * > -vshCommandOptArgv(const vshCmd *cmd, int count) > -{ > - vshCmdOpt *opt = cmd->opts; > - > - while (opt) { > - if (opt->def && opt->def->type == VSH_OT_ARGV) { > - if (count-- == 0) > - return opt->data; > - } > - opt = opt->next; > - } > - return NULL; > -} > - > /* Determine whether CMD->opts includes an option with name OPTNAME. > If not, give a diagnostic and return false. > If so, return true. */ I'm not entirely sure I understand what the effect of this patch is. Can you explain what the change in semantics for the parser is with this patch applied