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@redhat.com> --- 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