On Tue, Jun 04, 2024 at 04:17:29PM +0200, Greg KH wrote:
> On Tue, May 21, 2024 at 10:42:03PM +0200, Danilo Krummrich wrote:
> > On Tue, May 21, 2024 at 11:24:38AM +0200, Greg KH wrote:
> > > On Mon, May 20, 2024 at 10:22:22PM +0200, Danilo Krummrich wrote:
> > > > > > +impl Device {
> > > > > > + /// Creates a new ref-counted instance of an existing device pointer.
> > > > > > + ///
> > > > > > + /// # Safety
> > > > > > + ///
> > > > > > + /// Callers must ensure that `ptr` is valid, non-null, and has a non-zero reference count.
> > > > >
> > > > > Callers NEVER care about the reference count of a struct device, anyone
> > > > > poking in that is asking for trouble.
> > > >
> > > > That's confusing, if not the caller who's passing the device pointer somewhere,
> > > > who else?
> > > >
> > > > Who takes care that a device' reference count is non-zero when a driver's probe
> > > > function is called?
> > >
> > > A device's reference count will be non-zero, I'm saying that sometimes,
> > > some driver core functions are called with a 'struct device' that is
> > > NULL, and it can handle it just fine. Hopefully no callbacks to the
> > > rust code will happen that way, but why aren't you checking just "to be
> > > sure!" otherwise you could have a bug here, and it costs nothing to
> > > verify it, right?
> >
> > I get your point on that one. But let me explain a bit more why I think that
> > check is not overly helpful here.
> >
> > In Rust we have the concept of marking functions as 'unsafe'. Unsafe functions
> > need to document their safety preconsitions, i.e. the conditions the caller of
> > the function must guarantee. The caller of an unsafe function needs an unsafe
> > block for it to compile and every unsafe block needs an explanation why it is
> > safe to call this function with the corresponding arguments.
> >
> > (Ideally, we want to avoid having them in the first place, but for C abstractions
> > we have to deal with raw pointers we receive from the C side and dereferencing a
> > raw pointer is unsafe by definition.)
> >
> > In this case we have a function that constructs the Rust `Device` structure from
> > a raw (device) pointer we potentially received from the C side. Now we have to
> > decide whether this function is going to be unsafe or safe.
> >
> > In order for this function to be safe we would need to be able to guarantee that
> > this is a valid, non-null pointer with a non-zero reference count, which
> > unfortunately we can't. Hence, it's going to be an unsafe function.
>
> But you can verify it is non-null, so why not?
I suggest to check out the code making use of this.
