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

[Osier Yang]

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.