Re: [libvirt] [python PATCH] Check return value of libvirt_uintUnwrap
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(a)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
Attachments:
- signature.asc (application/pgp-signature — 539 bytes)