| 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
| ||
|
Message-ID: <87jz9ewcf1.fsf@kernel.org>
Date: Tue, 25 Feb 2025 11:23:30 +0100
From: Andreas Hindborg <a.hindborg@...nel.org>
To: "Daniel Almeida" <daniel.almeida@...labora.com>
Cc: "Danilo Krummrich" <dakr@...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>,
"Alice Ryhl" <aliceryhl@...gle.com>, "Trevor Gross" <tmgross@...ch.edu>,
"Joel Becker" <jlbec@...lplan.org>, "Christoph Hellwig" <hch@....de>,
"Peter Zijlstra" <peterz@...radead.org>, "Ingo Molnar"
<mingo@...hat.com>, "Will Deacon" <will@...nel.org>, "Waiman Long"
<longman@...hat.com>, "Fiona Behrens" <me@...enk.dev>, "Charalampos
Mitrodimas" <charmitro@...teo.net>, <rust-for-linux@...r.kernel.org>,
<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v4 3/4] rust: configfs: add a sample demonstrating
configfs usage
"Daniel Almeida" <daniel.almeida@...labora.com> writes:
> Hi Andreas,
>
>> On 24 Feb 2025, at 10:21, Andreas Hindborg <a.hindborg@...nel.org> wrote:
>>
>> Add a sample to the samples folder, demonstrating the intended use of the
>> rust configfs API.
>
> nit: this is not the first time I see Rust not capitalized in this series.
Will fix 👍
[...]
>> +impl kernel::InPlaceModule for RustConfigfs {
>> + fn init(_module: &'static ThisModule) -> impl PinInit<Self, Error> {
>> + pr_info!("Rust configfs sample (init)\n");
>> +
>> + let item_type = configfs_attrs! {
>> + container: configfs::Subsystem<Configuration>,
>> + data: Configuration,
>> + child: Child,
>> + attributes: [
>> + message: 0,
>> + bar: 1,
>> + ],
>> + };
>
> As I said in the previous patch, this macro is a bit complex. Is there anything more you can do with it?
>
> If so, it better be in this driver, because I hardly think anybody will go through the source code itself
> to figure out. My 2c.
I can add some inline comments on the usage here. Is that what you are
thinking of?
>
>
>> +
>> + try_pin_init!(Self {
>> + config <- configfs::Subsystem::new(
>> + c_str!("rust_configfs"), item_type, Configuration::new()
>> + ),
>> + })
>> + }
>> +}
>> +
>> +#[vtable]
>> +impl configfs::GroupOperations for Configuration {
>> + type Child = Child;
>> +
>> + fn make_group(&self, name: &CStr) -> Result<impl PinInit<configfs::Group<Child>, Error>> {
>> + let tpe = configfs_attrs! {
>> + container: configfs::Group<Child>,
>> + data: Child,
>> + child: GrandChild,
>> + attributes: [
>> + baz: 0,
>> + ],
>> + };
>> +
>> + Ok(configfs::Group::new(name.try_into()?, tpe, Child::new()))
>> + }
>> +}
>> +
>> +#[vtable]
>> +impl configfs::AttributeOperations<0> for Configuration {
>> + type Data = Configuration;
>> +
>> + fn show(container: &Configuration, page: &mut [u8; PAGE_SIZE]) -> Result<usize> {
>> + pr_info!("Show message\n");
>> + let data = container.message;
>> + page[0..data.len()].copy_from_slice(data);
>> + Ok(data.len())
>> + }
>> +}
>> +
>> +#[vtable]
>> +impl configfs::AttributeOperations<1> for Configuration {
>> + type Data = Configuration;
>> +
>> + fn show(container: &Configuration, page: &mut [u8; PAGE_SIZE]) -> Result<usize> {
>> + pr_info!("Show bar\n");
>> + let guard = container.bar.lock();
>> + let data = guard.0.as_slice();
>> + let len = guard.1;
>> + page[0..len].copy_from_slice(&data[0..len]);
>> + Ok(len)
>> + }
>> +
>> + fn store(container: &Configuration, page: &[u8]) -> Result {
>> + pr_info!("Store bar\n");
>> + let mut guard = container.bar.lock();
>> + guard.0[0..page.len()].copy_from_slice(page);
>> + guard.1 = page.len();
>> + Ok(())
>> + }
>> +}
>> +
>> +#[pin_data]
>> +struct Child {}
>
> nit: you don’t need the braces here
Can't do that. The `pin_data` macro won't eat it. I'll add a comment.
>
>> +
>> +impl Child {
>> + fn new() -> impl PinInit<Self, Error> {
>> + try_pin_init!(Self {})
>> + }
>> +}
>> +
>> +#[vtable]
>> +impl configfs::GroupOperations for Child {
>> + type Child = GrandChild;
>> +
>> + fn make_group(&self, name: &CStr) -> Result<impl PinInit<configfs::Group<GrandChild>, Error>> {
>> + let tpe = configfs_attrs! {
>> + container: configfs::Group<GrandChild>,
>> + data: GrandChild,
>> + attributes: [
>> + gc: 0,
>> + ],
>> + };
>> +
>> + Ok(configfs::Group::new(
>> + name.try_into()?,
>> + tpe,
>> + GrandChild::new(),
>> + ))
>> + }
>> +}
>> +
>> +#[vtable]
>> +impl configfs::AttributeOperations<0> for Child {
>> + type Data = Child;
>> +
>> + fn show(_container: &Child, page: &mut [u8; PAGE_SIZE]) -> Result<usize> {
>> + pr_info!("Show baz\n");
>> + let data = c"Hello Baz\n".to_bytes();
>> + page[0..data.len()].copy_from_slice(data);
>> + Ok(data.len())
>> + }
>> +}
>> +
>> +#[pin_data]
>> +struct GrandChild {}
>
> …nor here
>
>> +
>> +impl GrandChild {
>> + fn new() -> impl PinInit<Self, Error> {
>> + try_pin_init!(Self {})
>> + }
>> +}
>> +
>> +#[vtable]
>> +impl configfs::AttributeOperations<0> for GrandChild {
>> + type Data = GrandChild;
>> +
>> + fn show(_container: &GrandChild, page: &mut [u8; PAGE_SIZE]) -> Result<usize> {
>> + pr_info!("Show baz\n");
>
> As I said in the cover letter, perhaps this one slip through :)
Yes, thank you.
>
>> + let data = c"Hello GC\n".to_bytes();
>> + page[0..data.len()].copy_from_slice(data);
>> + Ok(data.len())
>> + }
>> +}
>>
>> --
>> 2.47.0
>>
>>
>
> I’m OK with this patch. It works, and it does what it’s supposed to do, i.e.: showcase the API.
>
> With the “Show baz” nit fixed:
>
> Reviewed-by: Daniel Almeida <daniel.almeida@...labora.com>
Thanks!
Best regards,
Andreas Hindborg
Powered by blists - more mailing lists