[libvirt] [python PATCH] Check return value of libvirt_uintUnwrap
Eric Blake
eblake at redhat.com
Thu Nov 6 11:14:43 UTC 2014
On 11/06/2014 12:04 PM, Pavel Hrdina wrote:
> On 11/06/2014 11:05 AM, Jiri Denemark wrote:
>> libvirt_virDomainSendKey didn't check whether libvirt_uintUnwrap
>> succeeded or not.
>>
>> https://bugzilla.redhat.com/show_bug.cgi?id=1161039
>> Signed-off-by: Jiri Denemark <jdenemar at redhat.com>
>> ---
>>
>> Providing extra context for easier review...
>>
>>
>> if (!PyArg_ParseTuple(args, (char *)"OiiOiI:virDomainSendKey",
>> &pyobj_domain, &codeset, &holdtime,
>> &pyobj_list,
>> &nkeycodes, &flags)) {
>> DEBUG("%s failed to parse tuple\n", __FUNCTION__);
>> return VIR_PY_INT_FAIL;
Pavel makes a good point. Returning VIR_PY_INT_FAIL (aka -1) implies
that we have set a stack-local libvirt error to be turned into a libvirt
exception. Returning NULL implies that we do not have a libvirt error,
but DO have a python error.
This existing code is wrong and should return NULL here.
A quick audit of libvirt-override.c found that at least
libvirt_virDomainGetJobStats(), libvirt_virConnectDomainEventRegister(),
libvirt_virEventRegisterImpl(), libvirt_virEventInvokeHandleCallback(),
and others also have a bug with this.
>> }
>> domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain);
>>
>> if (!PyList_Check(pyobj_list)) {
>> return VIR_PY_INT_FAIL;
>> }
This code is wrong and should return NULL.
>>
>> if (nkeycodes != PyList_Size(pyobj_list) ||
>> nkeycodes > VIR_DOMAIN_SEND_KEY_MAX_KEYS) {
>> return VIR_PY_INT_FAIL;
>> }
This code is wrong - it needs to raise a python error, then return NULL.
>>
>> for (i = 0; i < nkeycodes; i++) {
>> - libvirt_uintUnwrap(PyList_GetItem(pyobj_list, i),
>> &(keycodes[i]));
>> + if (libvirt_uintUnwrap(PyList_GetItem(pyobj_list, i),
>> &keycodes[i]) < 0)
>> + return VIR_PY_INT_FAIL;
>
> Return NULL instead of PyObject with value -1. Function
> "libvirt_uintUnwrap" sets an exception on error and in that case the
> NULL should be returned.
>
> ACK with that change,
As the mis-use of -1 instead of NULL is more pervasive than just this
function, we need a cleanup patch to cover all the wrong uses.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 539 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20141106/21f4c1ad/attachment-0001.sig>
More information about the libvir-list
mailing list