Conversation
kngwyu
left a comment
kngwyu
left a comment
There was a problem hiding this comment.
Thanks, looks reasonable for me.
| unsafe { | ||
| let mut buf = Box::pin(ffi::Py_buffer::new()); | ||
| // TODO: use nightly API Box::new_uninit() once stable | ||
| let mut buf = Box::new(mem::MaybeUninit::uninit()); |
There was a problem hiding this comment.
If you take this approach, then you can remove Py_buffer::new().
Though I'm a bit doubtful about how effective MaybeUninit is (I mean that this is effective only when we raise an error, but then buf is immediately dropped...).
There was a problem hiding this comment.
In the future I would like to remove Py_buffer::new(), but I think to do so right now would be inconvenient for users because PyBufferProtocol basically forces you to handle the raw ffi pointer (which probably points to uninitialized memory which C created).
I chose MaybeUninit here because that's pretty much identical to what C would do: we create a slab of memory and pass its location to PyObject_GetBuffer to initialize the contents.
davidhewitt
force-pushed
the
buffer-errors
branch
from
d2fc2fc to
71dd405
Compare
|
Could you please add some tests where construction fails? |
davidhewitt
force-pushed
the
buffer-errors
branch
3 times, most recently
from
dc7c2fc to
aa0b5d8
Compare
2 participants
Motivated by #1128 (comment)