Re: [libvirt] [PATCH 1/7] virsh: Make vshCommandOpt* report error

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(a)redhat.com&gt; --- tests/vcpupin | 1 + tools/virsh.c | 72 +++++++++++++++++++++++++++++++++++++++++++++++++++++------

tools/virsh.h | 2 +- 3 files changed, 67 insertions(+), 8 deletions(-) diff --git a/tests/vcpupin b/tests/vcpupin index f1fb038..a616216 100755 --- a/tests/vcpupin +++ b/tests/vcpupin @@ -34,6 +34,7 @@ fail=0 $abs_top_builddir/tools/virsh --connect test:///default vcpupin test a 0,1 > out 2>&1 test $? = 1 || fail=1 cat <<\EOF > exp || fail=1 +error: Unable to parse integer parameter to --vcpu error: vcpupin: Invalid or missing vCPU number. EOF diff --git a/tools/virsh.c b/tools/virsh.c index f2e4c4a..1371abb 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -1489,8 +1489,18 @@ vshCommandOptInt(const vshCmd *cmd, const char *name, int *value) if (ret <= 0) return ret; - if (virStrToLong_i(arg->data, NULL, 10, value) < 0) + if (virStrToLong_i(arg->data, NULL, 10, value) < 0) { + if (arg->def->flags & VSH_OFLAG_REQ) { + vshError(NULL,

+ _("missing --%s parameter value"), + name);

+ } else { + vshError(NULL, + _("Unable to parse integer parameter to --%s"), + name);

@@ -1692,9 +1742,17 @@ vshCommandOptScaledInt(const vshCmd *cmd, const char *name, ret = vshCommandOptString(cmd, name, &str); if (ret <= 0) return ret; - if (virStrToLong_ull(str, &end, 10, value) < 0 || - virScaleInteger(value, end, scale, max) < 0) + if (virStrToLong_ull(str, &end, 10, value) < 0) { + vshError(NULL, + _("Unable to parse integer parameter to --%s"), + name); return -1; + } + + if (virScaleInteger(value, end, scale, max) < 0) { + /* Error reported by the helper. */

+ return -1; + } return 1; } diff --git a/tools/virsh.h b/tools/virsh.h index 3e0251b..6eb17b3 100644 --- a/tools/virsh.h +++ b/tools/virsh.h @@ -168,7 +168,7 @@ struct _vshCmdInfo { struct _vshCmdOptDef { const char *name; /* the name of option, or NULL for list end */ vshCmdOptType type; /* option type */ - unsigned int flags; /* flags */ + unsigned int flags; /* bitwise OR of VSH_OFLAG_**/ const char *help; /* non-NULL help string; or for VSH_OT_ALIAS * the name of a later public option */ vshCompleter completer; /* option completer */