By the way, one of the things I dislike about this series is that there's a needless distinction between struct Foo(bindgen::foo) vs struct FooRef(*mut bindgen::foo) This gets in the way of having an owned Foo embedded into a larger struct. It also gets in the way of instantiating an owned Foo on the stack. My first thought was to use enums: enum Foo { Owned(bindgen::foo), NotOwned(*mut bindgen::foo), } But that would mean that users would invariably pay the price for the owned variant always, as enums use as much space as its largest variant. My current understanding is that we can move all the implementations to traits, with a suitable bound on AsRef<bindings::foo> and AsMut<bindings::foo>. Here is a code example for the wrapper of bindings::v4l2_format (see format.rs), which was extended to account for both owned and non-owned bindgen types: ``` use core::cell::UnsafeCell; /// The shared implementation between Format and FormatRef. pub trait FormatImpl: AsRef<bindings::v4l2_format> + AsMut<bindings::v4l2_format> { /// Returns the `type_` field. fn type_(&self) -> u32 { self.as_ref().type_ } /// Get the field `field` for the `pix` union member. fn pix_field(&self) -> Result<enums::Field> { let fmt = self.as_ref(); let pix = &unsafe { fmt.fmt.pix }; enums::Field::try_from(pix.field) } /// Get the field `width` for the `pix` union member. fn pix_width(&self) -> u32 { let fmt = self.as_ref(); let pix = &unsafe { fmt.fmt.pix }; pix.width } /// Get the field `height` for the `pix` union member. fn pix_height(&self) -> u32 { let fmt = self.as_ref(); let pix = &unsafe { fmt.fmt.pix }; pix.height } /// Get the field `pixelformat` for the `pix` union member. fn pix_pixelformat(&self) -> u32 { let fmt = self.as_ref(); let pix = &unsafe { fmt.fmt.pix }; pix.pixelformat } /// Get the field `bytesperline` for the `pix` union member. fn pix_bytesperline(&self) -> u32 { let fmt = self.as_ref(); let pix = &unsafe { fmt.fmt.pix }; pix.bytesperline } /// Get the field `sizeimage` for the `pix` union member. fn pix_sizeimage(&self) -> u32 { let fmt = self.as_ref(); let pix = &unsafe { fmt.fmt.pix }; pix.sizeimage } /// Get the field `colorspace` for the `pix` union member. fn pix_colorspace(&self) -> Result<enums::Colorspace> { let fmt = self.as_ref(); let pix = &unsafe { fmt.fmt.pix }; enums::Colorspace::try_from(pix.colorspace) } /// Set the field `field` for the `pix` union member. fn set_pix_field(&mut self, field: enums::Field) { let fmt = self.as_mut(); let pix = &mut unsafe { fmt.fmt.pix }; pix.field = field as u32; } /// Set the field `width` for the `pix` union member. fn set_pix_width(&mut self, width: u32) { let fmt = self.as_mut(); let pix = &mut unsafe { fmt.fmt.pix }; pix.width = width; } /// Set the field `height` for the `pix` union member. fn set_pix_height(&mut self, height: u32) { let fmt = self.as_mut(); let pix = &mut unsafe { fmt.fmt.pix }; pix.height = height; } /// Set the field `pixelformat` for the `pix` union member. fn set_pix_pixel_format(&mut self, pixel_format: u32) { let fmt = self.as_mut(); let pix = &mut unsafe { fmt.fmt.pix }; pix.pixelformat = pixel_format; } /// Set the field `bytesperline` for the `pix` union member. fn set_pix_bytesperline(&mut self, bytesperline: u32) { let fmt = self.as_mut(); let pix = &mut unsafe { fmt.fmt.pix }; pix.bytesperline = bytesperline; } /// Set the field `sizeimage` for the `pix` union member. fn set_pix_sizeimage(&mut self, sizeimage: u32) { let fmt = self.as_mut(); let pix = &mut unsafe { fmt.fmt.pix }; pix.sizeimage = sizeimage; } /// Set the field `sizeimage` for the `pix` union member. fn set_pix_colorspace(&mut self, colorspace: enums::Colorspace) { let fmt = self.as_mut(); let pix = &mut unsafe { fmt.fmt.pix }; pix.colorspace = colorspace as u32; } } /// A wrapper over a pointer to `struct v4l2_format`. #[derive(Copy, Clone, Debug, PartialEq, PartialOrd)] pub struct FormatRef(*mut bindings::v4l2_format); impl FormatRef { /// # Safety /// The caller must ensure that `ptr` is valid and remains valid for the lifetime of the /// returned [`FormatRef`] instance. pub unsafe fn from_ptr(ptr: *mut bindings::v4l2_format) -> Self { Self(ptr) } } impl AsRef<bindings::v4l2_format> for FormatRef { fn as_ref(&self) -> &bindings::v4l2_format { // SAFETY: ptr is safe during the lifetime of [`FormatRef`] as per // the safety requirement in `from_ptr()` unsafe { self.0.as_ref().unwrap() } } } impl AsMut<bindings::v4l2_format> for FormatRef { fn as_mut(&mut self) -> &mut bindings::v4l2_format { // SAFETY: ptr is safe during the lifetime of [`FormatRef`] as per // the safety requirement in `from_ptr()` unsafe { self.0.as_mut().unwrap() } } } impl FormatImpl for FormatRef {} /// An owned version of `FormatRef`. #[derive(Default)] pub struct Format(UnsafeCell<bindings::v4l2_format>); impl AsRef<bindings::v4l2_format> for Format { fn as_ref(&self) -> &bindings::v4l2_format { // SAFETY: // It is safe to dereference the pointer since it is valid whenever this type is instantiated. // It is safe to cast this into a shared reference: because this is the // only method that returns &bindings::v4l2_format and because we take // &self, the compiler takes care of enforcing the Rust reference rules for // us. Thus, enforcing the safety guarantees of UnsafeCell::get() by // proxy. unsafe { &*self.0.get() } } } impl AsMut<bindings::v4l2_format> for Format { fn as_mut(&mut self) -> &mut bindings::v4l2_format { // SAFETY: // It is safe to dereference the pointer since it is valid whenever this type is instantiated. // It is safe to cast this into an exclusive reference: because this is the // only method that returns &mut bindings::v4l2_format and because we take // &mut self, the compiler takes care of enforcing the Rust reference rules for // us. Thus, enforcing the safety guarantees of UnsafeCell::get() by // proxy. unsafe { &mut *self.0.get() } } } impl FormatImpl for Format {} ``` This makes it possible to: - Share the implementations between Format and FormatRef, - Have both Format (while paying the cost of storing the bindings::v4l2_format member) and FormatRef (while paying the cost of storing a pointer) separately. - Be generic over Format and FormatRef when needed, e.g.: ``` fn some_fn(format: impl FormatImpl) {...} ``` Thoughts? -- Daniel