> 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).