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@xxxxxxxxxx> >> --- >> >> 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
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list