On 2013年02月04日 19:32, Peter Krempa wrote:
On 02/04/13 12:28, Osier Yang wrote:
On 2013年02月04日 18:47, Peter Krempa wrote:
On 01/31/13 06:18, Osier Yang wrote:
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.
Not in virsh.c. All surrounding functions don't have the colon in the comment. This is worth cleaning up separately instead of doing it in multiple ways in a single file.
Agreed, a follow up patch will be good then. But others still stands.
I changed the comment to: /** + * vshCommandOptStringReq: + * @ctl virsh control structure + * @cmd command structure + * @name option name + * @value result (updated to NULL or the option argument) + * + * Gets a option argument as string. This function reports errors.
I think "The function reports errors" can just be removed. As it doesn't report any error on success.
+ * + * Returns 0 on success or when the option is not present and not + * required, *value is set to the option argument. On error -1 is + * returned and error message printed. + */
Peter
ACK with that.