[PATCH] rust: kernel: str: Implement Debug for CString

[PATCH] rust: kernel: str: Implement Debug for CString

[Asahi Lina]

Trivial implementation.

Signed-off-by: Asahi Lina <lina@asahilina.net>
---
 rust/kernel/str.rs | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
index c9dd3bf59e34..a94e396d39e1 100644
--- a/rust/kernel/str.rs
+++ b/rust/kernel/str.rs
@@ -606,6 +606,12 @@ fn try_from(cstr: &'a CStr) -> Result<CString, AllocError> {
     }
 }
 
+impl fmt::Debug for CString {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        fmt::Debug::fmt(&**self, f)
+    }
+}
+
 /// A convenience alias for [`core::format_args`].
 #[macro_export]
 macro_rules! fmt {

---
base-commit: 06c2afb862f9da8dc5efa4b6076a0e48c3fbaaa5
change-id: 20230714-cstring-debug-ca021fe0ad78

Thank you,
~~ Lina

Re: [PATCH] rust: kernel: str: Implement Debug for CString

[Alice Ryhl]

Asahi Lina <lina@asahilina.net> writes:
> Trivial implementation.
> 
> Signed-off-by: Asahi Lina <lina@asahilina.net>

The commit message is a bit short, but the change itself looks fine.

Reviewed-by: Alice Ryhl <aliceryhl@google.com>

Re: [PATCH] rust: kernel: str: Implement Debug for CString

[Asahi Lina]

On 14/07/2023 18.48, Alice Ryhl wrote:
> Asahi Lina <lina@asahilina.net> writes:
>> Trivial implementation.
>>
>> Signed-off-by: Asahi Lina <lina@asahilina.net>
> 
> The commit message is a bit short, but the change itself looks fine.
> 
> Reviewed-by: Alice Ryhl <aliceryhl@google.com>

It's so trivial I just didn't know what else to write... suggestions 
welcome (for this or next time I have a patch like this) ^^

~~ Lina

Re: [PATCH] rust: kernel: str: Implement Debug for CString

[Martin Rodriguez Reboredo]

On 7/14/23 09:21, Asahi Lina wrote:
> On 14/07/2023 18.48, Alice Ryhl wrote:
>> Asahi Lina <lina@asahilina.net> writes:
>>> Trivial implementation.
>>>
>>> Signed-off-by: Asahi Lina <lina@asahilina.net>
>>
>> The commit message is a bit short, but the change itself looks fine.
>>
>> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
> 
> It's so trivial I just didn't know what else to write... suggestions
> welcome (for this or next time I have a patch like this) ^^
> 
> ~~ Lina
> 

Just describe what it does, like "cast the `CStr` to an `&str` and call
`fmt::Debug::fmt` with it". I'll add my Reviewed-by, so retain it in
case it's asked to change the commit message.

Reviewed-by: Martin Rodriguez Reboredo <yakoyoku@gmail.com>

Re: [PATCH] rust: kernel: str: Implement Debug for CString

[Ariel Miculas]

On Fri, Jul 14, 2023 at 12:39 PM Asahi Lina <lina@asahilina.net> wrote:
>
> Trivial implementation.
>
> Signed-off-by: Asahi Lina <lina@asahilina.net>
> ---
>  rust/kernel/str.rs | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
> index c9dd3bf59e34..a94e396d39e1 100644
> --- a/rust/kernel/str.rs
> +++ b/rust/kernel/str.rs
> @@ -606,6 +606,12 @@ fn try_from(cstr: &'a CStr) -> Result<CString, AllocError> {
>      }
>  }
>
> +impl fmt::Debug for CString {
> +    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
> +        fmt::Debug::fmt(&**self, f)
> +    }
> +}
> +
>  /// A convenience alias for [`core::format_args`].
>  #[macro_export]
>  macro_rules! fmt {
>
> ---
> base-commit: 06c2afb862f9da8dc5efa4b6076a0e48c3fbaaa5
> change-id: 20230714-cstring-debug-ca021fe0ad78
>
> Thank you,
> ~~ Lina
>

Glad I wasn't the only one missing this, now I don't have to write the
awkard `&*` anymore, as in:
```
pr_debug!("trying to open {:?}\n", &*filename);
```

Cheers,
Ariel

Re: [PATCH] rust: kernel: str: Implement Debug for CString

[Alice Ryhl]

Asahi Lina <lina@asahilina.net> writes:
> On 14/07/2023 18.48, Alice Ryhl wrote:
>> Asahi Lina <lina@asahilina.net> writes:
>>> Trivial implementation.
>>>
>>> Signed-off-by: Asahi Lina <lina@asahilina.net>
>> 
>> The commit message is a bit short, but the change itself looks fine.
>> 
>> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
> 
> It's so trivial I just didn't know what else to write... suggestions 
> welcome (for this or next time I have a patch like this) ^^
> 
> ~ Lina

Adding some sort of motivation usually works quite well, e.g.:

Make it possible to use a CString with the `pr_*` macros directly, that
is, instead of

	pr_debug!("trying to open {:?}\n", &*filename);

we can now write

	pr_debug!("trying to open {:?}\n", filename);

Alice

Re: [PATCH] rust: kernel: str: Implement Debug for CString

[Miguel Ojeda]

On Fri, Jul 14, 2023 at 4:02 PM Alice Ryhl <aliceryhl@google.com> wrote:
>
> Adding some sort of motivation usually works quite well, e.g.:
>
> Make it possible to use a CString with the `pr_*` macros directly, that
> is, instead of
>
>         pr_debug!("trying to open {:?}\n", &*filename);
>
> we can now write
>
>         pr_debug!("trying to open {:?}\n", filename);

Indeed, this would be the most important bit, i.e. answering the "why?".

The "what?" and the "how?" are pretty much explained by the title, but
it is also fine giving more details (but if the implementation
requires an explanation, then it is usually best to write an actual
source code comment instead).

Cheers,
Miguel

Re: [PATCH] rust: kernel: str: Implement Debug for CString

[Ariel Miculas (amiculas)]

On 23/07/14 05:01PM, Miguel Ojeda wrote:
> On Fri, Jul 14, 2023 at 4:02 PM Alice Ryhl <aliceryhl@google.com> wrote:
> >
> > Adding some sort of motivation usually works quite well, e.g.:
> >
> > Make it possible to use a CString with the `pr_*` macros directly, that
> > is, instead of
> >
> >         pr_debug!("trying to open {:?}\n", &*filename);
> >
> > we can now write
> >
> >         pr_debug!("trying to open {:?}\n", filename);
> 
> Indeed, this would be the most important bit, i.e. answering the "why?".
> 
> The "what?" and the "how?" are pretty much explained by the title, but
> it is also fine giving more details (but if the implementation
> requires an explanation, then it is usually best to write an actual
> source code comment instead).
> 
> Cheers,
> Miguel

Any follow-up on this? It sure would make my logging cleaner.

Cheers,
Ariel

Re: [PATCH] rust: kernel: str: Implement Debug for CString

[Miguel Ojeda]

On Wed, Oct 25, 2023 at 6:18 PM Ariel Miculas (amiculas)
<amiculas@cisco.com> wrote:
>
> Any follow-up on this? It sure would make my logging cleaner.

We were expecting a new version, but I can pick it up with e.g. the
message that Alice suggested (i.e. marking it as modified by me).

Cheers,
Miguel

Re: [PATCH] rust: kernel: str: Implement Debug for CString

[Benno Lossin]

On 14.07.23 11:19, Asahi Lina wrote:
> Trivial implementation.
> 
> Signed-off-by: Asahi Lina <lina@asahilina.net>

With a better commit message you can add

Reviewed-by: Benno Lossin <benno.lossin@proton.me>

-- 
Cheers,
Benno

> ---
>   rust/kernel/str.rs | 6 ++++++
>   1 file changed, 6 insertions(+)
> 
> diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
> index c9dd3bf59e34..a94e396d39e1 100644
> --- a/rust/kernel/str.rs
> +++ b/rust/kernel/str.rs
> @@ -606,6 +606,12 @@ fn try_from(cstr: &'a CStr) -> Result<CString, AllocError> {
>       }
>   }
> 
> +impl fmt::Debug for CString {
> +    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
> +        fmt::Debug::fmt(&**self, f)
> +    }
> +}
> +
>   /// A convenience alias for [`core::format_args`].
>   #[macro_export]
>   macro_rules! fmt {
> 
> ---
> base-commit: 06c2afb862f9da8dc5efa4b6076a0e48c3fbaaa5
> change-id: 20230714-cstring-debug-ca021fe0ad78
> 
> Thank you,
> ~~ Lina
>

Re: [PATCH] rust: kernel: str: Implement Debug for CString

[Miguel Ojeda]

On Fri, Jul 14, 2023 at 11:19 AM Asahi Lina <lina@asahilina.net> wrote:
>
> Trivial implementation.
>
> Signed-off-by: Asahi Lina <lina@asahilina.net>

Applied to `rust-next` with the commit message from Alice.

Thanks everyone!

Cheers,
Miguel