Implement Deref<Target=[T]> for AutoArray and AutoPrimitiveArray

This makes it possible to read and write the elements of a Java array
without resorting to unsafe code.

Both AutoArray and AutoPrimitiveArray now need to be constructed with
a known array length (::new() will query the length and an unsafe
::new_with_len() API could be used in case the length was already known)

The `.size()` API has been replace with `.len()` and `.is_empty()` and
`.len()` doesn't return a Result since it's no longer possible to
create an `Auto[Primitive]Array` without a known length.

src/wrapper/jnienv.rs

@@ -306,7 +306,7 @@ impl<'local> JNIEnv<'local> {
             name.as_ptr(),
             loader.as_raw(),
             buf.as_ptr(),
-            buf.size()?
+            buf.len() as _
         );
         Ok(unsafe { JClass::from_raw(class) })
     }

src/wrapper/objects/auto_array.rs

@@ -1,7 +1,7 @@
 use log::error;
 use std::ptr::NonNull;
 
-use crate::sys::{jboolean, jbyte, jchar, jdouble, jfloat, jint, jlong, jshort, jsize};
+use crate::sys::{jboolean, jbyte, jchar, jdouble, jfloat, jint, jlong, jshort};
 use crate::wrapper::objects::ReleaseMode;
 use crate::{errors::*, sys, JNIEnv};
 
@@ -134,16 +134,21 @@ impl TypeArray for jdouble {}
 /// - `'ptr` represents the lifetime of the acquired pointer to the `array` elements.
 pub struct AutoArray<'local, 'other_local, 'array, T: TypeArray> {
     array: &'array JPrimitiveArray<'other_local, T>,
+    len: usize,
     ptr: NonNull<T>,
     mode: ReleaseMode,
     is_copy: bool,
     env: JNIEnv<'local>,
 }
 
 impl<'local, 'other_local, 'array, T: TypeArray> AutoArray<'local, 'other_local, 'array, T> {
-    pub(crate) fn new(
+    /// # Safety
+    ///
+    /// `len` must be the correct length (number of elements) of the given `array`
+    pub(crate) unsafe fn new_with_len(
         env: &mut JNIEnv<'local>,
         array: &'array JPrimitiveArray<'other_local, T>,
+        len: usize,
         mode: ReleaseMode,
     ) -> Result<Self> {
         // Safety: The cloned `JNIEnv` will not be used to create any local references. It will be
@@ -155,13 +160,23 @@ impl<'local, 'other_local, 'array, T: TypeArray> AutoArray<'local, 'other_local,
         let ptr = unsafe { T::get(&mut env, array.as_raw(), &mut is_copy) }?;
         Ok(AutoArray {
             array,
+            len,
             ptr: NonNull::new(ptr).ok_or(Error::NullPtr("Non-null ptr expected"))?,
             mode,
             is_copy: is_copy == sys::JNI_TRUE,
             env,
         })
     }
 
+    pub(crate) fn new(
+        env: &mut JNIEnv<'local>,
+        array: &'array JPrimitiveArray<'other_local, T>,
+        mode: ReleaseMode,
+    ) -> Result<Self> {
+        let len = env.get_array_length(array.as_raw())? as usize;
+        unsafe { Self::new_with_len(env, array, len, mode) }
+    }
+
     /// Get a reference to the wrapped pointer
     pub fn as_ptr(&self) -> *mut T {
         self.ptr.as_ptr()
@@ -199,9 +214,14 @@ impl<'local, 'other_local, 'array, T: TypeArray> AutoArray<'local, 'other_local,
         self.is_copy
     }
 
-    /// Returns the array size
-    pub fn size(&self) -> Result<jsize> {
-        self.env.get_array_length(self.array.as_raw())
+    /// Returns the array length (number of elements)
+    pub fn len(&self) -> usize {
+        self.len
+    }
+
+    /// Returns true if the vector contains no elements.
+    pub fn is_empty(&self) -> bool {
+        self.len == 0
     }
 }
 
@@ -234,3 +254,21 @@ impl<'local, 'other_local, 'array, T: TypeArray> From<&AutoArray<'local, 'other_
         other.as_ptr()
     }
 }
+
+impl<'local, 'other_local, 'array, T: TypeArray> std::ops::Deref
+    for AutoArray<'local, 'other_local, 'array, T>
+{
+    type Target = [T];
+
+    fn deref(&self) -> &Self::Target {
+        unsafe { std::slice::from_raw_parts(self.ptr.as_ptr(), self.len) }
+    }
+}
+
+impl<'local, 'other_local, 'array, T: TypeArray> std::ops::DerefMut
+    for AutoArray<'local, 'other_local, 'array, T>
+{
+    fn deref_mut(&mut self) -> &mut Self::Target {
+        unsafe { std::slice::from_raw_parts_mut(self.ptr.as_mut(), self.len) }
+    }
+}

src/wrapper/objects/auto_primitive_array.rs

@@ -1,7 +1,7 @@
 use log::error;
 use std::ptr::NonNull;
 
