Subject: Re: [PATCH net-next v10 1/4] rust: core abstractions for network PHY drivers

From: Boqun Feng <[email-masked]>
Date: Wed, 13 Dec 2023 15:59:24 -0800

On Wed, Dec 13, 2023 at 10:48:39PM +0100, Andrew Lunn wrote:
> > Well, a safety comment is a basic part of Rust, which identifies the
> > safe/unsafe boundary (i.e. where the code could go wrong in memory
> > safety) and without that, the code will be just using Rust syntax and
> > grammar. Honestly, if one doesn't try hard to identify the safe/unsafe
> > boundaries, why do they try to use Rust? Unsafe Rust is harder to write
> > than C, and safe Rust is pointless without a clear safe/unsafe boundary.
> > Plus the syntax is not liked by anyone last time I heard ;-)
> 
> Maybe comments are the wrong format for this? Maybe it should be a

Maybe, but they are what we have right now. I do believe the unsafe <->
safe boundary in Rust needs better tooling: using comments may be a
little bit abitrary. However, let's stick to what we have right now.
Benno is actually working our Rust safety standard (feel free to correct
me if I get the name wrong), hopefully that helps.

> formal language? It could then be compiled into an executable form and
> tested? It won't show it is complete, but it would at least show it is
> correct/incorrect description of the assumptions. For normal builds it
> would not be included in the final binary, but maybe debug or formal
> verification builds it would be included?
> 

Good idea, we actually want something similar when we were talking about
Benno's safety standard, but as you may know a complete one would take
years, or maybe impossbile. Also, kernel's safety requirement sometimes
is weird and non-trivial, it's going to be a learning process ;-) A tool
is certainly what we would like to look into if we have more experience
in Rust abstraction and more time.

> > Having a correct safety comment is really the bottom line. Without that,
> > it's just bad Rust code, which I don't think netdev doesn't want either?
> > Am I missing something here?
> 
> It seems much easier to agree actual code is correct, maybe because it
> is a formal language, with a compiler, and a method to test it. Is
> that code really bad without the comments? It would be interesting to

Note that most of the comment reviews are on *safety* comments (type
invariants are related to safety comments). To me, in the review
process, safety comments are similar to commit logs. In kernel patch
reviews, we sometimes say "thanks for the explanation but please put
that in the commit log", that is, even we know the code is correct and
the patchset has been explained, we still want to see the explanation
written down somewhere. Commit logs are ways to explain "why do I want
to do this", and safety comments are ways to explain "why can I do the
following safely". They are both ways to communicate during the review
and for the future readers.

So is a patchset really bad without commit logs? ;-)

> look back and see how much the actual code has changed because of
> these comments? I _think_ most of the review comments have resulted in

I was trying to figure out the answer, but the patchset just has too
many versions.. and discussions were splitted between versions...

> changes to the comments, not the executable code itself. Does that
> mean it is much harder to write correct comments than correct code?
> 

If one can explain their code to others and make sure others understand
the correctness of the code, then the code is very likely correct. So
I think yes, writing correct comments is harder than correct code.

I admit the review process took a bit long because it's a learning
process for both developers and reviewers: being the first in-tree Rust
driver may not be that easy, please understand the high standard
expectation.

Regards,
Boqun

>        Andrew