Re: [RFC PATCH 3/3] samples: rust: add kobject sample

[Greg KH <gregkh@linuxfoundation.org>]

On Sun, Dec 08, 2024 at 02:15:45PM +0100, Daniel Sedlak wrote:
> Add basic example using the kobject API using Rust.
> The example is similar to the already existing
> examples samples/kobject/{kobject-example.c,kset-example.c}.

No, please no.  let's not have "raw kobjects in rust" even be an example
of something that is a good idea to use...

That being said:

> +config SAMPLE_RUST_KOBJECT
> +	bool "Build kobject example in Rust"
> +	help
> +	  This config option allows to build kobject
> +	  exampel written in Rust.

spelling check :)

And wrap your lines at a longer length please.

> index 000000000000..90db937dea5a
> --- /dev/null
> +++ b/samples/rust/kobject_example.rs
> @@ -0,0 +1,66 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Rust example using kobjects.

No copyright?   No authorship?

> +use kernel::{
> +    c_str,
> +    kobject::{subsystems, KObject, KObjectTextAttribute},
> +    prelude::*,
> +    str::CString,
> +};
> +
> +module! {
> +    type: RustKObject,
> +    name: "rust_kobject",
> +    author: "Rust for Linux Contributors",

No taking ownership of this?  I REALLY want someone to own this if it
ever happens, so that they can handle all of the fallout over time with
it :)

> +    license: "GPL",
> +}
> +
> +struct RustKObject {
> +    // This kobject may be referenced later by other parts of code to update
> +    // the values stored within [`MyKObject`].
> +    _kobject: Pin<KBox<KObject<MyKObject>>>,
> +}
> +
> +/// Sample KObject state which creates directory in the `/sys/kernel/my_kobject`.
> +#[derive(Default)]
> +pub struct MyKObject {
> +    text: Option<CString>,
> +}
> +
> +/// Attribute of the [`MyKObject`], creates file in the `/sys/kernel/my_kobject/my-text`.
> +/// Where `show()` is triggered when read is called and `store()` is called
> +/// when new data are written.
> +pub struct MyKObjectAttribute;
> +#[vtable]
> +impl KObjectTextAttribute<MyKObject> for MyKObjectAttribute {
> +    const NAME: &'static CStr = c_str!("my-text");
> +
> +    /// Called for example by `cat /sys/kernel/my_kobject/my-text`.
> +    fn show(this: &mut MyKObject) -> Result<CString> {
> +        if let Some(text) = &this.text {
> +            CString::try_from_fmt(fmt!("Text that was stored: '{text:?}'\n"))
> +        } else {
> +            CString::try_from_fmt(fmt!("No one stored anything yet, value {}.\n", this.value))

Wouldn't this also happen if the text was ""?  If not, why isn't an
empty string the default to start with?

thanks,

greg k-h