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

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

 



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@xxxxxxxxxx>
>> ---
>>
>> 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

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]