[Cython] Problem with Py_buffer struct definition in Builtin.py

[Stefan Behnel]

mark florisson, 02.03.2012 17:33:
> On 2 March 2012 10:09, Stefan Behnel wrote:
>> Stefan Behnel, 02.03.2012 10:45:
>>> the builtin Py_buffer struct type is currently defined as follows:
>>>
>>> """
>>> builtin_structs_table = [
>>>     ('Py_buffer', 'Py_buffer',
>>>      [("buf",        PyrexTypes.c_void_ptr_type),
>>>       ("obj",        PyrexTypes.py_object_type),
>>>       ("len",        PyrexTypes.c_py_ssize_t_type),
>>>       ...
>>> """
>>>
>>> I hadn't noticed this before, but when you actually use it in a
>>> "__getbuffer__()" special method, you have to assign the buffer owner (i.e.
>>> self) to the .obj field, which is currently defined as "object".
>>
>> Oh, well, I should really learn to read code before composing a lengthy
>> e-mail...
>>
>> "__getbuffer__()" is already special-cased and sets the value to None. I
>> think I even recall that we discussed this back when Dag implemented
>> support for buffers. The reason I had originally noticed this was this
>> recent change in Py3.3:
>>
>> """
>> static PyObject *
>> _PyManagedBuffer_FromObject(PyObject *base)
>> {
>>    _PyManagedBufferObject *mbuf;
>>
>>    mbuf = mbuf_alloc();
>>    if (mbuf == NULL)
>>        return NULL;
>>
>>    if (PyObject_GetBuffer(base, &mbuf->master, PyBUF_FULL_RO) < 0) {
>>        /* mbuf->master.obj must be NULL. */
>>        Py_DECREF(mbuf);
>>        return NULL;
>>    }
>>
>>    /* Assume that master.obj is a new reference to base. */
>>    assert(mbuf->master.obj == base);

I asked on python-dev and I think this assert will be removed.

http://thread.gmane.org/gmane.comp.python.devel/130365

There is now a ticket to allow buffer redirection by "__getbuffer__()" as a
general feature.

http://bugs.python.org/issue14181


>>    return (PyObject *)mbuf;
>> }
>> """
>>
>> Note the assertion (which is unreleased as of now, so it may still be
>> subject to changes). Is there any reason the value should be set to None by
>> Cython's special casing code instead of self?
> 
> It sets it to None to it can later reset it to NULL. Python 3.3 is the
> first version to document the 'obj' Py_buffer attribute, and it
> mentions that the exporter may set it to NULL.

I don't find it very clear on that part, though. The way I read it, NULL is
only to be set on error (which Cython does already).


> The code above is not
> the PyObject_GetBuffer function, so setting it to NULL should still
> work. Defaulting it to 'self' instead would work equally well I'd
> think.

I'll wait and see what the discussion on python-dev brings up. If the
outcome is that NULL is for errors and that it should have a value
otherwise, I think 'self' is a better default.

Stefan