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@xxxxxxxxxx +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