Re: [PATCH] rust: Implement fmt::Display for kernel Box<T, A>

[Boqun Feng <boqun.feng@gmail.com>]

Hi,

Thank you for the patch! A few comments below:

First, the patch title should be as tight as possible, please consider
using something like:

	rust: alloc: Implement Display for Box

* `Display` is a common trait in Rust, so can avoid mention its mod name
  (i.e. "fmt::").

* Since this is a *kernel* patch, so the word "kernel" can be avoided.

* The generic parameter is not related to the impl, so can be avoid.

On Mon, Oct 28, 2024 at 10:52:47PM +0800, JCKeep wrote:
> Currently fmt::Display is not implemented for Box<T, A>. We can
> make the following code work by implementing fmt::Display.
> 

Usually phrases like "We can" are not preferred in changelogs,
especially when describing the "why" part of the changes, because in
this part you want to describe the current situation based on *facts*
not on *desires*. So how about:

    Currently `impl Display` is missing for `Box<T, A>`, as a result,
    things like using `Box<..>` directly as an operand in `pr_info!()`
    are impossible, which is less ergonomic compared to `Box` in Rust
    std.

    Therefore add `impl Display` for `Box`.

?

> let v = KBox::new(42, GFP_KERNEL)?;
> pr_info!("{}", v);
> 
> Link: https://github.com/Rust-for-Linux/linux/issues/1126

Please use a Closes tag to refer an issue you want to close on GitHub.

Regards,
Boqun

> Signed-off-by: JCKeep <2407018371@qq.com>
> ---
>  rust/kernel/alloc/kbox.rs | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/rust/kernel/alloc/kbox.rs b/rust/kernel/alloc/kbox.rs
> index d69c32496..5f4765800 100644
> --- a/rust/kernel/alloc/kbox.rs
> +++ b/rust/kernel/alloc/kbox.rs
> @@ -427,6 +427,16 @@ fn deref_mut(&mut self) -> &mut T {
>      }
>  }
>  
> +impl<T, A> fmt::Display for Box<T, A>
> +where
> +    T: ?Sized + fmt::Display,
> +    A: Allocator,
> +{
> +    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
> +        fmt::Display::fmt(&**self, f)
> +    }
> +}
> +
>  impl<T, A> fmt::Debug for Box<T, A>
>  where
>      T: ?Sized + fmt::Debug,
> -- 
> 2.34.1
>