linux-kernel - Re: [PATCH RFC 2/3] rust: Add bindings for device properties

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAH5fLgiXPZqKpWSSNdx-Ww-E9h2tOLcF3_8Y4C_JQ0eU8EMwFw@mail.gmail.com>
Date: Tue, 29 Oct 2024 19:48:15 +0100
From: Alice Ryhl <aliceryhl@...gle.com>
To: Rob Herring <robh@...nel.org>
Cc: Saravana Kannan <saravanak@...gle.com>, Greg Kroah-Hartman <gregkh@...uxfoundation.org>, 
	"Rafael J. Wysocki" <rafael@...nel.org>, Miguel Ojeda <ojeda@...nel.org>, 
	Alex Gaynor <alex.gaynor@...il.com>, Boqun Feng <boqun.feng@...il.com>, 
	Gary Guo <gary@...yguo.net>, Björn Roy Baron <bjorn3_gh@...tonmail.com>, 
	Benno Lossin <benno.lossin@...ton.me>, Andreas Hindborg <a.hindborg@...nel.org>, 
	Trevor Gross <tmgross@...ch.edu>, Danilo Krummrich <dakr@...nel.org>, Dirk Behme <dirk.behme@...il.com>, 
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org, 
	rust-for-linux@...r.kernel.org
Subject: Re: [PATCH RFC 2/3] rust: Add bindings for device properties

On Tue, Oct 29, 2024 at 6:58 PM Rob Herring <robh@...nel.org> wrote:
>
> On Tue, Oct 29, 2024 at 9:16 AM Alice Ryhl <aliceryhl@...gle.com> wrote:
> >
> > On Fri, Oct 25, 2024 at 11:06 PM Rob Herring (Arm) <robh@...nel.org> wrote:
> > > +
> > > +    /// Returns array length for firmware property `name`
> > > +    ///
> > > +    /// Valid types are i8, u8, i16, u16, i32, u32, i64, u64
> > > +    pub fn property_count_elem<T>(&self, name: &CStr) -> Result<usize> {
> >
> > This always returns usize? I'm a bit confused ...
>
> The C version returned an int so we could return an errno or positive
> count. With Result, we don't need negative values and isn't usize
> generally used for counts of things like size_t in C?

Ok, I think I misunderstood what this does. usize is fine.

> > > +        match size_of::<T>() {
> > > +            1 => {
> > > +                ret = unsafe {
> > > +                    bindings::device_property_read_u8_array(
> > > +                        self.as_raw(),
> > > +                        name.as_ptr() as *const i8,
> > > +                        ptr::null_mut(),
> > > +                        0,
> > > +                    )
> > > +                }
> > > +            }
> > > +            2 => {
> > > +                ret = unsafe {
> > > +                    bindings::device_property_read_u16_array(
> > > +                        self.as_raw(),
> > > +                        name.as_ptr() as *const i8,
> > > +                        ptr::null_mut(),
> > > +                        0,
> > > +                    )
> > > +                }
> > > +            }
> > > +            4 => {
> > > +                ret = unsafe {
> > > +                    bindings::device_property_read_u32_array(
> > > +                        self.as_raw(),
> > > +                        name.as_ptr() as *const i8,
> > > +                        ptr::null_mut(),
> > > +                        0,
> > > +                    )
> > > +                }
> > > +            }
> > > +            8 => {
> > > +                ret = unsafe {
> > > +                    bindings::device_property_read_u64_array(
> > > +                        self.as_raw(),
> > > +                        name.as_ptr() as *const i8,
> > > +                        ptr::null_mut(),
> > > +                        0,
> > > +                    )
> > > +                }
> > > +            }
> > > +            _ => return Err(EINVAL),
> >
> > You can use `kernel::build_error!` here to trigger a build failure if
> > the size is wrong.
>
> I really want a build error if the type is wrong, then the _ case
> would be unreachable. No way to do that?

One option is to define a trait for integers:

trait Integer: FromBytes + AsBytes + Copy {
    const SIZE: IntSize;
}

enum IntSize {
    S8,
    S16,
    S32,
    S64,
}

macro_rules! impl_int {
    ($($typ:ty),* $(,)?) => {$(
        impl Integer for $typ {
            const SIZE: IntSize = match size_of::<Self>() {
                1 => IntSize::S8,
                2 => IntSize::S16,
                4 => IntSize::S32,
                8 => IntSize::S64,
                _ => panic!("invalid size"),
            };
        }
    )*};
}

impl_int! {
    u8, u16, u32, u64, usize,
    i8, i16, i32, i64, isize,
}

Using the above trait, you can match on the IntSize.

pub fn property_count_elem<T: Integer>(&self, name: &CStr) -> Result<usize> {
match T::SIZE {
    IntSize::S8 => ...,
    IntSize::S16 => ...,
    IntSize::S32 => ...,
    IntSize::S64 => ...,
}

this leaves no catch-all case and calling it with non-integer types
will not compile.

Alice