Re: [PATCH net-next v6 1/6] rust: sizes: add commonly used constants

[Greg KH <gregkh@linuxfoundation.org>]

On Wed, Aug 21, 2024 at 01:54:43AM +0200, Danilo Krummrich wrote:
> On 8/21/24 1:41 AM, Greg KH wrote:
> > On Tue, Aug 20, 2024 at 10:57:14PM +0000, FUJITA Tomonori wrote:
> > > Add rust equivalent to include/linux/sizes.h, makes code more
> > > readable.
> > > 
> > > Reviewed-by: Alice Ryhl <aliceryhl@google.com>
> > > Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> > > Reviewed-by: Benno Lossin <benno.lossin@proton.me>
> > > Reviewed-by: Trevor Gross <tmgross@umich.edu>
> > > Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
> > > ---
> > >   rust/kernel/lib.rs   |  1 +
> > >   rust/kernel/sizes.rs | 26 ++++++++++++++++++++++++++
> > >   2 files changed, 27 insertions(+)
> > >   create mode 100644 rust/kernel/sizes.rs
> > > 
> > > diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> > > index 274bdc1b0a82..58ed400198bf 100644
> > > --- a/rust/kernel/lib.rs
> > > +++ b/rust/kernel/lib.rs
> > > @@ -43,6 +43,7 @@
> > >   pub mod page;
> > >   pub mod prelude;
> > >   pub mod print;
> > > +pub mod sizes;
> > >   mod static_assert;
> > >   #[doc(hidden)]
> > >   pub mod std_vendor;
> > > diff --git a/rust/kernel/sizes.rs b/rust/kernel/sizes.rs
> > > new file mode 100644
> > > index 000000000000..834c343e4170
> > > --- /dev/null
> > > +++ b/rust/kernel/sizes.rs
> > > @@ -0,0 +1,26 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +
> > > +//! Commonly used sizes.
> > > +//!
> > > +//! C headers: [`include/linux/sizes.h`](srctree/include/linux/sizes.h).
> > > +
> > > +/// 0x00000400
> > > +pub const SZ_1K: usize = bindings::SZ_1K as usize;
> > > +/// 0x00000800
> > > +pub const SZ_2K: usize = bindings::SZ_2K as usize;
> > > +/// 0x00001000
> > > +pub const SZ_4K: usize = bindings::SZ_4K as usize;
> > > +/// 0x00002000
> > > +pub const SZ_8K: usize = bindings::SZ_8K as usize;
> > > +/// 0x00004000
> > > +pub const SZ_16K: usize = bindings::SZ_16K as usize;
> > > +/// 0x00008000
> > > +pub const SZ_32K: usize = bindings::SZ_32K as usize;
> > > +/// 0x00010000
> > > +pub const SZ_64K: usize = bindings::SZ_64K as usize;
> > > +/// 0x00020000
> > > +pub const SZ_128K: usize = bindings::SZ_128K as usize;
> > > +/// 0x00040000
> > > +pub const SZ_256K: usize = bindings::SZ_256K as usize;
> > > +/// 0x00080000
> > > +pub const SZ_512K: usize = bindings::SZ_512K as usize;
> > 
> > Why only some of the values in sizes.h?
> > 
> > And why can't sizes.h be directly translated into rust code without
> > having to do it "by hand" here?  We do that for other header file
> > bindings, right?
> 
> Those are all generated bindings from C headers, e.g. `bindings::SZ_2K `, but
> bindgen isn't guaranteed to assign the type we want (`usize` in this case), plus
> for size constants having the `bindings::` prefix doesn't really add any value.
> 
> Rust could also define those without using bindings at all, but this was already
> discussed in earlier versions of this series.

Then this information should probably go in the changelog text so that
others don't keep asking the same question :)

thanks,

greg k-h