diff mbox series

rust: kernel: str: Implement Debug for CString

Message ID 20230714-cstring-debug-v1-1-4e7c3018dd4f@asahilina.net
State New
Headers
Series rust: kernel: str: Implement Debug for CString |

Commit Message

Asahi Lina July 14, 2023, 9:19 a.m. UTC 
  Trivial implementation.

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


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

Thank you,
~~ Lina

Comments

Alice Ryhl July 14, 2023, 9:48 a.m. UTC | #1 
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>
Ariel Miculas July 14, 2023, 10:05 a.m. UTC | #2 
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
Asahi Lina July 14, 2023, 12:21 p.m. UTC | #3 
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
Martin Rodriguez Reboredo July 14, 2023, 1:46 p.m. UTC | #4 
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>
Alice Ryhl July 14, 2023, 2:02 p.m. UTC | #5 
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
Miguel Ojeda July 14, 2023, 3:01 p.m. UTC | #6 
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
Benno Lossin July 15, 2023, 10 a.m. UTC | #7 
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>
Ariel Miculas Oct. 25, 2023, 4:18 p.m. UTC | #8 
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
Miguel Ojeda Oct. 25, 2023, 4:55 p.m. UTC | #9 
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
Miguel Ojeda Dec. 13, 2023, 6:43 p.m. UTC | #10 
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
diff mbox series

Patch

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 {