[libvirt] [PATCH 2/2] python: make python APIs use these helper functions

[Eric Blake]

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 at redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 620 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20120326/33fde973/attachment-0001.sig>