[PySide] Bug with new hash table feature

[Nathan Smith]

Hi Hugo,

Thanks for getting back to me.  I thought that was the case, and the
PyErr_format function is called within the isValid(PyObject *pyObj)
function, but no exception is ever raised in Python.  The following is
called from within the body of isValid(PyObject *pyObj):

PyErr_Format(PyExc_RuntimeError, "Internal C++ object (%s) already
deleted.", pyObj->ob_type->tp_name);


My modified hash function *always* returns even when called on invalid
objects.  The updated code is shown below for reference:

Py_hash_t hash(PyObject* pyObj)
{
    assert(Shiboken::Object::checkType(pyObj));
    // begin changes by Nathan
    if (!isValid(pyObj)) {
        return 0L;
    }
    // end changes by Nathan
    return
reinterpret_cast<Py_hash_t>(reinterpret_cast<SbkObject*>(pyObj)->d->cptr[0]);
}

// Unchanged
bool isValid(PyObject* pyObj)
{
    if (!pyObj || pyObj == Py_None
        || Py_TYPE(pyObj->ob_type) != &SbkObjectType_Type) {
        return true;
    }

    SbkObjectPrivate* priv = reinterpret_cast<SbkObject*>(pyObj)->d;

    if (!priv->cppObjectCreated && isUserType(pyObj)) {
        PyErr_Format(PyExc_RuntimeError, "'__init__' method of object's
base class (%s) not called.", pyObj->ob_type->tp_name);
        return false;
    }

    if (!priv->validCppObject) {
        PyErr_Format(PyExc_RuntimeError, "Internal C++ object (%s) already
deleted.", pyObj->ob_type->tp_name);
        return false;
    }

    return true;
}

When passed an invalid object, this function just returns 0 rather than
raising a RuntimeError.  Why isn't a runtime error raised?

Nathan

On Wed, May 30, 2012 at 1:12 PM, Hugo Parente Lima
<hugo.lima at openbossa.org>wrote:

> On Wednesday, May 30, 2012 09:37:15 AM Nathan Smith wrote:
> > Hi all,
> >
> > I uncovered a bug in the new hash feature of Shiboken in the
> repositories.
> >  I'll submit a bug report when the system comes back up, but I'm
> interested
> > in actually submitting a patch.
> >
> > The new hash function doesn't check to see if the provided object is
> valid
> > before calculating the hash value.  This can be seen
> > in shiboken/libshiboken/basewrapper.cpp at line 754 and demonstrated
> using
> >
> > the following code:
> > >>> from PySide import QtCore
> > >>> import shiboken
> > >>> a = QtCore.QObject()
> > >>> b = QtCore.QObject(a)
> > >>> shiboken.isValid(a)
> >
> > True
> >
> > >>> shiboken.isValid(b)
> >
> > True
> >
> > >>> del a
> > >>> shiboken.isValid(b)
> >
> > False
> >
> > >>> hash(b)
> >
> > Segmentation Fault
> >
> >
> > I made the following change in the code, but it didn't do what I
> expected:
> >
> > if (!isValid(pyObj)) {
> >   return 0L;
> > }
> >
> >
> > isValid raises a RuntimeException in addition to returning false, but I
> > never see that exception in Python.  So, I have two questions:
> >
> >    1. Is there some other code that should be checking isValid before the
> >    call ever gets to the C++ hash() function?
> >    2. How do you correctly raise an exception from within Shiboken?
>
> You raise an exception using the Python C API.
>
> Use the PyErr_* functions, e.g. PyErr_SetString
>
> > Nathan
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.qt-project.org/pipermail/pyside/attachments/20120530/efd97a2b/attachment.html>