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

[Osier Yang]

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.

>
>>
>>> + * @cmd command structure
>>> + * @name option name
>>> + * @value result (updated to NULL or the actual value)
>>
>> s/actual value/option argument/,
>>
>>> + *
>>
>> [1]. Right here.
>>
>
> Peter