On 12/13/23 11:28, Andrew Lunn wrote:

quoted

I still think you need to justify why `mdio.bus` is a pointer that you
can give to `midobus_read`.

Maybe a dumb question. Why are you limiting this to just a few members
of struct phy_device? It has ~50 members, any of which can be used by
the C side when Rust calls into C.

I limited it to those few members, because the Rust side only uses
those.
Theoretically one could specify all invariants for all members, but that
seems like a *lot* of work.

In Rust everything [1] has to be initialized with a valid value.
Contrast this with C where everything is potentially uninitialized. As
an example, let's look at the first few fields of `PhyDevice`:

    struct PhyDevice {
        mdio: MdioDevice,
        drv: Box<PhyDriver>,
        devlink: Box<DeviceLink>,
        phy_id: u32,
        c45_ids: PhyC45DeviceIds,
        // ...
    }

Note that in Rust we now do not need to write down any invariants, since
they are all implicitly encoded. For example the `drv` field is of type
`Box<PhyDriver>`, it *always* is a pointer to an allocation that
contains a valid `PhyDriver`. So while on the C side you would have to
state this, on the Rust side we get it for free.

Rust function also make use of the "everything is in a valid state"
rule, they know that the fields are valid and thus the Rust equivalent
of `phy_read` could be safe and without any comments:

    impl PhyDevice {
        fn phy_read(&self, regnum: u32) -> i32 {
            self.mdio.bus.mdiobus_read(self.mdio.addr, regnum)
        }
    }

All of this only applies to safe code, `unsafe` code is allowed to
violate all of these things temporarily. However, when it gives a value
back to safe code [2], that value needs to be valid.

I think that specifying all of these implicit invariants in C will be
extremely laborious. Especially since the usual way of doing things in C
is not considering these invariants (at least not consciously), but
rather "just do the thing".

The way we do the interoperability is to just fully trust the C side to
produce valid values that we can feed back to the C side. Of course
there are caveats, so e.g. one needs to initialize a `struct mutex`
before it can be used, but that is what we need to capture with the
safety comments.

[1]: There are exceptions for this, but for the purposes of this
     discussion, they can be ignored. If you want to know more, you can
     read this article in the nomicon:
     https://doc.rust-lang.org/nomicon/unchecked-uninit.html

[2]: There are also exceptions, but I will omit them here.

-- 
Cheers,
Benno