Re: [PATCH v4 6/9] rust: device: Add bindings for reading device properties

[Danilo Krummrich <dakr@kernel.org>]

On Mon, May 19, 2025 at 05:43:17PM +0200, Remo Senekowitsch wrote:
> On Mon May 12, 2025 at 3:36 PM CEST, Danilo Krummrich wrote:
> >> +/// Implemented for all integers that can be read as properties.
> >> +///
> >> +/// This helper trait is needed on top of the existing [`Property`]
> >> +/// trait to associate the integer types of various sizes with their
> >> +/// corresponding `fwnode_property_read_*_array` functions.
> >> +pub trait PropertyInt: Copy {
> >> +    /// # Safety
> >> +    ///
> >> +    /// Callers must uphold the same safety invariants as for the various
> >> +    /// `fwnode_property_read_*_array` functions.
> >
> > I think you have additional requirements on the fwnode, propname and val
> > pointers as well as on nval, please document them as well.
> 
> What are the additional requirements? The implementation just calls the
> underlying `fwnode_property_read_*_array` with the exact same arguments,
> so I don't know what the additional requirements are.

First of all, I don't think you can refer to the safety requirements of the
`fwnode_property_read_*_array` functions, since they don't have any documented
safety requirements.

So, I think you have safety requirements regarding pointer validity of fwnode,
propname and val.

Additionally, there's the requirement that val has to be an array of nval
length.

Also, the PropertyInt trait itself has to be unsafe, given that it contains
unsafe functions.

I also pinged Benno about it, he usually knows best how to cover such things
properly. :)

> >> +    unsafe fn read_array_from_fwnode_property(
> >> +        fwnode: *const bindings::fwnode_handle,
> >> +        propname: *const ffi::c_char,
> >> +        val: *mut Self,
> >> +        nval: usize,
> >> +    ) -> ffi::c_int;
> >> +}