On 03/26/2012 12:11 AM, Guannan Ren wrote:
*libvirt_virDomainGetCPUStats *setPyVirTypedParameter --- python/libvirt-override-api.xml | 4 +- python/libvirt-override.c | 62 +++++--------------------------------- 2 files changed, 11 insertions(+), 55 deletions(-)
Nice ratio for size reduction.
case VIR_TYPED_PARAM_INT: { - long long_val = PyInt_AsLong(value); - if ((long_val == -1) && PyErr_Occurred()) + if (libvirt_intUnwrap(value, &temp->value.i) < 0) goto cleanup; - if ((int)long_val == long_val) { - temp->value.i = long_val; - } else { - PyErr_Format(PyExc_ValueError, - "The value of " - "attribute \"%s\" is out of int range", keystr); - goto cleanup; - } }
Nit: now that you are no longer declaring a variable in each 'case label:', you can also get rid of the outermost {} after the case and before the break.
case VIR_TYPED_PARAM_BOOLEAN: { - /* Hack - Python's definition of Py_True breaks strict - * aliasing rules, so can't directly compare - */ - if (PyBool_Check(value)) { - PyObject *hacktrue = PyBool_FromLong(1); - temp->value.b = hacktrue == value ? 1 : 0; - Py_DECREF(hacktrue); - } else { - PyErr_Format(PyExc_TypeError, - "The value type of " - "attribute \"%s\" must be bool", keystr); + if (libvirt_boolUnwrap(value, (bool *)&temp->value.b) < 0)
This can break in strict C99 compilers, as it is not type-safe (we are _not_ guaranteed that char and bool are the same size). Here, you need a temporary variable: bool b; if (libvirt_boolUnwrap(value, &b) < 0) goto cleanup; temp->value.b = b; Also, there are a lot of other places where we have the 'hacktrue' or 'hackfalse' code; we should be fixing all of those call-sites to use the new libvirt_boolUnwrap() (although it's fine if you want to break the cleanup into multiple commits). -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org