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

[Michal Privoznik <mprivozn@xxxxxxxxxx>]

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 |  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);
+        }
         return -1;
+    }
     return 1;
 }
 
@@ -1514,8 +1524,18 @@ vshCommandOptUInt(const vshCmd *cmd, const char *name, unsigned int *value)
     if (ret <= 0)
         return ret;
 
-    if (virStrToLong_ui(arg->data, NULL, 10, value) < 0)
+    if (virStrToLong_ui(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);
+        }
         return -1;
+    }
     return 1;
 }
 
@@ -1539,8 +1559,18 @@ vshCommandOptUL(const vshCmd *cmd, const char *name, unsigned long *value)
     if (ret <= 0)
         return ret;
 
-    if (virStrToLong_ul(arg->data, NULL, 10, value) < 0)
+    if (virStrToLong_ul(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);
+        }
         return -1;
+    }
     return 1;
 }
 
@@ -1638,8 +1668,18 @@ vshCommandOptLongLong(const vshCmd *cmd, const char *name,
     if (ret <= 0)
         return ret;
 
-    if (virStrToLong_ll(arg->data, NULL, 10, value) < 0)
+    if (virStrToLong_ll(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);
+        }
         return -1;
+    }
     return 1;
 }
 
@@ -1663,8 +1703,18 @@ vshCommandOptULongLong(const vshCmd *cmd, const char *name,
     if (ret <= 0)
         return ret;
 
-    if (virStrToLong_ull(arg->data, NULL, 10, value) < 0)
+    if (virStrToLong_ull(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);
+        }
         return -1;
+    }
     return 1;
 }
 
@@ -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 */
-- 
1.9.0

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list