Re: [PATCH v2 1/2] rust: add abstraction for struct device

[Lyude Paul]

On Thu, 2024-06-13 at 14:22 +0200, Danilo Krummrich wrote:
> On Thu, Jun 13, 2024 at 07:47:13AM +0200, Greg KH wrote:
> > On Wed, Jun 12, 2024 at 10:56:43PM +0200, Danilo Krummrich wrote:
> > 
> > As someone who has written comments in code for functions that are
> > always ignored, I am happy to see you think that this is actually
> > going
> > to do anything :)
> 
> One advantage we have in Rust is, that if something has a specific
> requirement
> we mark it as `unsafe` and the user of the API has to put it in an
> `unsafe`
> block, which we require a sfety comment for, where the user of the
> API has to
> explain why this is the correct thing to do in this case.
> 
> In other words we can technically enforce that people read the
> comment and
> explain how they ensure to fulfill what the comment asks for.

Been going through this thread to catch up on the status of the patches
here: I've been working with danilo for a while, but have been busy
with kernel modesetting bindings for rust. Anyway, as someone who's
spent quite a lot of time writing up really detailed documentation for
some of the DisplayPort MST kernel APIs only to keep running into
situations where said documentation was ignored (not to criticize
anyone! I'm certainly guilty of having misread documentation or not
reading something myself :) I like to think about unsafe blocks in the
sense of being one thing I'm less likely to have to keep repeating
myself about over a mailing list. Because I know at the very least that
a contributor has to actively ignore a compiler warning or be aware in
some sense that there is some kind of safety constraint they're being
held responsible for that will come up during code review. If it's just
a comment in C code, I don't even have a guarantee that a contributor's
IDE isn't hiding large kdoc comments by default.

Also, in regards to rust code relying on C code not being guaranteed
safe even if it is 'safe' code: frankly, sometimes I wish "unsafe" was
just named "danger". "un-safe" implies code outside the block is
perfectly "safe" - but it is still possible to introduce UB in entirely
safe code bases (source: cve-rs) and most rust contributors I've talked
to don't see safe code as being perfectly-safe either.

> 
> > 
> > Heck, I have put huge messages in kernel code for when people
> > implement
> > the api wrong[1], and they still ignore that at runtime.  Only way
> > to get
> > it noticed is if you break someone's build.
> 
> You only see the ones who still do it wrong. You probably don't have
> visibility
> of the ones who did it right due to your effort to write it down. :-)
> 
> > 
> > So you all need to really define what `Send` means, as it sounds
> > like a
> > userspace thing that isn't going to fit in well within the kernel
> > model.
> 
> Please see the explanation above.
> 
> > 
> > thanks,
> > 
> > greg k-h
> > 
> > [1] See the pr_debug() calls in kobject_cleanup() as proof, people
> > just
> >     create an "empty" release function to shut them up, thinking
> > that is
> >     the correct solution when obviously it is not...
> > 
> 

-- 
Cheers,
 Lyude Paul (she/her)
 Software Engineer at Red Hat