On 03/20/2012 01:36 AM, Eric Blake wrote:
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.
On RHEL5.7 Python 2.4.3, the PyObject_IsTrue() is available to use.
so, the original comparing from
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);
goto cleanup;
}
will become:
if (PyBool_Check(value)) {
temp->value.b = PyObject_IsTrue(value) ? 1 : 0;
} else {
PyErr_Format(PyExc_TypeError,
"The value type of "
"attribute \"%s\" must be bool", keystr);
goto cleanup;
}
Guannan Ren
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list