Re: [PATCH 04/17] virsh: Add helper to request string arguments with error reporting

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2013年01月22日 02:07, Peter Krempa wrote:
This patch adds a helper function with similar semantics to
vshCommandOptString that requests a string argument, but does some error
reporting without the need to do it in the functions themselves.

The error reporting also provides information about the parameter whose
retrieval failed.
---
  tools/virsh.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++
  tools/virsh.h |  4 ++++
  2 files changed, 55 insertions(+)

diff --git a/tools/virsh.c b/tools/virsh.c
index 908c6a1..1a3cab0 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -1417,6 +1417,57 @@ vshCommandOptString(const vshCmd *cmd, const char *name, const char **value)
  }

  /**
+ * vshCommandOptStringReq: Get a required string argumment

Trivial, but we usually describe what the function does at [1].

+ * @ctl virsh control structure

And have a ":" after @foo.

+ * @cmd command structure
+ * @name option name
+ * @value result (updated to NULL or the actual value)

s/actual value/option argument/,

+ *

[1]. Right here.

+ * Returns option as string. Prints error message if needed

I think you mean option argument here.

+ * Return value:
+ * 0 if option is correct (available or not required)
+ * -1 and error message printed on error

Not to be captious. But how about rewording above like:

Returns 0 on success or the option is not present and not
requirent, *value is set to the option argument if success;
Or -1 on failure, with error message printed.

+ */
+int
+vshCommandOptStringReq(vshControl *ctl,
+                       const vshCmd *cmd,
+                       const char *name,
+                       const char **value)
+{
+    vshCmdOpt *arg;
+    int ret;
+    const char *error = NULL;
+
+    /* clear out the value */
+    *value = NULL;
+
+    ret = vshCommandOpt(cmd, name,&arg);
+    /* option is not required and not present */
+    if (ret == 0)
+        return 0;
+    /* this should not be propagated here, just to be sure */
+    if (ret == -1)
+        error = N_("Mandatory option not present");
+
+    if (ret == -2)
+        error = N_("Programming error: Invalid option name");
+
+    if (!arg->data)
+        error = N_("Programming error: Requested option is a boolean");
+
+    if (!*arg->data&&  !(arg->def->flags&  VSH_OFLAG_EMPTY_OK))
+        error = N_("Option argument is empty");
+
+    if (error) {
+        vshError(ctl, _("Failed to get option '%s': %s"), name, _(error));
+        return -1;
+    }
+
+    *value = arg->data;
+    return 0;
+}
+
+/**
   * vshCommandOptLongLong:
   * @cmd command reference
   * @name option name
diff --git a/tools/virsh.h b/tools/virsh.h
index ab7161f..fec9785 100644
--- a/tools/virsh.h
+++ b/tools/virsh.h
@@ -275,6 +275,10 @@ int vshCommandOptUL(const vshCmd *cmd, const char *name,
  int vshCommandOptString(const vshCmd *cmd, const char *name,
                          const char **value)
      ATTRIBUTE_NONNULL(3) ATTRIBUTE_RETURN_CHECK;
+int vshCommandOptStringReq(vshControl *ctl, const vshCmd *cmd,
+                           const char *name, const char **value)
+    ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3)
+    ATTRIBUTE_NONNULL(4) ATTRIBUTE_RETURN_CHECK;
  int vshCommandOptLongLong(const vshCmd *cmd, const char *name,
                            long long *value)
      ATTRIBUTE_NONNULL(3) ATTRIBUTE_RETURN_CHECK;

ACK with the small nits fixed.

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



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]