Antoine Pitrou <pit...@free.fr> added the comment: > On an eyeball review, the structure of do_release seems a little > questionable to me. I'd be happier if view.obj and view.buf were > copied to C locals and then set to NULL at the start of the function > before any real work is done.
You can't do that, since PyBuffer_Release() needs the information (at least `obj`, and then the underlying object might rely on `buf` in its bf_releasebuffer method). (note, this code is simply factored out from tp_dealloc) > For the test code, I suggest including: > - a test where release() is called inside the cm to check that > __exit__ does not raise ValueError. > - check getbuffer() correctly raises ValueError > - tests that the other properties correctly raise ValueError (ndim, > shape, strides, format, itemsize, suboffsets) Ok. I was bit too lazy and didn't include all the API. > (For the properties, perhaps using a list of strings, a loop and > getattr rather than writing each one out separately) It's more clever but less practical if there's a failure: you can't know up front which property failed the test. > _check_released should perhaps also take a second argument that is > used for the last two NotEqual checks. That is: Hmm, why not. > There's also a minor typo in a comment in the tests: "called at > second" should be "called a second". Ah, thanks. > One other question: should IS_RELEASED use "||" rather than "&&"? > > Is there any case where either of those pointers can be NULL and we > still want to continue on rather than bailing out with a ValueError? Well, view.obj can be NULL in case the buffer has been filled by hand by some C code which simply wants to export a lazy view to some chunk of memory (the IO lib uses this). I could simply test for view.buf. I was thinking that maybe NULL could be a proper data pointer on some platforms, but that would probably be crazy (since using NULL as "non initialized" or "failed doing something" is so common in C code). ---------- _______________________________________ Python tracker <rep...@bugs.python.org> <http://bugs.python.org/issue9757> _______________________________________ _______________________________________________ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
