On Tue, Jun 07, 2011 at 05:11:09PM +0800, Lai Jiangshan wrote:
> Signed-off-by: Lai Jiangshan <laijs@xxxxxxxxxxxxxx>
> ---
> tools/virsh.c | 47 ++++++++++++++++++++++++-----------------------
> 1 files changed, 24 insertions(+), 23 deletions(-)
>
> diff --git a/tools/virsh.c b/tools/virsh.c
> index 61eb11e..638029c 100644
> --- a/tools/virsh.c
> +++ b/tools/virsh.c
> @@ -280,7 +280,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)
> @@ -10341,6 +10361,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;
>
> @@ -10349,10 +10370,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) {
Stylewise, I'm not really a fan of hiding for/while loops
behind macros. I prefer to see the loop logic unobscured.
Since 'for_each_variable_arg' is only used once and the
code isn't really significantly simpler/ shorter, I'd
prefer if we just removed the macro here.
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list