ndk/hardware_buffer: Don't call assume_init() before checking error (#426)

We were unconditionally passing an `assume_init()` value to
`status_to_io_result()`, which would only return that value back if
there was no error.  This is UB if the `MaybeUninit` was never written
to, which is typically the case when an error is returned.  Instead a
`.map(|()| ...assume_init())` should be used to ensure we only move the
`MaybeUninit` into an initialized Rust value when the error code says it
is okay to do so.

As this is the only place where a non-void (`()`) value was passed to
`status_to_io_result()`, the `value: T` argument has been removed in
favour of always returning `()` just like the
`BitmapError::from_status()` and `MediaError::from_status()` APIs.

Author: MarijnS95

ndk/src/hardware_buffer.rs

@@ -108,7 +108,7 @@ pub type Rect = ffi::ARect;
 fn construct<T>(with_ptr: impl FnOnce(*mut T) -> i32) -> Result<T> {
     let mut result = MaybeUninit::uninit();
     let status = with_ptr(result.as_mut_ptr());
-    status_to_io_result(status, unsafe { result.assume_init() })
+    status_to_io_result(status).map(|()| unsafe { result.assume_init() })
 }
 
 /// A native [`AHardwareBuffer *`]
@@ -317,7 +317,7 @@ impl HardwareBuffer {
                 bytes_per_stride.as_mut_ptr(),
             )
         };
-        status_to_io_result(status, ()).map(|()| unsafe {
+        status_to_io_result(status).map(|()| unsafe {
             LockedPlaneInfo {
                 virtual_address: virtual_address.assume_init(),
                 bytes_per_pixel: bytes_per_pixel.assume_init() as u32,
@@ -372,7 +372,7 @@ impl HardwareBuffer {
     /// a non-blocking variant that returns a file descriptor to be signaled on unlocking instead.
     pub fn unlock(&self) -> Result<()> {
         let status = unsafe { ffi::AHardwareBuffer_unlock(self.as_ptr(), std::ptr::null_mut()) };
-        status_to_io_result(status, ())
+        status_to_io_result(status)
     }
 
     /// Unlock the [`HardwareBuffer`] from direct CPU access.
@@ -413,7 +413,7 @@ impl HardwareBuffer {
         let status = unsafe {
             ffi::AHardwareBuffer_sendHandleToUnixSocket(self.as_ptr(), socket_fd.as_raw_fd())
         };
-        status_to_io_result(status, ())
+        status_to_io_result(status)
     }
 
     /// Acquire a reference on the given [`HardwareBuffer`] object.

ndk/src/input_queue.rs

@@ -45,7 +45,7 @@ impl InputQueue {
     pub fn get_event(&self) -> Result<Option<InputEvent>> {
         let mut out_event = ptr::null_mut();
         let status = unsafe { ffi::AInputQueue_getEvent(self.ptr.as_ptr(), &mut out_event) };
-        match status_to_io_result(status, ()) {
+        match status_to_io_result(status) {
             Ok(()) => {
                 debug_assert!(!out_event.is_null());
                 Ok(Some(unsafe {

ndk/src/native_window.rs

@@ -103,7 +103,7 @@ impl NativeWindow {
         let status = unsafe {
             ffi::ANativeWindow_setBuffersGeometry(self.ptr.as_ptr(), width, height, format as i32)
         };
-        status_to_io_result(status, ())
+        status_to_io_result(status)
     }
 
     /// Return the [`NativeWindow`] associated with a JNI [`android.view.Surface`] pointer.
@@ -139,10 +139,10 @@ impl NativeWindow {
             None => std::ptr::null_mut(),
         };
         let mut buffer = MaybeUninit::uninit();
-        let ret = unsafe {
+        let status = unsafe {
             ffi::ANativeWindow_lock(self.ptr.as_ptr(), buffer.as_mut_ptr(), dirty_bounds)
         };
-        status_to_io_result(ret, ())?;
+        status_to_io_result(status)?;
 
         Ok(NativeWindowBufferLockGuard {
             window: self,

ndk/src/surface_texture.rs

@@ -84,7 +84,7 @@ impl SurfaceTexture {
     /// context at a time.
     pub fn attach_to_gl_context(&self, tex_name: u32) -> Result<()> {
         let status = unsafe { ffi::ASurfaceTexture_attachToGLContext(self.ptr.as_ptr(), tex_name) };
-        status_to_io_result(status, ())
+        status_to_io_result(status)
     }
 
     /// Detach the [`SurfaceTexture`] from the OpenGL ES context that owns the OpenGL ES texture
@@ -100,7 +100,7 @@ impl SurfaceTexture {
     /// context at a time.
     pub fn detach_from_gl_context(&self) -> Result<()> {
         let status = unsafe { ffi::ASurfaceTexture_detachFromGLContext(self.ptr.as_ptr()) };
-        status_to_io_result(status, ())
+        status_to_io_result(status)
     }
 
     /// Retrieve the 4x4 texture coordinate transform matrix associated with the texture image set
@@ -154,6 +154,6 @@ impl SurfaceTexture {
     /// texture target.
     pub fn update_tex_image(&self) -> Result<()> {
         let status = unsafe { ffi::ASurfaceTexture_updateTexImage(self.ptr.as_ptr()) };
-        status_to_io_result(status, ())
+        status_to_io_result(status)
     }
 }

ndk/src/utils.rs

@@ -7,9 +7,9 @@ use std::io::{Error, Result};
 /// Rust's [`std::io::Error`].
 ///
 /// [`Errors.h`]: https://cs.android.com/android/platform/superproject/+/master:system/core/libutils/include/utils/Errors.h
-pub(crate) fn status_to_io_result<T>(status: i32, value: T) -> Result<T> {
+pub(crate) fn status_to_io_result(status: i32) -> Result<()> {
     match status {
-        0 => Ok(value),
+        0 => Ok(()),
         r if r < 0 => Err(Error::from_raw_os_error(-r)),
         r => unreachable!("Status is positive integer {}", r),
     }