| msg154969 - (view) |
Author: Stefan Krah (skrah) *  |
Date: 2012-03-05 18:32 |
| bytearray_getbuffer() checks for view==NULL. But this has never been allowed: PEP: "The second argument is the address to a bufferinfo structure. Both arguments must never be NULL." DOCS (3.2): "view must point to an existing Py_buffer structure allocated by the caller". A quick grep through the source tree shows no instances where the middle argument of either PyObject_GetBuffer of bf_getbuffer is NULL or 0. Patch attached, all tests pass. I wouldn't be comfortable to commit it without review though (it's just too strange). BTW, the next conditional in bytearray_getbuffer ... if (ret >= 0) { obj->ob_exports++; } is also superfluous, since PyBuffer_FillInfo() cannot fail if readonly==0. |
|
|
| msg154971 - (view) |
Author: Stefan Krah (skrah) * |
Date: 2012-03-05 18:56 |
| array_buffer_getbuf does a similar thing: if (view==NULL) goto finish; finish: self->ob_exports++; // ??? return 0 Where does this view==NULL thing come from? |
|
|
| msg154972 - (view) |
Author: Stefan Krah (skrah) * |
Date: 2012-03-05 19:25 |
| I seems to be a feature to get a "lock" on an exporter without the exporter filling in a buffer. It was there from the beginning: http://svn.python.org/view/python/branches/release30-maint/Objects/bytearrayobject.c?r1=56849&r2=57181 Last use that I can see is here: http://hg.python.org/cpython/file/df3b2b5db900/Modules/posixmodule.c#l561 |
|
|
| msg219995 - (view) |
Author: Kelley Nielsen (kelleynnn) |
Date: 2014-06-07 22:51 |
| I have verified that this feature is unused in the source tree; in fact, there are no internal calls to bytearray_getbuffer() at all. The only thing bytearray_getbuffer() does with its second arg is pass it to PyBuffer_FillInfo(), which immediately checks it and passes 0 up the call stack if it is NULL. (A comment in PyBuffer_FillInfo() asks why -1 is not passed up instead; it's probably to distinguish this feature from the error condition handled in the immediately following conditional block.) There are potentially other issues stemming from this legacy feature in bytearray_getbuffer(), PyBuffer_FillInfo(), and elsewhere. The maintainers may see fit to open tickets on these issues as well. There's more relevant commentary on the feature here: http://comments.gmane.org/gmane.comp.python.devel/130521 |
|
|
| msg235346 - (view) |
Author: Stefan Krah (skrah) * |
Date: 2015-02-03 15:14 |
| New patch with tests. |
|
|
| msg235348 - (view) |
Author: Roundup Robot (python-dev)  |
Date: 2015-02-03 15:57 |
| New changeset e8fe32d43c96 by Stefan Krah in branch 'default': Issue #14203: Remove obsolete support for view==NULL in PyBuffer_FillInfo() https://hg.python.org/cpython/rev/e8fe32d43c96 |
|
|
| msg235352 - (view) |
Author: Stefan Krah (skrah) * |
Date: 2015-02-03 16:35 |
| bytesiobuf_getbuffer() also still has this obsolete feature, so BufferError should be raised if view==NULL. I'm unsure if this plays well with the new SHARED_BUF(b) thing. Should the exception be raised before or after? |
|
|
| msg235354 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2015-02-03 17:05 |
| I think before. It doesn't harm, but it doesn't make much sense to unshare the buffer if its address is not used. |
|
|
| msg235355 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2015-02-03 17:11 |
| E.g. the buffer should be unshared before incrementing b->exports, but if an exception is raised instead, there is no need to unshare the buffer before raising. |
|
|
| msg235358 - (view) |
Author: Stefan Krah (skrah) * |
Date: 2015-02-03 20:17 |
| Ok, thanks! -3.diff should be correct, then. |
|
|
| msg235359 - (view) |
Author: Roundup Robot (python-dev) |
Date: 2015-02-03 20:43 |
| New changeset 0d5095a2422f by Stefan Krah in branch 'default': Issue #14203: Remove obsolete support for view==NULL in bytesiobuf_getbuffer() https://hg.python.org/cpython/rev/0d5095a2422f |
|
|
| msg235360 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2015-02-03 20:57 |
| Compiling failed on Windows: http://buildbot.python.org/all/builders/AMD64%20Windows7%20SP1%203.x/builds/5650 |
|
|
| msg235361 - (view) |
Author: Stefan Krah (skrah) * |
Date: 2015-02-03 21:02 |
| Argh, the extern _PyBytesIOBuffer_Type hack. I knew it would cause some trouble. Any ideas how to test bytesiobuf_getbuffer() with a NULL view in a more dignified way? |
|
|
| msg235363 - (view) |
Author: Roundup Robot (python-dev) |
Date: 2015-02-03 21:27 |
| New changeset 17a8c5f8ca48 by Stefan Krah in branch 'default': Issue #14203: Temporary fix for the compile failure on Windows. https://hg.python.org/cpython/rev/17a8c5f8ca48 |
|
|
| msg235386 - (view) |
Author: Stefan Krah (skrah) * |
Date: 2015-02-04 13:51 |
| I think it's sufficient to test bytesiobuf_getbuffer() on Linux and FreeBSD. The test just checks that the exception is raised. |
|
|