Add safety comment and error handling

mupdf-sys/src/lib.rs

@@ -5,12 +5,60 @@
 
 include!(concat!(env!("OUT_DIR"), "/bindings.rs"));
 
+use core::{
+    ffi::{c_int, CStr},
+    ptr,
+};
+
+/// This function allocates a new device and returns a pointer to it if no error occured. For the
+/// required structure of `T` check the example below.
+///
+/// The pointer `errptr` points to is expected to be null at the time this function is called.
+/// If an error occurs this inner pointer will be set to the error and null returned from this
+/// function. Was this inner pointer non-null before this function was called a new device will be
+/// allocated but the function will assume that an error has occured and will return null. This device
+/// will not be freed. It will, however, not cause unsafe behavior either.
+///
+/// # Safety
+///
+/// The caller must ensure `ctx` and `errptr` to be a valid pointers.
+///
+/// It must also ensure `T` to be a type that starts with `fz_device`. Memory will be allocated for
+/// a new instance of `T`, but only the `fz_device` portion will be initialized. The rest is
+/// currently being zero-initialized, but this might change in the future.
+///
+/// # Example
+///
+/// This is how a compliant `T` might look like. The `repr(C)` is necessary as `repr(Rust)` does
+/// not guarantee stable field orderings.
+///
+/// ```rust
+/// use mupdf_sys::fz_device;
+///
+/// #[repr(C)]
+/// struct MyDevice {
+///     base: fz_device,
+///     foo: u32,
+/// }
+/// ```
 pub unsafe fn mupdf_new_derived_device<T>(
     ctx: *mut fz_context,
-    label: *const std::ffi::c_char,
+    label: &'static CStr,
+    errptr: *mut *mut mupdf_error_t,
 ) -> *mut T {
-    let size = std::ffi::c_int::try_from(size_of::<T>()).unwrap();
-    let device = fz_new_device_of_size(ctx, size);
-    let label = Memento_label(device.cast(), label);
+    let SIZE: c_int = const {
+        if (c_int::MAX as usize) < size_of::<T>() {
+            panic!("device too big")
+        } else {
+            size_of::<T>() as c_int
+        }
+    };
+
+    let device = mupdf_new_device_of_size(ctx, SIZE, errptr);
+    if !(*errptr).is_null() {
+        return ptr::null_mut();
+    }
+
+    let label = Memento_label(device.cast(), label.as_ptr());
     label.cast()
 }

mupdf-sys/wrapper.c

@@ -2640,6 +2640,20 @@ fz_device *mupdf_new_draw_device(fz_context *ctx, fz_pixmap *pixmap, fz_irect cl
     return device;
 }
 
+fz_device *mupdf_new_device_of_size(fz_context *ctx, int size, mupdf_error_t **errptr)
+{
+    fz_device *device = NULL;
+    fz_try(ctx)
+    {
+        device = fz_new_device_of_size(ctx, size);
+    }
+    fz_catch(ctx)
+    {
+        mupdf_save_error(ctx, errptr);
+    }
+    return device;
+}
+
 fz_device *mupdf_new_display_list_device(fz_context *ctx, fz_display_list *list, mupdf_error_t **errptr)
 {
     fz_device *device = NULL;

src/device.rs

@@ -182,7 +182,7 @@ impl Device {
         Self { dev, list }
     }
 
-    pub fn from_native<D: NativeDevice>(device: D) -> Self {
+    pub fn from_native<D: NativeDevice>(device: D) -> Result<Self, Error> {
         native::create(device)
     }
 

src/device/native.rs

@@ -8,8 +8,8 @@ use std::{
 use mupdf_sys::*;
 
 use crate::{
-    context, BlendMode, ColorParams, Colorspace, Device, Function, Image, Matrix, Path, Rect,
-    Shade, StrokeState, Text,
+    context, BlendMode, ColorParams, Colorspace, Device, Error, Function, Image, Matrix, Path,
+    Rect, Shade, StrokeState, Text,
 };
 
 use super::{DefaultColorspaces, DeviceFlag, Metatext, Structure};
@@ -360,9 +360,10 @@ impl<T: NativeDevice + ?Sized> NativeDevice for &mut T {
     }
 }
 
-pub(crate) fn create<D: NativeDevice>(device: D) -> Device {
-    unsafe {
-        let c_device = mupdf_new_derived_device::<CDevice<D>>(context(), c"RustDevice".as_ptr());
+pub(crate) fn create<D: NativeDevice>(device: D) -> Result<Device, Error> {
+    let ret = unsafe {
+        let c_device: *mut CDevice<D> =
+            ffi_try!(mupdf_new_derived_device(context(), c"RustDevice"));
         ptr::write(&raw mut (*c_device).rust_device, device);
 
         (*c_device).base.close_device = Some(close_device::<D>);
@@ -407,7 +408,8 @@ pub(crate) fn create<D: NativeDevice>(device: D) -> Device {
         (*c_device).base.end_metatext = Some(end_metatext::<D>);
 
         Device::from_raw(c_device.cast(), ptr::null_mut())
-    }
+    };
+    Ok(ret)
 }
 
 #[repr(C)]
@@ -956,7 +958,7 @@ mod test {
         }
 
         let mut ndev = Test::default();
-        let dev = Device::from_native(&mut ndev);
+        let dev = Device::from_native(&mut ndev).unwrap();
 
         dev.begin_layer("begin layer name").unwrap();
         assert_eq!(ndev.begin_layer, 1);
@@ -984,7 +986,7 @@ mod test {
         let detector = proof.clone();
         assert_eq!(Rc::strong_count(&proof), 2, "setup failed");
 
-        let dev = Device::from_native(DropDevice(detector));
+        let dev = Device::from_native(DropDevice(detector)).unwrap();
         assert_eq!(Rc::strong_count(&proof), 2, "dropped too early");
         drop(dev);
         assert_eq!(Rc::strong_count(&proof), 1, "not dropped");