This picks up from Michal Rostecki's work[0]. Per Michal's guidance I
have omitted Co-authored tags, as the end result is quite different.

Link: https://lore.kernel.org/rust-for-linux/20240819153656.28807-2-vadorovsky@protonmail.com/t/#u [0]
Closes: https://github.com/Rust-for-Linux/linux/issues/1075

Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
Changes in v8:
- Move `{from,as}_char_ptr` back to `CStrExt`. This reduces the diff
  some.
- Restore `from_bytes_with_nul_unchecked_mut`, `to_cstring`.
- Link to v7: https://lore.kernel.org/r/20250202-cstr-core-v7-0-da1802520438@gmail.com

Changes in v7:
- Rebased on mainline.
- Restore functionality added in commit a321f3ad0a5d ("rust: str: add
  {make,to}_{upper,lower}case() to CString").
- Used `diff.algorithm patience` to improve diff readability.
- Link to v6: https://lore.kernel.org/r/20250202-cstr-core-v6-0-8469cd6d29fd@gmail.com

Changes in v6:
- Split the work into several commits for ease of review.
- Restore `{from,as}_char_ptr` to allow building on ARM (see commit
  message).
- Add `CStrExt` to `kernel::prelude`. (Alice Ryhl)
- Remove `CStrExt::from_bytes_with_nul_unchecked_mut` and restore
  `DerefMut for CString`. (Alice Ryhl)
- Rename and hide `kernel::c_str!` to encourage use of C-String
  literals.
- Drop implementation and invocation changes in kunit.rs. (Trevor Gross)
- Drop docs on `Display` impl. (Trevor Gross)
- Rewrite docs in the style of the standard library.
- Restore the `test_cstr_debug` unit tests to demonstrate that the
  implementation has changed.

Changes in v5:
- Keep the `test_cstr_display*` unit tests.

Changes in v4:
- Provide the `CStrExt` trait with `display()` method, which returns a
   `CStrDisplay` wrapper with `Display` implementation. This addresses
   the lack of `Display` implementation for `core::ffi::CStr`.
- Provide `from_bytes_with_nul_unchecked_mut()` method in `CStrExt`,
   which might be useful and is going to prevent manual, unsafe casts.
- Fix a typo (s/preffered/prefered/).

Changes in v3:
- Fix the commit message.
- Remove redundant braces in `use`, when only one item is imported.

Changes in v2:
- Do not remove `c_str` macro. While it's preferred to use C-string
   literals, there are two cases where `c_str` is helpful:
   - When working with macros, which already return a Rust string literal
     (e.g. `stringify!`).
   - When building macros, where we want to take a Rust string literal as an
     argument (for caller's convenience), but still use it as a C-string
     internally.
- Use Rust literals as arguments in macros (`new_mutex`, `new_condvar`,
   `new_mutex`). Use the `c_str` macro to convert these literals to C-string
   literals.
- Use `c_str` in kunit.rs for converting the output of `stringify!` to a
   `CStr`.
- Remove `DerefMut` implementation for `CString`.

---
Tamir Duberstein (4):
      rust: move BStr,CStr Display impls behind method
      rust: replace `CStr` with `core::ffi::CStr`
      rust: replace `kernel::c_str!` with C-Strings
      rust: remove core::ffi::CStr reexport

 drivers/net/phy/ax88796b_rust.rs     |   7 +-
 drivers/net/phy/qt2025.rs            |   5 +-
 rust/kernel/device.rs                |   7 +-
 rust/kernel/devres.rs                |   2 +-
 rust/kernel/driver.rs                |   4 +-
 rust/kernel/error.rs                 |  10 +-
 rust/kernel/firmware.rs              |   7 +-
 rust/kernel/kunit.rs                 |  18 +-
 rust/kernel/lib.rs                   |   2 +-
 rust/kernel/miscdevice.rs            |   5 +-
 rust/kernel/net/phy.rs               |   8 +-
 rust/kernel/of.rs                    |   5 +-
 rust/kernel/pci.rs                   |   3 +-
 rust/kernel/platform.rs              |   7 +-
 rust/kernel/prelude.rs               |   2 +-
 rust/kernel/seq_file.rs              |   4 +-
 rust/kernel/str.rs                   | 542 +++++++++++++----------------------
 rust/kernel/sync.rs                  |   4 +-
 rust/kernel/sync/condvar.rs          |   3 +-
 rust/kernel/sync/lock.rs             |   4 +-
 rust/kernel/sync/lock/global.rs      |   6 +-
 rust/kernel/sync/poll.rs             |   1 +
 rust/kernel/workqueue.rs             |   1 +
 rust/macros/module.rs                |   2 +-
 samples/rust/rust_driver_pci.rs      |   4 +-
 samples/rust/rust_driver_platform.rs |   4 +-
 samples/rust/rust_misc_device.rs     |   3 +-
 27 files changed, 268 insertions(+), 402 deletions(-)
---
base-commit: 8b3f2116ea4a9521f852b7f9d1d6dd4d7ad86810
change-id: 20250201-cstr-core-d4b9b69120cf

Best regards,
-- 
Tamir Duberstein <tamird@gmail.com>

Gentle ping. Trevor, Alice, Benno: you all participated in the last
round of review - I'd appreciate it if you could take a look at this
series.

Cheers.
Tamir

On Mon, Feb 3, 2025 at 6:50 AM Tamir Duberstein <tamird@gmail.com> wrote:
>
> This picks up from Michal Rostecki's work[0]. Per Michal's guidance I
> have omitted Co-authored tags, as the end result is quite different.
>
> Link: https://lore.kernel.org/rust-for-linux/20240819153656.28807-2-vadorovsky@protonmail.com/t/#u [0]
> Closes: https://github.com/Rust-for-Linux/linux/issues/1075
>
> Signed-off-by: Tamir Duberstein <tamird@gmail.com>
> ---
> Changes in v8:
> - Move `{from,as}_char_ptr` back to `CStrExt`. This reduces the diff
>   some.
> - Restore `from_bytes_with_nul_unchecked_mut`, `to_cstring`.
> - Link to v7: https://lore.kernel.org/r/20250202-cstr-core-v7-0-da1802520438@gmail.com
>
> Changes in v7:
> - Rebased on mainline.
> - Restore functionality added in commit a321f3ad0a5d ("rust: str: add
>   {make,to}_{upper,lower}case() to CString").
> - Used `diff.algorithm patience` to improve diff readability.
> - Link to v6: https://lore.kernel.org/r/20250202-cstr-core-v6-0-8469cd6d29fd@gmail.com
>
> Changes in v6:
> - Split the work into several commits for ease of review.
> - Restore `{from,as}_char_ptr` to allow building on ARM (see commit
>   message).
> - Add `CStrExt` to `kernel::prelude`. (Alice Ryhl)
> - Remove `CStrExt::from_bytes_with_nul_unchecked_mut` and restore
>   `DerefMut for CString`. (Alice Ryhl)
> - Rename and hide `kernel::c_str!` to encourage use of C-String
>   literals.
> - Drop implementation and invocation changes in kunit.rs. (Trevor Gross)
> - Drop docs on `Display` impl. (Trevor Gross)
> - Rewrite docs in the style of the standard library.
> - Restore the `test_cstr_debug` unit tests to demonstrate that the
>   implementation has changed.
>
> Changes in v5:
> - Keep the `test_cstr_display*` unit tests.
>
> Changes in v4:
> - Provide the `CStrExt` trait with `display()` method, which returns a
>    `CStrDisplay` wrapper with `Display` implementation. This addresses
>    the lack of `Display` implementation for `core::ffi::CStr`.
> - Provide `from_bytes_with_nul_unchecked_mut()` method in `CStrExt`,
>    which might be useful and is going to prevent manual, unsafe casts.
> - Fix a typo (s/preffered/prefered/).
>
> Changes in v3:
> - Fix the commit message.
> - Remove redundant braces in `use`, when only one item is imported.
>
> Changes in v2:
> - Do not remove `c_str` macro. While it's preferred to use C-string
>    literals, there are two cases where `c_str` is helpful:
>    - When working with macros, which already return a Rust string literal
>      (e.g. `stringify!`).
>    - When building macros, where we want to take a Rust string literal as an
>      argument (for caller's convenience), but still use it as a C-string
>      internally.
> - Use Rust literals as arguments in macros (`new_mutex`, `new_condvar`,
>    `new_mutex`). Use the `c_str` macro to convert these literals to C-string
>    literals.
> - Use `c_str` in kunit.rs for converting the output of `stringify!` to a
>    `CStr`.
> - Remove `DerefMut` implementation for `CString`.
>
> ---
> Tamir Duberstein (4):
>       rust: move BStr,CStr Display impls behind method
>       rust: replace `CStr` with `core::ffi::CStr`
>       rust: replace `kernel::c_str!` with C-Strings
>       rust: remove core::ffi::CStr reexport
>
>  drivers/net/phy/ax88796b_rust.rs     |   7 +-
>  drivers/net/phy/qt2025.rs            |   5 +-
>  rust/kernel/device.rs                |   7 +-
>  rust/kernel/devres.rs                |   2 +-
>  rust/kernel/driver.rs                |   4 +-
>  rust/kernel/error.rs                 |  10 +-
>  rust/kernel/firmware.rs              |   7 +-
>  rust/kernel/kunit.rs                 |  18 +-
>  rust/kernel/lib.rs                   |   2 +-
>  rust/kernel/miscdevice.rs            |   5 +-
>  rust/kernel/net/phy.rs               |   8 +-
>  rust/kernel/of.rs                    |   5 +-
>  rust/kernel/pci.rs                   |   3 +-
>  rust/kernel/platform.rs              |   7 +-
>  rust/kernel/prelude.rs               |   2 +-
>  rust/kernel/seq_file.rs              |   4 +-
>  rust/kernel/str.rs                   | 542 +++++++++++++----------------------
>  rust/kernel/sync.rs                  |   4 +-
>  rust/kernel/sync/condvar.rs          |   3 +-
>  rust/kernel/sync/lock.rs             |   4 +-
>  rust/kernel/sync/lock/global.rs      |   6 +-
>  rust/kernel/sync/poll.rs             |   1 +
>  rust/kernel/workqueue.rs             |   1 +
>  rust/macros/module.rs                |   2 +-
>  samples/rust/rust_driver_pci.rs      |   4 +-
>  samples/rust/rust_driver_platform.rs |   4 +-
>  samples/rust/rust_misc_device.rs     |   3 +-
>  27 files changed, 268 insertions(+), 402 deletions(-)
> ---
> base-commit: 8b3f2116ea4a9521f852b7f9d1d6dd4d7ad86810
> change-id: 20250201-cstr-core-d4b9b69120cf
>
> Best regards,
> --
> Tamir Duberstein <tamird@gmail.com>
>

On Tue, Feb 18, 2025 at 5:05 PM Tamir Duberstein <tamird@gmail.com> wrote:
>
> Gentle ping. Trevor, Alice, Benno: you all participated in the last
> round of review - I'd appreciate it if you could take a look at this
> series.

The primary thing that comes to mind looking at this is that losing
the Display impl is pretty sad. Having to jump through hoops every
time you want to print a string isn't great :(

Alice

On Wed, 19 Feb 2025 14:21:35 +0000
Alice Ryhl <aliceryhl@google.com> wrote:

> On Tue, Feb 18, 2025 at 5:05 PM Tamir Duberstein <tamird@gmail.com> wrote:
> >
> > Gentle ping. Trevor, Alice, Benno: you all participated in the last
> > round of review - I'd appreciate it if you could take a look at this
> > series.  
> 
> The primary thing that comes to mind looking at this is that losing
> the Display impl is pretty sad. Having to jump through hoops every
> time you want to print a string isn't great :(
> 
> Alice

I'd want to add that we currently also have our own `BStr` and then we
have the `CStr` -> `BStr` -> `[u8]` deref chain which is quite often
useful. If we move to the `core::ffi::CStr` then we would lose ability
to do so.

The `BStr` is quite useful as a thin layer on `[u8]` to give a
semantical meaning that this is supposed to be printable, user-facing
string, but it isn't always UTF-8.

Best,
Gary

On Fri, Feb 21, 2025 at 9:28 AM Gary Guo <gary@garyguo.net> wrote:
>
> I'd want to add that we currently also have our own `BStr` and then we
> have the `CStr` -> `BStr` -> `[u8]` deref chain which is quite often
> useful. If we move to the `core::ffi::CStr` then we would lose ability
> to do so.

True. The best we could do is `impl AsRef<BStr> for CStr`, I think.

On Fri, Feb 21, 2025 at 10:59 AM Tamir Duberstein <tamird@gmail.com> wrote:
>
> On Fri, Feb 21, 2025 at 9:28 AM Gary Guo <gary@garyguo.net> wrote:
> >
> > I'd want to add that we currently also have our own `BStr` and then we
> > have the `CStr` -> `BStr` -> `[u8]` deref chain which is quite often
> > useful. If we move to the `core::ffi::CStr` then we would lose ability
> > to do so.
>
> True. The best we could do is `impl AsRef<BStr> for CStr`, I think.

BStr now exists upstream (though it is unstable for now). Please see
https://github.com/Rust-for-Linux/linux/issues/1146.

I sent https://github.com/rust-lang/rust/pull/138498 to see whether it
is possible to retain this functionality in the end state where
upstream's BStr is stable and we're able to use it.

On Wed, Feb 19, 2025 at 9:21 AM Alice Ryhl <aliceryhl@google.com> wrote:
>
> On Tue, Feb 18, 2025 at 5:05 PM Tamir Duberstein <tamird@gmail.com> wrote:
> >
> > Gentle ping. Trevor, Alice, Benno: you all participated in the last
> > round of review - I'd appreciate it if you could take a look at this
> > series.
>
> The primary thing that comes to mind looking at this is that losing
> the Display impl is pretty sad. Having to jump through hoops every
> time you want to print a string isn't great :(

There's the practical answer and the philosophical one. The former is
that core::ffi::CStr doesn't impl Display. The latter is that Display
implementations aren't meant to be lossy, and we shouldn't make it
ergonomic to do things that might surprise the user.

We could add our own UnicodeCStr which could impl Display and be
AsRef<CStr>. Do you think that should gate this work?

On Wed, Feb 19, 2025 at 2:33 PM Tamir Duberstein <tamird@gmail.com> wrote:
>
> On Wed, Feb 19, 2025 at 9:21 AM Alice Ryhl <aliceryhl@google.com> wrote:
> >
> > On Tue, Feb 18, 2025 at 5:05 PM Tamir Duberstein <tamird@gmail.com> wrote:
> > >
> > > Gentle ping. Trevor, Alice, Benno: you all participated in the last
> > > round of review - I'd appreciate it if you could take a look at this
> > > series.
> >
> > The primary thing that comes to mind looking at this is that losing
> > the Display impl is pretty sad. Having to jump through hoops every
> > time you want to print a string isn't great :(
>
> There's the practical answer and the philosophical one. The former is
> that core::ffi::CStr doesn't impl Display. The latter is that Display
> implementations aren't meant to be lossy, and we shouldn't make it
> ergonomic to do things that might surprise the user.

I don't think there's any problem with a lossy Display impl. It's
supposed to be user-facing, and that's more or less what it requires.
Does kernel printing even require the string to be utf-8 in the first
place? I guess there's a mismatch about that here.

> We could add our own UnicodeCStr which could impl Display and be
> AsRef<CStr>. Do you think that should gate this work?

The solution I actually want is just for CStr to be Display, but
that's not something we can control on the kernel-side. I won't block
this change over it.

Alice