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

Reply via email to