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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list