[libvirt] [PATCH] python: make virDomainSetSchedulerParameters accpet integer argument
When sending a Python integer as an argument to PyLong_AsUnsignedLong or PyLong_AsUnsignedLongLong, the following error occurs SystemError: Objects/longobject.c:980: bad argument to internal function This error comes from the fact that PyLong_AsUnsignedLong and PyLong_AsUnsignedLongLong, unlike PyLong_AsLong or PyLong_AsLongLong, does not check to see if the number is a long or integer, but only a long. The regression is introduced by 9c8466daac19379c41be39ec8f18db36c9573c02 >>> dom.setSchedulerParameters({'cpu_shares': 1024}) 0 dom.schedulerParameters() {'cpu_shares': 1024L} >>> dom.setSchedulerParameters({'cpu_shares': 1024L}) 0 >>> dom.schedulerParameters() {'cpu_shares': 1024L} --- python/libvirt-override.c | 9 ++++++++- 1 files changed, 8 insertions(+), 1 deletions(-) diff --git a/python/libvirt-override.c b/python/libvirt-override.c index 792cfa3..aec8f5a 100644 --- a/python/libvirt-override.c +++ b/python/libvirt-override.c @@ -233,7 +233,14 @@ setPyVirTypedParameter(PyObject *info, break; case VIR_TYPED_PARAM_ULLONG: { - unsigned long long ullong_val = PyLong_AsUnsignedLongLong(value); + unsigned long long ullong_val; + if (PyInt_Check(value)) + ullong_val = (unsigned long long)PyInt_AsLong(value); + else if (PyLong_Check(value)) + ullong_val = PyLong_AsUnsignedLongLong(value); + else + PyErr_SetString(PyExc_TypeError, "an integer is required"); + if ((ullong_val == -1) && PyErr_Occurred()) goto cleanup; temp->value.ul = ullong_val; -- 1.7.7.5
On 03/19/2012 11:45 AM, Guannan Ren wrote:
When sending a Python integer as an argument to PyLong_AsUnsignedLong or PyLong_AsUnsignedLongLong, the following error occurs
SystemError: Objects/longobject.c:980: bad argument to internal function
+++ b/python/libvirt-override.c @@ -233,7 +233,14 @@ setPyVirTypedParameter(PyObject *info, break; case VIR_TYPED_PARAM_ULLONG: { - unsigned long long ullong_val = PyLong_AsUnsignedLongLong(value); + unsigned long long ullong_val;
Pre-initialize this to -1; otherwise...
+ if (PyInt_Check(value)) + ullong_val = (unsigned long long)PyInt_AsLong(value);
Cast is unnecessary.
+ else if (PyLong_Check(value)) + ullong_val = PyLong_AsUnsignedLongLong(value); + else + PyErr_SetString(PyExc_TypeError, "an integer is required"); + if ((ullong_val == -1) && PyErr_Occurred())
...if I don't pass in PyInt or PyLong, then this error detection will be based on uninitialized memory. Why are you doing this for just VIR_TYPED_PARAM_ULLONG? I argue that we should be doing it for all of the integral conversions. Needs a v2. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 03/19/2012 12:03 PM, Eric Blake wrote:
Why are you doing this for just VIR_TYPED_PARAM_ULLONG? I argue that we should be doing it for all of the integral conversions.
In fact, I'd argue that we want new helper functions in typewrappers.c, as counterparts to our libvirt_intWrap() and friends. Perhaps: /* Return 0 if obj could be unwrapped into the corresponding C type, -1 with python exception set otherwise. */ int libvirt_intUnwrap(PyObject *obj, int *val); int libvirt_uintUnwrap(PyObject *obj, unsigned int *val); int libvirt_longlongUnwrap(PyObject *obj, long long *val); int libvirt_ulonglongUnwrap(PyObject *obj, unsigned long long *val); Then the code in question simplifies to: case VIR_TYPED_PARAM_ULLONG: { unsigned long long ullong_val; if (libvirt_ulonglongUnwrap(value, &ullong_val) < 0) goto cleanup; temp->value.ul = ullong_val; -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 03/20/2012 02:11 AM, Eric Blake wrote:
On 03/19/2012 12:03 PM, Eric Blake wrote:
Why are you doing this for just VIR_TYPED_PARAM_ULLONG? I argue that we should be doing it for all of the integral conversions. In fact, I'd argue that we want new helper functions in typewrappers.c, as counterparts to our libvirt_intWrap() and friends. Perhaps:
/* Return 0 if obj could be unwrapped into the corresponding C type, -1 with python exception set otherwise. */ int libvirt_intUnwrap(PyObject *obj, int *val); int libvirt_uintUnwrap(PyObject *obj, unsigned int *val); int libvirt_longlongUnwrap(PyObject *obj, long long *val);
About these three wrappers, we have implemented them in our code, the work here we gonna do is just to factor them from the codes to create wrappers for easier use in future.
int libvirt_ulonglongUnwrap(PyObject *obj, unsigned long long *val);
This wrapper will include the fix in it.
Then the code in question simplifies to:
case VIR_TYPED_PARAM_ULLONG: { unsigned long long ullong_val; if (libvirt_ulonglongUnwrap(value,&ullong_val)< 0) goto cleanup; temp->value.ul = ullong_val;
Okay, I will do it soon Guannan Ren
participants (2)
-
Eric Blake -
Guannan Ren