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

Re: [libvirt] [PATCH] virsh: detect programming errors with option parsing

On 08/28/2013 10:31 AM, Michal Privoznik wrote:
> On 17.08.2013 00:14, Eric Blake wrote:
>> Noticed while reviewing another patch that had an accidental
>> mismatch due to refactoring.  An audit of the code showed that
>> very few callers of vshCommandOpt were expecting a return of
>> -2, indicating programmer error, and of those that DID check,
>> they just propagated that status to yet another caller that
>> did not check.  Fix this by making the code blatantly warn
>> the programmer, rather than silently ignoring it and possibly
>> doing the wrong thing downstream.
>> I know that we frown on assert()/abort() inside libvirtd
>> (libraries should NEVER kill the program that linked them),
>> but as virsh is an app rather than the library, and as this
>> is not the first use of assert() in virsh, I think this
>> approach is okay.
>> * tools/virsh.h (vshCommandOpt): Drop declaration.
>> * tools/virsh.c (vshCommandOpt): Make static, and add a
>> parameter.  Abort on programmer errors rather than making callers
>> repeat that logic.
>> (vshCommandOptInt, vshCommandOptUInt, vshCommandOptUL)
>> (vshCommandOptString, vshCommandOptStringReq)
>> (vshCommandOptLongLong, vshCommandOptULongLong)
>> (vshCommandOptBool): Adjust callers.
>> Signed-off-by: Eric Blake <eblake redhat com>
>> ---
>> In response to my observation on Don's email:
>> https://www.redhat.com/archives/libvir-list/2013-August/msg00769.html
>>  tools/virsh.c | 97 +++++++++++++++++++++--------------------------------------
>>  tools/virsh.h |  5 +--
>>  2 files changed, 35 insertions(+), 67 deletions(-)
> ACK and safe for freeze.

Thanks; pushed.

Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

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