Re: [libvirt] [PATCH 2/2] python: make python APIs use these helper functions
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(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachments:
- signature.asc (application/pgp-signature — 620 bytes)