tustvold commented on code in PR #5082: URL: https://github.com/apache/arrow-rs/pull/5082#discussion_r1395589789
########## arrow-data/src/ffi.rs: ########## @@ -191,6 +191,22 @@ impl FFI_ArrowArray { } } + /// Takes ownership of the pointed to [`FFI_ArrowArray`] + /// + /// This acts to [move] the data out of `array`, setting the release callback to NULL + /// + /// # Safety + /// + /// * `array` must be [valid] for reads and writes + /// * `array` must be properly aligned + /// * `array` must point to a properly initialized value of [`FFI_ArrowArray`] + /// + /// [move]: https://arrow.apache.org/docs/format/CDataInterface.html#moving-an-array + /// [valid]: https://doc.rust-lang.org/std/ptr/index.html#safety + pub unsafe fn from_raw(array: *mut FFI_ArrowArray) -> Self { + std::ptr::replace(array, Self::empty()) Review Comment: Something that occurs to me is we don't verify that release isn't NULL, as it can't be an invariant that `FFI_ArrowArray` doesn't have a null release callback for obvious reasons, but then the accessors assume the data is valid. I think we should probably be also asserting that `FFI_ArrowArray` hasn't been released? The good news is I don't think you can hit this with safe code, but it is potentially interesting -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org