Rust-for-Linux / linux

Adding support for the Rust language to the Linux kernel.
https://rust-for-linux.com
Other
3.95k stars 421 forks source link

`try_from_foreign()` #1057

Closed fbq closed 7 months ago

fbq commented 8 months ago

Currently ForeignOwnable::from_foreign() only works for non-null pointers for the existing impls (e.g. Box, Arc). It may create a few duplicate code like:

// `p` is a maybe null pointer
if p.is_null() {
    None
} else {
     Some(unsafe { T::from_foreign(p) })
}

see this patch for example.

It's better if we have a function: *const void -> Option<ForeignOwnable>, which can handle null pointer cases. There are at least two ways I can think of:

  1. a try_from_foreign() function in ForeignOwnable trait:
    pub trait ForeignOwnable {
    unsafe fn try_from_foreign(ptr: *const void) -> Option<Self> {
        if ptr.is_null() {
            None
        } else {
            Some(Self::from_foreign(ptr))
        }
    }
    }
  2. impl ForeignOwnable for Option<T: ForeignOwnable>:
impl<T: ForeignOwnable> ForeignOwnable for Option<T> {
    type Borrowed<'a> = Option<T::Borrowed<'a>>;
    unsafe fn borrow<'a>(ptr: *const void) -> Self::Borrowed<'a> {
        if ptr.is_null() {
            None
        } else {
            Some(T::borrow(ptr))
        }
     }
    unsafe fn from_foreign(ptr: *const void) -> Self {
        if ptr.is_null() {
            None
        } else {
            Some(T::from_foreign(ptr))
        }
     }
}

Of course, there could be better ideas ;-)

y86-dev commented 8 months ago

I prefer option 1, since that way is simpler and can be better documented. People who want to do what you described probably will not reach for Option<T> on their first try, but rather do the null check manually.

ojeda commented 7 months ago

try_from_foreign (option 1 here) is now in rust-next.