On 04/04/2014 03:50 PM, Michal Privoznik wrote:
> Currently, the virsh code is plenty of the following pattern:
>
> if (vshCommandOptUInt(..) < 0) {
> vshError(...);
> goto cleanup;
> }
>
> It doesn't make much sense to repeat the code everywhere.
> Moreover, some functions from the family already report
> error some of them don't.
>
> Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
> ---
> tests/vcpupin | 2 +-
> tools/virsh-domain-monitor.c | 7 +--
> tools/virsh-domain.c | 102 +++++++++++++++----------------------------
> tools/virsh-host.c | 25 +++--------
> tools/virsh-interface.c | 4 +-
> tools/virsh-network.c | 4 +-
> tools/virsh-volume.c | 16 ++-----
> tools/virsh.c | 99 +++++++++++++++++++++++++++++++----------
> tools/virsh.h | 24 +++++-----
> 9 files changed, 140 insertions(+), 143 deletions(-)
>
> @@ -5819,9 +5806,7 @@ cmdVcpuPin(vshControl *ctl, const vshCmd *cmd)
> query = !cpulist;
>
> /* In query mode, "vcpu" is optional */
> - if (vshCommandOptInt(cmd, "vcpu", &vcpu) < !query) {
> - vshError(ctl, "%s",
> - _("vcpupin: Invalid or missing vCPU number."));
> + if (vshCommandOptInt(ctl, cmd, "vcpu", &vcpu) < !query) {
This should report an error for a missing option if query == false.
> virDomainFree(dom);
> return false;
> }
> @@ -8123,9 +8099,11 @@ cmdQemuAttach(vshControl *ctl, const vshCmd *cmd)
> bool ret = false;
> unsigned int flags = 0;
> unsigned int pid_value; /* API uses unsigned int, not pid_t */
> + int rv;
>
> - if (vshCommandOptUInt(cmd, "pid", &pid_value) <= 0) {
> - vshError(ctl, "%s", _("missing pid value"));
> + if ((rv = vshCommandOptUInt(ctl, cmd, "pid", &pid_value)) <= 0) {
> + if (rv == 0)
> + vshError(ctl, "%s", _("missing pid value"));
pid has the VSH_OFLAG_REQ flag set, 0 shouldn't be returned here.
> goto cleanup;
> }
>
> @@ -1469,6 +1469,7 @@ vshCommandOpt(const vshCmd *cmd, const char *name, vshCmdOpt **opt,
>
> /**
> * vshCommandOptInt:
> + * @ctl virsh control structure
> * @cmd command reference
> * @name option name
> * @value result
> @@ -1480,7 +1481,10 @@ vshCommandOpt(const vshCmd *cmd, const char *name, vshCmdOpt **opt,
> * <0 in all other cases (@value untouched)
> */
> int
> -vshCommandOptInt(const vshCmd *cmd, const char *name, int *value)
> +vshCommandOptInt(vshControl *ctl,
> + const vshCmd *cmd,
> + const char *name,
> + int *value)
> {
> vshCmdOpt *arg;
> int ret;
> @@ -1489,14 +1493,19 @@ vshCommandOptInt(const vshCmd *cmd, const char *name, int *value)
> if (ret <= 0)
Here you don't report an error for -1,
> return ret;
>
> - if (virStrToLong_i(arg->data, NULL, 10, value) < 0)
> + if (virStrToLong_i(arg->data, NULL, 10, value) < 0) {
> + vshError(ctl,
> + _("Unable to parse integer parameter to --%s"),
> + name);
here you do.
> return -1;
> + }
> return 1;
> }
>
Jan
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list