Re: [PATCH 5/5] rust: device: Add a stub abstraction for devices

[Greg Kroah-Hartman]

On Sat, Feb 25, 2023 at 12:10:47AM +0900, Asahi Lina wrote:
> >> +impl Device {
> >> +    /// Creates a new device instance.
> >> +    ///
> >> +    /// # Safety
> >> +    ///
> >> +    /// Callers must ensure that `ptr` is valid, non-null, and has a non-zero reference count.
> >> +    pub unsafe fn new(ptr: *mut bindings::device) -> Self {
> >> +        // SAFETY: By the safety requirements, ptr is valid and its refcounted will be incremented.
> >> +        unsafe { bindings::get_device(ptr) };
> > 
> > You don't check the return value of get_device()?  What if it failed
> > (hint, it can)?
> 
> Really? I looked at the implementation and I don't see how it can fail,
> as long as the argument is non-NULL and valid... (which is a
> precondition of this unsafe function). Did I miss something?

This function has changed a bit over time, but yes, it could fail if
someone else just dropped the last reference right before you tried to
grab a new one (look at the implementation of kobject_get()).

Yes, if you "know" you have a non-zero reference count first, it should
never fail, but how do you know this?  We have the ability to check the
return value of the function, shouldn't that happen?

thanks,

greg k-h