2023-07-14 10:41:28

by Asahi Lina

[permalink] [raw]
Subject: [PATCH] rust: kernel: str: Implement Debug for CString

Trivial implementation.

Signed-off-by: Asahi Lina <[email protected]>
---
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



2023-07-14 10:46:27

by Ariel Miculas

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

On Fri, Jul 14, 2023 at 12:39 PM Asahi Lina <[email protected]> wrote:
>
> Trivial implementation.
>
> Signed-off-by: Asahi Lina <[email protected]>
> ---
> 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

2023-07-14 14:17:45

by Alice Ryhl

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

Asahi Lina <[email protected]> writes:
> On 14/07/2023 18.48, Alice Ryhl wrote:
>> Asahi Lina <[email protected]> writes:
>>> Trivial implementation.
>>>
>>> Signed-off-by: Asahi Lina <[email protected]>
>>
>> The commit message is a bit short, but the change itself looks fine.
>>
>> Reviewed-by: Alice Ryhl <[email protected]>
>
> 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


2023-07-14 15:32:52

by Miguel Ojeda

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

On Fri, Jul 14, 2023 at 4:02 PM Alice Ryhl <[email protected]> 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

2023-07-15 10:30:21

by Benno Lossin

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

On 14.07.23 11:19, Asahi Lina wrote:
> Trivial implementation.
>
> Signed-off-by: Asahi Lina <[email protected]>

With a better commit message you can add

Reviewed-by: Benno Lossin <[email protected]>

--
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
>

2023-10-25 16:19:05

by Ariel Miculas

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

On 23/07/14 05:01PM, Miguel Ojeda wrote:
> On Fri, Jul 14, 2023 at 4:02 PM Alice Ryhl <[email protected]> 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

2023-10-25 16:57:33

by Miguel Ojeda

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

On Wed, Oct 25, 2023 at 6:18 PM Ariel Miculas (amiculas)
<[email protected]> 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

2023-12-13 18:43:39

by Miguel Ojeda

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

On Fri, Jul 14, 2023 at 11:19 AM Asahi Lina <[email protected]> wrote:
>
> Trivial implementation.
>
> Signed-off-by: Asahi Lina <[email protected]>

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

Thanks everyone!

Cheers,
Miguel