[libvirt] [PATCH v3 5/5] virsh: Move error messages inside vshCommandOpt*() functions.

[John Ferlan]

On 05/28/2015 05:31 AM, Andrea Bolognani wrote:
> ---
>  tests/vcpupin                |   4 +-
>  tools/virsh-domain-monitor.c |   9 +--
>  tools/virsh-domain.c         | 134 +++++++------------------------------------
>  tools/virsh-host.c           |  61 +++-----------------
>  tools/virsh-interface.c      |   6 +-
>  tools/virsh-network.c        |   6 +-
>  tools/virsh-volume.c         |  24 ++------
>  tools/virsh.c                | 121 ++++++++++++++++++++++----------------
>  8 files changed, 109 insertions(+), 256 deletions(-)
> 

...

> --- a/tools/virsh.c
> +++ b/tools/virsh.c


...
>  
>  /*
> @@ -1664,8 +1676,7 @@ vshCommandOptString(vshControl *ctl ATTRIBUTE_UNUSED, const vshCmd *cmd,
>      vshCmdOpt *arg;
>      int ret;
>  
> -    ret = vshCommandOpt(cmd, name, &arg, true);
> -    if (ret <= 0)
> +    if ((ret = vshCommandOpt(cmd, name, &arg, true)) <= 0)
>          return ret;

Too bad a few places decide to ignore the return status and continue on;
otherwise, you could move the following in here too:

vshError(ctl, "%s", _("Unable to parse string parameter"));



>  
>      if (!*arg->data && !(arg->def->flags & VSH_OFLAG_EMPTY_OK))

vshError(ctl, "%s", _("Cannot supply empty string parameter"));

or "Must supply non-empty string parameter"?

The rest seems to have followed Michal's previous review comments.

John