Re: [libvirt] [PATCH v2] python: add virDomainGetCPUStats python binding API

On 03/18/2012 04:41 AM, Guannan Ren wrote: >>> + if (totalbool == Py_False) { >> Per other code in libvirt-override.c, you can't compare totalbool (type >> PyObject) with Py_False, at least not on all compilers. You need >> something like this instead: >> >> /* 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); > Yes, it did report warning in compiling as follows due to the case > from PyIntObject* to PyObject* > warning :dereferencing type-punned pointer might break > strict-aliasing rules [-Wstrict-aliasing] And that would trip up a -Werror compilation, so I'm glad to see you changed it in v3. > GCC command line to reproduce the error: > gcc -Wstrict-aliasing=1 -O2 cpythonexample.c > > Actually PyObject_IsTrue() is a more light-weight approach to do > the checking instead of > creating a intermediate PyObject * for the compare. Is PyObject_IsTrue() available in the version of python present on RHEL 5? If so, I'd be in favor of a followup cleanup patch that removes all our hacks in favor of the python glue code that does the same thing. And even if not, we should write a decent wrapper in our own typewrappers.c, so that the rest of our code doesn't have to look so ugly with so much copy-and-paste.