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

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. -- libvir-list mailing list libvir-list(a)redhat.com https://www.redhat.com/mailman/listinfo/libvir-list