-use crate::sys::{jboolean, jsize};
+use crate::sys::jboolean;
 use crate::wrapper::objects::ReleaseMode;
 use crate::{errors::*, sys, JNIEnv};
 
@@ -28,6 +28,7 @@ use super::JByteArray;
 /// - `'ptr` represents the lifetime of the acquired pointer to the `array` elements.
 pub struct AutoPrimitiveArray<'local, 'other_local, 'array, 'env, T: TypeArray> {
     array: &'array JPrimitiveArray<'other_local, T>,
+    len: usize,
     ptr: NonNull<T>,
     mode: ReleaseMode,
     is_copy: bool,
@@ -37,9 +38,13 @@ pub struct AutoPrimitiveArray<'local, 'other_local, 'array, 'env, T: TypeArray>
 impl<'local, 'other_local, 'array, 'env, T: TypeArray>
     AutoPrimitiveArray<'local, 'other_local, 'array, 'env, T>
 {
-    pub(crate) fn new(
+    /// # Safety
+    ///
+    /// `len` must be the correct length (number of elements) of the given `array`
+    pub(crate) unsafe fn new_with_len(
         env: &'env mut JNIEnv<'local>,
         array: &'array JPrimitiveArray<'other_local, T>,
+        len: usize,
         mode: ReleaseMode,
     ) -> Result<Self> {
         let mut is_copy: jboolean = 0xff;
@@ -57,13 +62,23 @@ impl<'local, 'other_local, 'array, 'env, T: TypeArray>
 
         Ok(AutoPrimitiveArray {
             array,
+            len,
             ptr: NonNull::new(ptr).ok_or(Error::NullPtr("Non-null ptr expected"))?,
             mode,
             is_copy: is_copy == sys::JNI_TRUE,
             env,
         })
     }
 
+    pub(crate) fn new(
+        env: &'env mut JNIEnv<'local>,
+        array: &'array JPrimitiveArray<'other_local, T>,
+        mode: ReleaseMode,
+    ) -> Result<Self> {
+        let len = env.get_array_length(array.as_raw())? as usize;
+        unsafe { Self::new_with_len(env, array, len, mode) }
+    }
+
     /// Get a reference to the wrapped pointer
     pub fn as_ptr(&self) -> *mut T {
         self.ptr.as_ptr()
@@ -103,9 +118,14 @@ impl<'local, 'other_local, 'array, 'env, T: TypeArray>
         self.is_copy
     }
 
-    /// Returns the array size
-    pub fn size(&self) -> Result<jsize> {
-        self.env.get_array_length(self.array.as_raw())
+    /// Returns the array length (number of elements)
+    pub fn len(&self) -> usize {
+        self.len
+    }
+
+    /// Returns true if the vector contains no elements.
+    pub fn is_empty(&self) -> bool {
+        self.len == 0
     }
 }
 
@@ -139,3 +159,21 @@ impl<'local, 'other_local, 'array, 'env, T: TypeArray>
         other.as_ptr()
     }
 }
+
+impl<'local, 'other_local, 'array, 'env, T: TypeArray> std::ops::Deref
+    for AutoPrimitiveArray<'local, 'other_local, 'array, 'env, T>
+{
+    type Target = [T];
+
+    fn deref(&self) -> &Self::Target {
+        unsafe { std::slice::from_raw_parts(self.ptr.as_ptr(), self.len) }
+    }
+}
+
+impl<'local, 'other_local, 'array, 'env, T: TypeArray> std::ops::DerefMut
+    for AutoPrimitiveArray<'local, 'other_local, 'array, 'env, T>
+{
+    fn deref_mut(&mut self) -> &mut Self::Target {
+        unsafe { std::slice::from_raw_parts_mut(self.ptr.as_mut(), self.len) }
+    }
+}

tests/jni_api.rs

@@ -472,7 +472,7 @@ pub fn java_byte_array_from_slice() {
     assert_eq!(res[2], 3);
 }
 
-macro_rules! test_get_array_elements {
+macro_rules! test_auto_array_read_write {
     ( $test_name:tt, $jni_type:ty, $new_array:tt, $get_array:tt, $set_array:tt ) => {
         #[test]
         pub fn $test_name() {
@@ -490,15 +490,15 @@ macro_rules! test_get_array_elements {
             // Use a scope to test Drop
             {
                 // Get byte array elements auto wrapper
-                let auto_ptr: AutoArray<$jni_type> = {
+                let mut auto_ptr: AutoArray<$jni_type> = {
                     // Make sure the lifetime is tied to the environment,
                     // not the particular JNIEnv reference
                     let mut temporary_env: JNIEnv = unsafe { env.unsafe_clone() };
                     temporary_env.get_array_elements(&java_array, ReleaseMode::CopyBack).unwrap()
                 };
 
                 // Check array size
-                assert_eq!(auto_ptr.size().unwrap(), 2);
+                assert_eq!(auto_ptr.len(), 2);
 
                 // Check pointer access
                 let ptr = auto_ptr.as_ptr();
@@ -515,12 +515,27 @@ macro_rules! test_get_array_elements {
                 assert_eq!(unsafe { *ptr.offset(0) } as i32, 0);
                 assert_eq!(unsafe { *ptr.offset(1) } as i32, 1);
 
-                // Modify
+                // Check slice access
+                //
+                // # Safety
+                //
+                // We make sure that the slice is dropped before also testing access via `Deref`
+                // (to ensure we don't have aliased references)
                 unsafe {
-                    *ptr.offset(0) += 1 as $jni_type;
-                    *ptr.offset(1) -= 1 as $jni_type;
+                    let slice = std::slice::from_raw_parts(auto_ptr.as_ptr(), auto_ptr.len());
+                    assert_eq!(slice[0] as i32, 0);
+                    assert_eq!(slice[1] as i32, 1);
                 }
 
+                // Check access via Deref
+                assert_eq!(auto_ptr[0] as i32, 0);
+                assert_eq!(auto_ptr[1] as i32, 1);
+
+                // Modify via DerefMut
+                let tmp = auto_ptr[1];
+                auto_ptr[1] = auto_ptr[0];
+                auto_ptr[0] = tmp;
+
                 // Commit would be necessary here, if there were no closure
                 //auto_ptr.commit().unwrap();
             }
@@ -535,7 +550,7 @@ macro_rules! test_get_array_elements {
 }
 
 // Test generic get_array_elements
-test_get_array_elements!(
+test_auto_array_read_write!(
     get_array_elements,
     jint,
     new_int_array,
@@ -544,63 +559,63 @@ test_get_array_elements!(
 );
 
 // Test type-specific array accessors
-test_get_array_elements!(
+test_auto_array_read_write!(
     get_int_array_elements,
     jint,
     new_int_array,
     get_int_array_region,
     set_int_array_region
 );
 
-test_get_array_elements!(
+test_auto_array_read_write!(
     get_long_array_elements,
     jlong,
     new_long_array,
     get_long_array_region,
     set_long_array_region
 );
 
-test_get_array_elements!(
+test_auto_array_read_write!(
     get_byte_array_elements,
     jbyte,
     new_byte_array,
     get_byte_array_region,
     set_byte_array_region
 );
 
-test_get_array_elements!(
+test_auto_array_read_write!(
     get_boolean_array_elements,
     jboolean,
     new_boolean_array,
     get_boolean_array_region,
     set_boolean_array_region
 );
 
-test_get_array_elements!(
+test_auto_array_read_write!(
     get_char_array_elements,
     jchar,
     new_char_array,
     get_char_array_region,
     set_char_array_region
 );
 
-test_get_array_elements!(
+test_auto_array_read_write!(
     get_short_array_elements,
     jshort,
     new_short_array,
     get_short_array_region,
     set_short_array_region
 );
 
-test_get_array_elements!(
+test_auto_array_read_write!(
     get_float_array_elements,
     jfloat,
     new_float_array,
     get_float_array_region,
     set_float_array_region
 );
 
-test_get_array_elements!(
+test_auto_array_read_write!(
     get_double_array_elements,
     jdouble,
     new_double_array,
@@ -672,34 +687,35 @@ pub fn get_primitive_array_critical() {
     // Use a scope to test Drop
     {
         // Get primitive array elements auto wrapper
-        let auto_ptr = env
+        let mut auto_ptr = env
             .get_primitive_array_critical(&java_array, ReleaseMode::CopyBack)
             .unwrap();
 
         // Check array size
-        assert_eq!(auto_ptr.size().unwrap(), 3);
-
-        // Get pointer
-        let ptr = auto_ptr.as_ptr();
-
-        // Convert void pointer to an unsigned byte array, without copy
-        let mut vec;
-        unsafe { vec = Vec::from_raw_parts(ptr as *mut u8, 3, 3) }
-
-        // Check
-        assert_eq!(vec[0], 1);
-        assert_eq!(vec[1], 2);
-        assert_eq!(vec[2], 3);
+        assert_eq!(auto_ptr.len(), 3);
+
+        // Convert void pointer to a &[i8] slice, without copy
+        //
+        // # Safety
+        //
+        // We make sure that the slice is dropped before also testing access via `Deref`
+        // (to ensure we don't have aliased references)
+        unsafe {
+            let slice = std::slice::from_raw_parts(auto_ptr.as_ptr(), auto_ptr.len());
+            assert_eq!(slice[0], 1);
+            assert_eq!(slice[1], 2);
+            assert_eq!(slice[2], 3);
+        }
 
-        // Modify
-        vec[0] += 1;
-        vec[1] += 1;
-        vec[2] += 1;
+        // Also check access via `Deref`
+        assert_eq!(auto_ptr[0], 1);
+        assert_eq!(auto_ptr[1], 2);
+        assert_eq!(auto_ptr[2], 3);
 
-        // Release
-        // Make sure vec's destructor doesn't free the data it thinks it owns when it goes out
-        // of scope (avoid double free)
-        std::mem::forget(vec);
+        // Modify via `DerefMut`
+        auto_ptr[0] += 1;
+        auto_ptr[1] += 1;
+        auto_ptr[2] += 1;
     }
 
     // Confirm modification of original Java array