* [PATCH v8 0/4] rust: replace kernel::str::CStr w/ core::ffi::CStr
@ 2025-02-03 11:50 Tamir Duberstein
2025-02-03 11:50 ` [PATCH v8 1/4] rust: move BStr,CStr Display impls behind method Tamir Duberstein
` (4 more replies)
0 siblings, 5 replies; 12+ messages in thread
From: Tamir Duberstein @ 2025-02-03 11:50 UTC (permalink / raw)
To: Michal Rostecki, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross
Cc: rust-for-linux, linux-kernel, Tamir Duberstein
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>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v8 1/4] rust: move BStr,CStr Display impls behind method
2025-02-03 11:50 [PATCH v8 0/4] rust: replace kernel::str::CStr w/ core::ffi::CStr Tamir Duberstein
@ 2025-02-03 11:50 ` Tamir Duberstein
2025-02-03 11:50 ` [PATCH v8 2/4] rust: replace `CStr` with `core::ffi::CStr` Tamir Duberstein
` (3 subsequent siblings)
4 siblings, 0 replies; 12+ messages in thread
From: Tamir Duberstein @ 2025-02-03 11:50 UTC (permalink / raw)
To: Michal Rostecki, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross
Cc: rust-for-linux, linux-kernel, Tamir Duberstein
There are two reasons for doing this:
- Moving the Display impl behind a display method matches the style used
in the standard library for printing non-Unicode data.
- The standard library's core::ffi::CStr doesn't implement Display;
moving Display to a helper struct is a necessary step toward using it.
Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
rust/kernel/kunit.rs | 9 ++--
rust/kernel/str.rs | 121 +++++++++++++++++++++++++++++++++++----------------
2 files changed, 90 insertions(+), 40 deletions(-)
diff --git a/rust/kernel/kunit.rs b/rust/kernel/kunit.rs
index 824da0e9738a..630b947c708c 100644
--- a/rust/kernel/kunit.rs
+++ b/rust/kernel/kunit.rs
@@ -56,6 +56,7 @@ macro_rules! kunit_assert {
break 'out;
}
+ static NAME: &'static $crate::str::CStr = $crate::c_str!($name);
static FILE: &'static $crate::str::CStr = $crate::c_str!($file);
static LINE: i32 = core::line!() as i32 - $diff;
static CONDITION: &'static $crate::str::CStr = $crate::c_str!(stringify!($condition));
@@ -71,11 +72,13 @@ macro_rules! kunit_assert {
//
// This mimics KUnit's failed assertion format.
$crate::kunit::err(format_args!(
- " # {}: ASSERTION FAILED at {FILE}:{LINE}\n",
- $name
+ " # {NAME}: ASSERTION FAILED at {FILE}:{LINE}\n",
+ NAME = NAME.display(),
+ FILE = FILE.display(),
));
$crate::kunit::err(format_args!(
- " Expected {CONDITION} to be true, but is false\n"
+ " Expected {CONDITION} to be true, but is false\n",
+ CONDITION = CONDITION.display(),
));
$crate::kunit::err(format_args!(
" Failure not reported to KUnit since this is a non-KUnit task\n"
diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
index 28e2201604d6..c273f1367607 100644
--- a/rust/kernel/str.rs
+++ b/rust/kernel/str.rs
@@ -31,29 +31,77 @@ pub const fn from_bytes(bytes: &[u8]) -> &Self {
// SAFETY: `BStr` is transparent to `[u8]`.
unsafe { &*(bytes as *const [u8] as *const BStr) }
}
-}
-impl fmt::Display for BStr {
- /// Formats printable ASCII characters, escaping the rest.
+ /// Returns an object that implements [`Display`] for safely printing a [`BStr`] that may
+ /// contain non-Unicode data. If you would like an implementation which escapes the [`BStr`]
+ /// please use [`Debug`] instead.
+ ///
+ /// [`Display`]: fmt::Display
+ /// [`Debug`]: fmt::Debug
+ ///
+ /// # Examples
///
/// ```
- /// # use kernel::{fmt, b_str, str::{BStr, CString}};
+ /// # use kernel::{fmt, b_str, str::CString};
/// let ascii = b_str!("Hello, BStr!");
- /// let s = CString::try_from_fmt(fmt!("{}", ascii))?;
+ /// let s = CString::try_from_fmt(fmt!("{}", ascii.display()))?;
/// assert_eq!(s.as_bytes(), "Hello, BStr!".as_bytes());
///
/// let non_ascii = b_str!("🦀");
- /// let s = CString::try_from_fmt(fmt!("{}", non_ascii))?;
+ /// let s = CString::try_from_fmt(fmt!("{}", non_ascii.display()))?;
/// assert_eq!(s.as_bytes(), "\\xf0\\x9f\\xa6\\x80".as_bytes());
/// # Ok::<(), kernel::error::Error>(())
/// ```
+ #[inline]
+ pub fn display(&self) -> Display<'_> {
+ Display {
+ inner: self,
+ escape_common: true,
+ }
+ }
+}
+
+/// Helper struct for safely printing a [`BStr`] with [`fmt!`] and `{}`.
+///
+/// A [`BStr`] might contain non-Unicode data. This `struct` implements the [`Display`] trait in a
+/// way that mitigates that. It is created by the [`display`](BStr::display) method on [`BStr`].
+///
+/// If you would like an implementation which escapes the string please use [`Debug`] instead.
+///
+/// # Examples
+///
+/// ```
+/// # use kernel::{fmt, b_str, str::CString};
+/// let ascii = b_str!("Hello, BStr!");
+/// let s = CString::try_from_fmt(fmt!("{}", ascii.display()))?;
+/// assert_eq!(s.as_bytes(), "Hello, BStr!".as_bytes());
+///
+/// let non_ascii = b_str!("🦀");
+/// let s = CString::try_from_fmt(fmt!("{}", non_ascii.display()))?;
+/// assert_eq!(s.as_bytes(), "\\xf0\\x9f\\xa6\\x80".as_bytes());
+/// # Ok::<(), kernel::error::Error>(())
+/// ```
+///
+/// [`fmt!`]: crate::fmt
+/// [`Debug`]: fmt::Debug
+/// [`Display`]: fmt::Display
+pub struct Display<'a> {
+ inner: &'a BStr,
+ escape_common: bool,
+}
+
+impl fmt::Display for Display<'_> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
- for &b in &self.0 {
+ let Self {
+ inner: BStr(b),
+ escape_common,
+ } = self;
+ for &b in b {
match b {
// Common escape codes.
- b'\t' => f.write_str("\\t")?,
- b'\n' => f.write_str("\\n")?,
- b'\r' => f.write_str("\\r")?,
+ b'\t' if *escape_common => f.write_str("\\t")?,
+ b'\n' if *escape_common => f.write_str("\\n")?,
+ b'\r' if *escape_common => f.write_str("\\r")?,
// Printable characters.
0x20..=0x7e => f.write_char(b as char)?,
_ => write!(f, "\\x{:02x}", b)?,
@@ -68,7 +116,7 @@ impl fmt::Debug for BStr {
/// escaping the rest.
///
/// ```
- /// # use kernel::{fmt, b_str, str::{BStr, CString}};
+ /// # use kernel::{fmt, b_str, str::CString};
/// // Embedded double quotes are escaped.
/// let ascii = b_str!("Hello, \"BStr\"!");
/// let s = CString::try_from_fmt(fmt!("{:?}", ascii))?;
@@ -376,35 +424,35 @@ pub fn to_ascii_uppercase(&self) -> Result<CString, AllocError> {
Ok(s)
}
-}
-impl fmt::Display for CStr {
- /// Formats printable ASCII characters, escaping the rest.
+ /// Returns an object that implements [`Display`] for safely printing a [`CStr`] that may
+ /// contain non-Unicode data. If you would like an implementation which escapes the [`CStr`]
+ /// please use [`Debug`] instead.
+ ///
+ /// [`Display`]: fmt::Display
+ /// [`Debug`]: fmt::Debug
+ ///
+ /// # Examples
///
/// ```
/// # use kernel::c_str;
/// # use kernel::fmt;
- /// # use kernel::str::CStr;
/// # use kernel::str::CString;
/// let penguin = c_str!("🐧");
- /// let s = CString::try_from_fmt(fmt!("{}", penguin))?;
+ /// let s = CString::try_from_fmt(fmt!("{}", penguin.display()))?;
/// assert_eq!(s.as_bytes_with_nul(), "\\xf0\\x9f\\x90\\xa7\0".as_bytes());
///
/// let ascii = c_str!("so \"cool\"");
- /// let s = CString::try_from_fmt(fmt!("{}", ascii))?;
+ /// let s = CString::try_from_fmt(fmt!("{}", ascii.display()))?;
/// assert_eq!(s.as_bytes_with_nul(), "so \"cool\"\0".as_bytes());
/// # Ok::<(), kernel::error::Error>(())
/// ```
- fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
- for &c in self.as_bytes() {
- if (0x20..0x7f).contains(&c) {
- // Printable character.
- f.write_char(c as char)?;
- } else {
- write!(f, "\\x{:02x}", c)?;
- }
+ #[inline]
+ pub fn display(&self) -> Display<'_> {
+ Display {
+ inner: self,
+ escape_common: false,
}
- Ok(())
}
}
@@ -414,7 +462,6 @@ impl fmt::Debug for CStr {
/// ```
/// # use kernel::c_str;
/// # use kernel::fmt;
- /// # use kernel::str::CStr;
/// # use kernel::str::CString;
/// let penguin = c_str!("🐧");
/// let s = CString::try_from_fmt(fmt!("{:?}", penguin))?;
@@ -595,13 +642,13 @@ fn test_cstr_as_str_unchecked() {
#[test]
fn test_cstr_display() {
let hello_world = CStr::from_bytes_with_nul(b"hello, world!\0").unwrap();
- assert_eq!(format!("{}", hello_world), "hello, world!");
+ assert_eq!(format!("{}", hello_world.display()), "hello, world!");
let non_printables = CStr::from_bytes_with_nul(b"\x01\x09\x0a\0").unwrap();
- assert_eq!(format!("{}", non_printables), "\\x01\\x09\\x0a");
+ assert_eq!(format!("{}", non_printables.display()), "\\x01\\x09\\x0a");
let non_ascii = CStr::from_bytes_with_nul(b"d\xe9j\xe0 vu\0").unwrap();
- assert_eq!(format!("{}", non_ascii), "d\\xe9j\\xe0 vu");
+ assert_eq!(format!("{}", non_ascii.display()), "d\\xe9j\\xe0 vu");
let good_bytes = CStr::from_bytes_with_nul(b"\xf0\x9f\xa6\x80\0").unwrap();
- assert_eq!(format!("{}", good_bytes), "\\xf0\\x9f\\xa6\\x80");
+ assert_eq!(format!("{}", good_bytes.display()), "\\xf0\\x9f\\xa6\\x80");
}
#[test]
@@ -612,7 +659,7 @@ fn test_cstr_display_all_bytes() {
bytes[i as usize] = i.wrapping_add(1);
}
let cstr = CStr::from_bytes_with_nul(&bytes).unwrap();
- assert_eq!(format!("{}", cstr), ALL_ASCII_CHARS);
+ assert_eq!(format!("{}", cstr.display()), ALL_ASCII_CHARS);
}
#[test]
@@ -630,15 +677,15 @@ fn test_cstr_debug() {
#[test]
fn test_bstr_display() {
let hello_world = BStr::from_bytes(b"hello, world!");
- assert_eq!(format!("{}", hello_world), "hello, world!");
+ assert_eq!(format!("{}", hello_world.display()), "hello, world!");
let escapes = BStr::from_bytes(b"_\t_\n_\r_\\_\'_\"_");
- assert_eq!(format!("{}", escapes), "_\\t_\\n_\\r_\\_'_\"_");
+ assert_eq!(format!("{}", escapes.display()), "_\\t_\\n_\\r_\\_'_\"_");
let others = BStr::from_bytes(b"\x01");
- assert_eq!(format!("{}", others), "\\x01");
+ assert_eq!(format!("{}", others.display()), "\\x01");
let non_ascii = BStr::from_bytes(b"d\xe9j\xe0 vu");
- assert_eq!(format!("{}", non_ascii), "d\\xe9j\\xe0 vu");
+ assert_eq!(format!("{}", non_ascii.display()), "d\\xe9j\\xe0 vu");
let good_bytes = BStr::from_bytes(b"\xf0\x9f\xa6\x80");
- assert_eq!(format!("{}", good_bytes), "\\xf0\\x9f\\xa6\\x80");
+ assert_eq!(format!("{}", good_bytes.display()), "\\xf0\\x9f\\xa6\\x80");
}
#[test]
--
2.48.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v8 2/4] rust: replace `CStr` with `core::ffi::CStr`
2025-02-03 11:50 [PATCH v8 0/4] rust: replace kernel::str::CStr w/ core::ffi::CStr Tamir Duberstein
2025-02-03 11:50 ` [PATCH v8 1/4] rust: move BStr,CStr Display impls behind method Tamir Duberstein
@ 2025-02-03 11:50 ` Tamir Duberstein
2025-02-03 11:50 ` [PATCH v8 3/4] rust: replace `kernel::c_str!` with C-Strings Tamir Duberstein
` (2 subsequent siblings)
4 siblings, 0 replies; 12+ messages in thread
From: Tamir Duberstein @ 2025-02-03 11:50 UTC (permalink / raw)
To: Michal Rostecki, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross
Cc: rust-for-linux, linux-kernel, Tamir Duberstein
`std::ffi::CStr` was moved to `core::ffi::CStr` in Rust 1.64. Replace
`kernel::str::CStr` with `core::ffi::CStr` now that we can.
C-String literals were added in Rust 1.77. Opportunistically replace
instances of `kernel::c_str!` with C-String literals where other code
changes were already necessary; the rest will be done in a later commit.
Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
rust/kernel/device.rs | 4 +-
rust/kernel/error.rs | 4 +-
rust/kernel/kunit.rs | 4 +-
rust/kernel/miscdevice.rs | 2 +-
rust/kernel/net/phy.rs | 2 +-
rust/kernel/of.rs | 2 +-
rust/kernel/prelude.rs | 5 +-
rust/kernel/seq_file.rs | 4 +-
rust/kernel/str.rs | 407 +++++++++++-----------------------------
rust/kernel/sync/condvar.rs | 2 +-
rust/kernel/sync/lock.rs | 2 +-
rust/kernel/sync/lock/global.rs | 2 +-
12 files changed, 126 insertions(+), 314 deletions(-)
diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs
index db2d9658ba47..df4bfa5f51ea 100644
--- a/rust/kernel/device.rs
+++ b/rust/kernel/device.rs
@@ -12,7 +12,7 @@
use core::{fmt, ptr};
#[cfg(CONFIG_PRINTK)]
-use crate::c_str;
+use crate::str::CStrExt as _;
/// A reference-counted device.
///
@@ -176,7 +176,7 @@ unsafe fn printk(&self, klevel: &[u8], msg: fmt::Arguments<'_>) {
bindings::_dev_printk(
klevel as *const _ as *const crate::ffi::c_char,
self.as_raw(),
- c_str!("%pA").as_char_ptr(),
+ c"%pA".as_char_ptr(),
&msg as *const _ as *const crate::ffi::c_void,
)
};
diff --git a/rust/kernel/error.rs b/rust/kernel/error.rs
index f6ecf09cb65f..cff9204d9dd6 100644
--- a/rust/kernel/error.rs
+++ b/rust/kernel/error.rs
@@ -163,6 +163,8 @@ pub fn name(&self) -> Option<&'static CStr> {
if ptr.is_null() {
None
} else {
+ use crate::str::CStrExt as _;
+
// SAFETY: The string returned by `errname` is static and `NUL`-terminated.
Some(unsafe { CStr::from_char_ptr(ptr) })
}
@@ -187,7 +189,7 @@ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
Some(name) => f
.debug_tuple(
// SAFETY: These strings are ASCII-only.
- unsafe { core::str::from_utf8_unchecked(name) },
+ unsafe { core::str::from_utf8_unchecked(name.to_bytes()) },
)
.finish(),
}
diff --git a/rust/kernel/kunit.rs b/rust/kernel/kunit.rs
index 630b947c708c..d858757aeace 100644
--- a/rust/kernel/kunit.rs
+++ b/rust/kernel/kunit.rs
@@ -101,12 +101,12 @@ unsafe impl Sync for Location {}
unsafe impl Sync for UnaryAssert {}
static LOCATION: Location = Location($crate::bindings::kunit_loc {
- file: FILE.as_char_ptr(),
+ file: $crate::str::as_char_ptr_in_const_context(FILE),
line: LINE,
});
static ASSERTION: UnaryAssert = UnaryAssert($crate::bindings::kunit_unary_assert {
assert: $crate::bindings::kunit_assert {},
- condition: CONDITION.as_char_ptr(),
+ condition: $crate::str::as_char_ptr_in_const_context(CONDITION),
expected_true: true,
});
diff --git a/rust/kernel/miscdevice.rs b/rust/kernel/miscdevice.rs
index e14433b2ab9d..78c150270080 100644
--- a/rust/kernel/miscdevice.rs
+++ b/rust/kernel/miscdevice.rs
@@ -34,7 +34,7 @@ pub const fn into_raw<T: MiscDevice>(self) -> bindings::miscdevice {
// SAFETY: All zeros is valid for this C type.
let mut result: bindings::miscdevice = unsafe { MaybeUninit::zeroed().assume_init() };
result.minor = bindings::MISC_DYNAMIC_MINOR as _;
- result.name = self.name.as_char_ptr();
+ result.name = crate::str::as_char_ptr_in_const_context(self.name);
result.fops = create_vtable::<T>();
result
}
diff --git a/rust/kernel/net/phy.rs b/rust/kernel/net/phy.rs
index bb654a28dab3..73df0f55f94e 100644
--- a/rust/kernel/net/phy.rs
+++ b/rust/kernel/net/phy.rs
@@ -504,7 +504,7 @@ unsafe impl Sync for DriverVTable {}
pub const fn create_phy_driver<T: Driver>() -> DriverVTable {
// INVARIANT: All the fields of `struct phy_driver` are initialized properly.
DriverVTable(Opaque::new(bindings::phy_driver {
- name: T::NAME.as_char_ptr().cast_mut(),
+ name: crate::str::as_char_ptr_in_const_context(T::NAME),
flags: T::FLAGS,
phy_id: T::PHY_DEVICE_ID.id,
phy_id_mask: T::PHY_DEVICE_ID.mask_as_int(),
diff --git a/rust/kernel/of.rs b/rust/kernel/of.rs
index 04f2d8ef29cb..12ea65df46de 100644
--- a/rust/kernel/of.rs
+++ b/rust/kernel/of.rs
@@ -29,7 +29,7 @@ fn index(&self) -> usize {
impl DeviceId {
/// Create a new device id from an OF 'compatible' string.
pub const fn new(compatible: &'static CStr) -> Self {
- let src = compatible.as_bytes_with_nul();
+ let src = compatible.to_bytes_with_nul();
// Replace with `bindings::of_device_id::default()` once stabilized for `const`.
// SAFETY: FFI type is valid to be zero-initialized.
let mut of: bindings::of_device_id = unsafe { core::mem::zeroed() };
diff --git a/rust/kernel/prelude.rs b/rust/kernel/prelude.rs
index dde2e0649790..96e7029c27da 100644
--- a/rust/kernel/prelude.rs
+++ b/rust/kernel/prelude.rs
@@ -34,7 +34,10 @@
pub use super::error::{code::*, Error, Result};
-pub use super::{str::CStr, ThisModule};
+pub use super::{
+ str::{CStr, CStrExt as _},
+ ThisModule,
+};
pub use super::init::{InPlaceInit, InPlaceWrite, Init, PinInit};
diff --git a/rust/kernel/seq_file.rs b/rust/kernel/seq_file.rs
index 04947c672979..073a38e6991f 100644
--- a/rust/kernel/seq_file.rs
+++ b/rust/kernel/seq_file.rs
@@ -4,7 +4,7 @@
//!
//! C header: [`include/linux/seq_file.h`](srctree/include/linux/seq_file.h)
-use crate::{bindings, c_str, types::NotThreadSafe, types::Opaque};
+use crate::{bindings, str::CStrExt as _, types::NotThreadSafe, types::Opaque};
/// A utility for generating the contents of a seq file.
#[repr(transparent)]
@@ -35,7 +35,7 @@ pub fn call_printf(&self, args: core::fmt::Arguments<'_>) {
unsafe {
bindings::seq_printf(
self.inner.get(),
- c_str!("%pA").as_char_ptr(),
+ c"%pA".as_char_ptr(),
&args as *const _ as *const crate::ffi::c_void,
);
}
diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
index c273f1367607..13dd2f6cc572 100644
--- a/rust/kernel/str.rs
+++ b/rust/kernel/str.rs
@@ -4,7 +4,7 @@
use crate::alloc::{flags::*, AllocError, KVec};
use core::fmt::{self, Write};
-use core::ops::{self, Deref, DerefMut, Index};
+use core::ops::{Deref, DerefMut};
use crate::error::{code::*, Error};
@@ -45,11 +45,11 @@ pub const fn from_bytes(bytes: &[u8]) -> &Self {
/// # use kernel::{fmt, b_str, str::CString};
/// let ascii = b_str!("Hello, BStr!");
/// let s = CString::try_from_fmt(fmt!("{}", ascii.display()))?;
- /// assert_eq!(s.as_bytes(), "Hello, BStr!".as_bytes());
+ /// assert_eq!(s.to_bytes(), "Hello, BStr!".as_bytes());
///
/// let non_ascii = b_str!("🦀");
/// let s = CString::try_from_fmt(fmt!("{}", non_ascii.display()))?;
- /// assert_eq!(s.as_bytes(), "\\xf0\\x9f\\xa6\\x80".as_bytes());
+ /// assert_eq!(s.to_bytes(), "\\xf0\\x9f\\xa6\\x80".as_bytes());
/// # Ok::<(), kernel::error::Error>(())
/// ```
#[inline]
@@ -74,11 +74,11 @@ pub fn display(&self) -> Display<'_> {
/// # use kernel::{fmt, b_str, str::CString};
/// let ascii = b_str!("Hello, BStr!");
/// let s = CString::try_from_fmt(fmt!("{}", ascii.display()))?;
-/// assert_eq!(s.as_bytes(), "Hello, BStr!".as_bytes());
+/// assert_eq!(s.to_bytes(), "Hello, BStr!".as_bytes());
///
/// let non_ascii = b_str!("🦀");
/// let s = CString::try_from_fmt(fmt!("{}", non_ascii.display()))?;
-/// assert_eq!(s.as_bytes(), "\\xf0\\x9f\\xa6\\x80".as_bytes());
+/// assert_eq!(s.to_bytes(), "\\xf0\\x9f\\xa6\\x80".as_bytes());
/// # Ok::<(), kernel::error::Error>(())
/// ```
///
@@ -120,11 +120,11 @@ impl fmt::Debug for BStr {
/// // Embedded double quotes are escaped.
/// let ascii = b_str!("Hello, \"BStr\"!");
/// let s = CString::try_from_fmt(fmt!("{:?}", ascii))?;
- /// assert_eq!(s.as_bytes(), "\"Hello, \\\"BStr\\\"!\"".as_bytes());
+ /// assert_eq!(s.to_bytes(), "\"Hello, \\\"BStr\\\"!\"".as_bytes());
///
/// let non_ascii = b_str!("😺");
/// let s = CString::try_from_fmt(fmt!("{:?}", non_ascii))?;
- /// assert_eq!(s.as_bytes(), "\"\\xf0\\x9f\\x98\\xba\"".as_bytes());
+ /// assert_eq!(s.to_bytes(), "\"\\xf0\\x9f\\x98\\xba\"".as_bytes());
/// # Ok::<(), kernel::error::Error>(())
/// ```
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
@@ -177,55 +177,19 @@ macro_rules! b_str {
}};
}
-/// Possible errors when using conversion functions in [`CStr`].
-#[derive(Debug, Clone, Copy)]
-pub enum CStrConvertError {
- /// Supplied bytes contain an interior `NUL`.
- InteriorNul,
+pub use core::ffi::CStr;
- /// Supplied bytes are not terminated by `NUL`.
- NotNulTerminated,
+/// Returns a C pointer to the string.
+// It is a free function rather than a method on an extension trait because:
+//
+// - error[E0379]: functions in trait impls cannot be declared const
+#[inline]
+pub const fn as_char_ptr_in_const_context(c_str: &CStr) -> *const crate::ffi::c_char {
+ c_str.as_ptr().cast()
}
-impl From<CStrConvertError> for Error {
- #[inline]
- fn from(_: CStrConvertError) -> Error {
- EINVAL
- }
-}
-
-/// A string that is guaranteed to have exactly one `NUL` byte, which is at the
-/// end.
-///
-/// Used for interoperability with kernel APIs that take C strings.
-#[repr(transparent)]
-pub struct CStr([u8]);
-
-impl CStr {
- /// Returns the length of this string excluding `NUL`.
- #[inline]
- pub const fn len(&self) -> usize {
- self.len_with_nul() - 1
- }
-
- /// Returns the length of this string with `NUL`.
- #[inline]
- pub const fn len_with_nul(&self) -> usize {
- if self.0.is_empty() {
- // SAFETY: This is one of the invariant of `CStr`.
- // We add a `unreachable_unchecked` here to hint the optimizer that
- // the value returned from this function is non-zero.
- unsafe { core::hint::unreachable_unchecked() };
- }
- self.0.len()
- }
-
- /// Returns `true` if the string only includes `NUL`.
- #[inline]
- pub const fn is_empty(&self) -> bool {
- self.len() == 0
- }
-
+/// Extensions to [`CStr`].
+pub trait CStrExt {
/// Wraps a raw C string pointer.
///
/// # Safety
@@ -233,54 +197,9 @@ pub const fn is_empty(&self) -> bool {
/// `ptr` must be a valid pointer to a `NUL`-terminated C string, and it must
/// last at least `'a`. When `CStr` is alive, the memory pointed by `ptr`
/// must not be mutated.
- #[inline]
- pub unsafe fn from_char_ptr<'a>(ptr: *const crate::ffi::c_char) -> &'a Self {
- // SAFETY: The safety precondition guarantees `ptr` is a valid pointer
- // to a `NUL`-terminated C string.
- let len = unsafe { bindings::strlen(ptr) } + 1;
- // SAFETY: Lifetime guaranteed by the safety precondition.
- let bytes = unsafe { core::slice::from_raw_parts(ptr as _, len) };
- // SAFETY: As `len` is returned by `strlen`, `bytes` does not contain interior `NUL`.
- // As we have added 1 to `len`, the last byte is known to be `NUL`.
- unsafe { Self::from_bytes_with_nul_unchecked(bytes) }
- }
-
- /// Creates a [`CStr`] from a `[u8]`.
- ///
- /// The provided slice must be `NUL`-terminated, does not contain any
- /// interior `NUL` bytes.
- pub const fn from_bytes_with_nul(bytes: &[u8]) -> Result<&Self, CStrConvertError> {
- if bytes.is_empty() {
- return Err(CStrConvertError::NotNulTerminated);
- }
- if bytes[bytes.len() - 1] != 0 {
- return Err(CStrConvertError::NotNulTerminated);
- }
- let mut i = 0;
- // `i + 1 < bytes.len()` allows LLVM to optimize away bounds checking,
- // while it couldn't optimize away bounds checks for `i < bytes.len() - 1`.
- while i + 1 < bytes.len() {
- if bytes[i] == 0 {
- return Err(CStrConvertError::InteriorNul);
- }
- i += 1;
- }
- // SAFETY: We just checked that all properties hold.
- Ok(unsafe { Self::from_bytes_with_nul_unchecked(bytes) })
- }
-
- /// Creates a [`CStr`] from a `[u8]` without performing any additional
- /// checks.
- ///
- /// # Safety
- ///
- /// `bytes` *must* end with a `NUL` byte, and should only have a single
- /// `NUL` byte (or the string will be truncated).
- #[inline]
- pub const unsafe fn from_bytes_with_nul_unchecked(bytes: &[u8]) -> &CStr {
- // SAFETY: Properties of `bytes` guaranteed by the safety precondition.
- unsafe { core::mem::transmute(bytes) }
- }
+ // This function exists to paper over the fact that `CStr::from_ptr` takes a `*const
+ // core::ffi::c_char` rather than a `*const crate::ffi::c_char`.
+ unsafe fn from_char_ptr<'a>(ptr: *const crate::ffi::c_char) -> &'a Self;
/// Creates a mutable [`CStr`] from a `[u8]` without performing any
/// additional checks.
@@ -289,77 +208,16 @@ pub const fn from_bytes_with_nul(bytes: &[u8]) -> Result<&Self, CStrConvertError
///
/// `bytes` *must* end with a `NUL` byte, and should only have a single
/// `NUL` byte (or the string will be truncated).
- #[inline]
- pub unsafe fn from_bytes_with_nul_unchecked_mut(bytes: &mut [u8]) -> &mut CStr {
- // SAFETY: Properties of `bytes` guaranteed by the safety precondition.
- unsafe { &mut *(bytes as *mut [u8] as *mut CStr) }
- }
+ unsafe fn from_bytes_with_nul_unchecked_mut(bytes: &mut [u8]) -> &mut Self;
/// Returns a C pointer to the string.
- #[inline]
- pub const fn as_char_ptr(&self) -> *const crate::ffi::c_char {
- self.0.as_ptr()
- }
-
- /// Convert the string to a byte slice without the trailing `NUL` byte.
- #[inline]
- pub fn as_bytes(&self) -> &[u8] {
- &self.0[..self.len()]
- }
-
- /// Convert the string to a byte slice containing the trailing `NUL` byte.
- #[inline]
- pub const fn as_bytes_with_nul(&self) -> &[u8] {
- &self.0
- }
-
- /// Yields a [`&str`] slice if the [`CStr`] contains valid UTF-8.
- ///
- /// If the contents of the [`CStr`] are valid UTF-8 data, this
- /// function will return the corresponding [`&str`] slice. Otherwise,
- /// it will return an error with details of where UTF-8 validation failed.
- ///
- /// # Examples
- ///
- /// ```
- /// # use kernel::str::CStr;
- /// let cstr = CStr::from_bytes_with_nul(b"foo\0")?;
- /// assert_eq!(cstr.to_str(), Ok("foo"));
- /// # Ok::<(), kernel::error::Error>(())
- /// ```
- #[inline]
- pub fn to_str(&self) -> Result<&str, core::str::Utf8Error> {
- core::str::from_utf8(self.as_bytes())
- }
-
- /// Unsafely convert this [`CStr`] into a [`&str`], without checking for
- /// valid UTF-8.
- ///
- /// # Safety
- ///
- /// The contents must be valid UTF-8.
- ///
- /// # Examples
- ///
- /// ```
- /// # use kernel::c_str;
- /// # use kernel::str::CStr;
- /// let bar = c_str!("ツ");
- /// // SAFETY: String literals are guaranteed to be valid UTF-8
- /// // by the Rust compiler.
- /// assert_eq!(unsafe { bar.as_str_unchecked() }, "ツ");
- /// ```
- #[inline]
- pub unsafe fn as_str_unchecked(&self) -> &str {
- // SAFETY: TODO.
- unsafe { core::str::from_utf8_unchecked(self.as_bytes()) }
- }
+ // This function exists to paper over the fact that `CStr::as_ptr` returns a `*const
+ // core::ffi::c_char` rather than a `*const crate::ffi::c_char`.
+ fn as_char_ptr(&self) -> *const crate::ffi::c_char;
/// Convert this [`CStr`] into a [`CString`] by allocating memory and
/// copying over the string data.
- pub fn to_cstring(&self) -> Result<CString, AllocError> {
- CString::try_from(self)
- }
+ fn to_cstring(&self) -> Result<CString, AllocError>;
/// Converts this [`CStr`] to its ASCII lower case equivalent in-place.
///
@@ -370,11 +228,7 @@ pub fn to_cstring(&self) -> Result<CString, AllocError> {
/// [`to_ascii_lowercase()`].
///
/// [`to_ascii_lowercase()`]: #method.to_ascii_lowercase
- pub fn make_ascii_lowercase(&mut self) {
- // INVARIANT: This doesn't introduce or remove NUL bytes in the C
- // string.
- self.0.make_ascii_lowercase();
- }
+ fn make_ascii_lowercase(&mut self);
/// Converts this [`CStr`] to its ASCII upper case equivalent in-place.
///
@@ -385,11 +239,7 @@ pub fn make_ascii_lowercase(&mut self) {
/// [`to_ascii_uppercase()`].
///
/// [`to_ascii_uppercase()`]: #method.to_ascii_uppercase
- pub fn make_ascii_uppercase(&mut self) {
- // INVARIANT: This doesn't introduce or remove NUL bytes in the C
- // string.
- self.0.make_ascii_uppercase();
- }
+ fn make_ascii_uppercase(&mut self);
/// Returns a copy of this [`CString`] where each character is mapped to its
/// ASCII lower case equivalent.
@@ -400,13 +250,7 @@ pub fn make_ascii_uppercase(&mut self) {
/// To lowercase the value in-place, use [`make_ascii_lowercase`].
///
/// [`make_ascii_lowercase`]: str::make_ascii_lowercase
- pub fn to_ascii_lowercase(&self) -> Result<CString, AllocError> {
- let mut s = self.to_cstring()?;
-
- s.make_ascii_lowercase();
-
- Ok(s)
- }
+ fn to_ascii_lowercase(&self) -> Result<CString, AllocError>;
/// Returns a copy of this [`CString`] where each character is mapped to its
/// ASCII upper case equivalent.
@@ -417,13 +261,7 @@ pub fn to_ascii_lowercase(&self) -> Result<CString, AllocError> {
/// To uppercase the value in-place, use [`make_ascii_uppercase`].
///
/// [`make_ascii_uppercase`]: str::make_ascii_uppercase
- pub fn to_ascii_uppercase(&self) -> Result<CString, AllocError> {
- let mut s = self.to_cstring()?;
-
- s.make_ascii_uppercase();
-
- Ok(s)
- }
+ fn to_ascii_uppercase(&self) -> Result<CString, AllocError>;
/// Returns an object that implements [`Display`] for safely printing a [`CStr`] that may
/// contain non-Unicode data. If you would like an implementation which escapes the [`CStr`]
@@ -440,113 +278,93 @@ pub fn to_ascii_uppercase(&self) -> Result<CString, AllocError> {
/// # use kernel::str::CString;
/// let penguin = c_str!("🐧");
/// let s = CString::try_from_fmt(fmt!("{}", penguin.display()))?;
- /// assert_eq!(s.as_bytes_with_nul(), "\\xf0\\x9f\\x90\\xa7\0".as_bytes());
+ /// assert_eq!(s.to_bytes_with_nul(), "\\xf0\\x9f\\x90\\xa7\0".as_bytes());
///
/// let ascii = c_str!("so \"cool\"");
/// let s = CString::try_from_fmt(fmt!("{}", ascii.display()))?;
- /// assert_eq!(s.as_bytes_with_nul(), "so \"cool\"\0".as_bytes());
+ /// assert_eq!(s.to_bytes_with_nul(), "so \"cool\"\0".as_bytes());
/// # Ok::<(), kernel::error::Error>(())
/// ```
- #[inline]
- pub fn display(&self) -> Display<'_> {
- Display {
- inner: self,
- escape_common: false,
- }
- }
+ fn display(&self) -> Display<'_>;
}
-impl fmt::Debug for CStr {
- /// Formats printable ASCII characters with a double quote on either end, escaping the rest.
- ///
- /// ```
- /// # use kernel::c_str;
- /// # use kernel::fmt;
- /// # use kernel::str::CString;
- /// let penguin = c_str!("🐧");
- /// let s = CString::try_from_fmt(fmt!("{:?}", penguin))?;
- /// assert_eq!(s.as_bytes_with_nul(), "\"\\xf0\\x9f\\x90\\xa7\"\0".as_bytes());
- ///
- /// // Embedded double quotes are escaped.
- /// let ascii = c_str!("so \"cool\"");
- /// let s = CString::try_from_fmt(fmt!("{:?}", ascii))?;
- /// assert_eq!(s.as_bytes_with_nul(), "\"so \\\"cool\\\"\"\0".as_bytes());
- /// # Ok::<(), kernel::error::Error>(())
- /// ```
- fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
- f.write_str("\"")?;
- for &c in self.as_bytes() {
- match c {
- // Printable characters.
- b'\"' => f.write_str("\\\"")?,
- 0x20..=0x7e => f.write_char(c as char)?,
- _ => write!(f, "\\x{:02x}", c)?,
- }
- }
- f.write_str("\"")
- }
+/// Converts a mutable C string to a mutable byte slice.
+///
+/// # Safety
+///
+/// The caller must ensure that the slice ends in a NUL byte and contains no other NUL bytes before
+/// the borrow ends and the underlying [`CStr`] is used.
+unsafe fn to_bytes_mut(s: &mut CStr) -> &mut [u8] {
+ // SAFETY: the cast from `&CStr` to `&[u8]` is safe since `CStr` has the same layout as `&[u8]`
+ // (this is technically not guaranteed, but we rely on it here). The pointer dereference is
+ // safe since it comes from a mutable reference which is guaranteed to be valid for writes.
+ unsafe { &mut *(s as *mut CStr as *mut [u8]) }
}
-impl AsRef<BStr> for CStr {
+impl CStrExt for CStr {
#[inline]
- fn as_ref(&self) -> &BStr {
- BStr::from_bytes(self.as_bytes())
+ unsafe fn from_char_ptr<'a>(ptr: *const crate::ffi::c_char) -> &'a Self {
+ // SAFETY: The safety preconditions are the same as for `CStr::from_ptr`.
+ unsafe { CStr::from_ptr(ptr.cast()) }
}
-}
-impl Deref for CStr {
- type Target = BStr;
+ #[inline]
+ unsafe fn from_bytes_with_nul_unchecked_mut(bytes: &mut [u8]) -> &mut Self {
+ // SAFETY: the cast from `&[u8]` to `&CStr` is safe since the properties of `bytes` are
+ // guaranteed by the safety precondition and `CStr` has the same layout as `&[u8]` (this is
+ // technically not guaranteed, but we rely on it here). The pointer dereference is safe
+ // since it comes from a mutable reference which is guaranteed to be valid for writes.
+ unsafe { &mut *(bytes as *mut [u8] as *mut CStr) }
+ }
#[inline]
- fn deref(&self) -> &Self::Target {
- self.as_ref()
+ fn as_char_ptr(&self) -> *const crate::ffi::c_char {
+ self.as_ptr().cast()
}
-}
-impl Index<ops::RangeFrom<usize>> for CStr {
- type Output = CStr;
+ fn to_cstring(&self) -> Result<CString, AllocError> {
+ CString::try_from(self)
+ }
- #[inline]
- fn index(&self, index: ops::RangeFrom<usize>) -> &Self::Output {
- // Delegate bounds checking to slice.
- // Assign to _ to mute clippy's unnecessary operation warning.
- let _ = &self.as_bytes()[index.start..];
- // SAFETY: We just checked the bounds.
- unsafe { Self::from_bytes_with_nul_unchecked(&self.0[index.start..]) }
+ fn make_ascii_lowercase(&mut self) {
+ // SAFETY: This doesn't introduce or remove NUL bytes in the C string.
+ unsafe { to_bytes_mut(self) }.make_ascii_lowercase();
}
-}
-impl Index<ops::RangeFull> for CStr {
- type Output = CStr;
+ fn make_ascii_uppercase(&mut self) {
+ // SAFETY: This doesn't introduce or remove NUL bytes in the C string.
+ unsafe { to_bytes_mut(self) }.make_ascii_uppercase();
+ }
- #[inline]
- fn index(&self, _index: ops::RangeFull) -> &Self::Output {
- self
+ fn to_ascii_lowercase(&self) -> Result<CString, AllocError> {
+ let mut s = self.to_cstring()?;
+
+ s.make_ascii_lowercase();
+
+ Ok(s)
}
-}
-mod private {
- use core::ops;
+ fn to_ascii_uppercase(&self) -> Result<CString, AllocError> {
+ let mut s = self.to_cstring()?;
- // Marker trait for index types that can be forward to `BStr`.
- pub trait CStrIndex {}
+ s.make_ascii_uppercase();
- impl CStrIndex for usize {}
- impl CStrIndex for ops::Range<usize> {}
- impl CStrIndex for ops::RangeInclusive<usize> {}
- impl CStrIndex for ops::RangeToInclusive<usize> {}
-}
+ Ok(s)
+ }
-impl<Idx> Index<Idx> for CStr
-where
- Idx: private::CStrIndex,
- BStr: Index<Idx>,
-{
- type Output = <BStr as Index<Idx>>::Output;
+ #[inline]
+ fn display(&self) -> Display<'_> {
+ Display {
+ inner: self.as_ref(),
+ escape_common: false,
+ }
+ }
+}
+impl AsRef<BStr> for CStr {
#[inline]
- fn index(&self, index: Idx) -> &Self::Output {
- &self.as_ref()[index]
+ fn as_ref(&self) -> &BStr {
+ BStr::from_bytes(self.to_bytes())
}
}
@@ -630,25 +448,15 @@ fn test_cstr_to_str_panic() {
checked_cstr.to_str().unwrap();
}
- #[test]
- fn test_cstr_as_str_unchecked() {
- let good_bytes = b"\xf0\x9f\x90\xA7\0";
- let checked_cstr = CStr::from_bytes_with_nul(good_bytes).unwrap();
- // SAFETY: The contents come from a string literal which contains valid UTF-8.
- let unchecked_str = unsafe { checked_cstr.as_str_unchecked() };
- assert_eq!(unchecked_str, "🐧");
- }
-
#[test]
fn test_cstr_display() {
- let hello_world = CStr::from_bytes_with_nul(b"hello, world!\0").unwrap();
- assert_eq!(format!("{}", hello_world.display()), "hello, world!");
- let non_printables = CStr::from_bytes_with_nul(b"\x01\x09\x0a\0").unwrap();
- assert_eq!(format!("{}", non_printables.display()), "\\x01\\x09\\x0a");
- let non_ascii = CStr::from_bytes_with_nul(b"d\xe9j\xe0 vu\0").unwrap();
- assert_eq!(format!("{}", non_ascii.display()), "d\\xe9j\\xe0 vu");
- let good_bytes = CStr::from_bytes_with_nul(b"\xf0\x9f\xa6\x80\0").unwrap();
- assert_eq!(format!("{}", good_bytes.display()), "\\xf0\\x9f\\xa6\\x80");
+ assert_eq!(format!("{}", c"hello, world!".display()), "hello, world!");
+ assert_eq!(format!("{}", c"\x01\x09\x0a".display()), "\\x01\\x09\\x0a");
+ assert_eq!(format!("{}", c"d\xe9j\xe0 vu".display()), "d\\xe9j\\xe0 vu");
+ assert_eq!(
+ format!("{}", c"\xf0\x9f\xa6\x80".display()),
+ "\\xf0\\x9f\\xa6\\x80"
+ );
}
#[test]
@@ -664,14 +472,13 @@ fn test_cstr_display_all_bytes() {
#[test]
fn test_cstr_debug() {
- let hello_world = CStr::from_bytes_with_nul(b"hello, world!\0").unwrap();
- assert_eq!(format!("{:?}", hello_world), "\"hello, world!\"");
- let non_printables = CStr::from_bytes_with_nul(b"\x01\x09\x0a\0").unwrap();
- assert_eq!(format!("{:?}", non_printables), "\"\\x01\\x09\\x0a\"");
- let non_ascii = CStr::from_bytes_with_nul(b"d\xe9j\xe0 vu\0").unwrap();
- assert_eq!(format!("{:?}", non_ascii), "\"d\\xe9j\\xe0 vu\"");
- let good_bytes = CStr::from_bytes_with_nul(b"\xf0\x9f\xa6\x80\0").unwrap();
- assert_eq!(format!("{:?}", good_bytes), "\"\\xf0\\x9f\\xa6\\x80\"");
+ assert_eq!(format!("{:?}", c"hello, world!"), "\"hello, world!\"");
+ assert_eq!(format!("{:?}", c"\x01\x09\x0a"), "\"\\x01\\t\\n\"");
+ assert_eq!(format!("{:?}", c"d\xe9j\xe0 vu"), "\"d\\xe9j\\xe0 vu\"");
+ assert_eq!(
+ format!("{:?}", c"\xf0\x9f\xa6\x80"),
+ "\"\\xf0\\x9f\\xa6\\x80\""
+ );
}
#[test]
@@ -851,14 +658,14 @@ fn write_str(&mut self, s: &str) -> fmt::Result {
/// # Examples
///
/// ```
-/// use kernel::{str::CString, fmt};
+/// use kernel::{fmt, str::CString};
///
/// let s = CString::try_from_fmt(fmt!("{}{}{}", "abc", 10, 20))?;
-/// assert_eq!(s.as_bytes_with_nul(), "abc1020\0".as_bytes());
+/// assert_eq!(s.to_bytes_with_nul(), "abc1020\0".as_bytes());
///
/// let tmp = "testing";
/// let s = CString::try_from_fmt(fmt!("{tmp}{}", 123))?;
-/// assert_eq!(s.as_bytes_with_nul(), "testing123\0".as_bytes());
+/// assert_eq!(s.to_bytes_with_nul(), "testing123\0".as_bytes());
///
/// // This fails because it has an embedded `NUL` byte.
/// let s = CString::try_from_fmt(fmt!("a\0b{}", 123));
@@ -928,7 +735,7 @@ impl<'a> TryFrom<&'a CStr> for CString {
fn try_from(cstr: &'a CStr) -> Result<CString, AllocError> {
let mut buf = KVec::new();
- buf.extend_from_slice(cstr.as_bytes_with_nul(), GFP_KERNEL)?;
+ buf.extend_from_slice(cstr.to_bytes_with_nul(), GFP_KERNEL)?;
// INVARIANT: The `CStr` and `CString` types have the same invariants for
// the string data, and we copied it over without changes.
diff --git a/rust/kernel/sync/condvar.rs b/rust/kernel/sync/condvar.rs
index 7df565038d7d..7901aa040f0e 100644
--- a/rust/kernel/sync/condvar.rs
+++ b/rust/kernel/sync/condvar.rs
@@ -10,7 +10,7 @@
ffi::{c_int, c_long},
init::PinInit,
pin_init,
- str::CStr,
+ str::{CStr, CStrExt as _},
task::{MAX_SCHEDULE_TIMEOUT, TASK_INTERRUPTIBLE, TASK_NORMAL, TASK_UNINTERRUPTIBLE},
time::Jiffies,
types::Opaque,
diff --git a/rust/kernel/sync/lock.rs b/rust/kernel/sync/lock.rs
index eb80048e0110..37f1a6a53b12 100644
--- a/rust/kernel/sync/lock.rs
+++ b/rust/kernel/sync/lock.rs
@@ -9,7 +9,7 @@
use crate::{
init::PinInit,
pin_init,
- str::CStr,
+ str::{CStr, CStrExt as _},
types::{NotThreadSafe, Opaque, ScopeGuard},
};
use core::{cell::UnsafeCell, marker::PhantomPinned};
diff --git a/rust/kernel/sync/lock/global.rs b/rust/kernel/sync/lock/global.rs
index 480ee724e3cc..6a28f5b60482 100644
--- a/rust/kernel/sync/lock/global.rs
+++ b/rust/kernel/sync/lock/global.rs
@@ -5,7 +5,7 @@
//! Support for defining statics containing locks.
use crate::{
- str::CStr,
+ str::{CStr, CStrExt as _},
sync::lock::{Backend, Guard, Lock},
sync::{LockClassKey, LockedBy},
types::Opaque,
--
2.48.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v8 3/4] rust: replace `kernel::c_str!` with C-Strings
2025-02-03 11:50 [PATCH v8 0/4] rust: replace kernel::str::CStr w/ core::ffi::CStr Tamir Duberstein
2025-02-03 11:50 ` [PATCH v8 1/4] rust: move BStr,CStr Display impls behind method Tamir Duberstein
2025-02-03 11:50 ` [PATCH v8 2/4] rust: replace `CStr` with `core::ffi::CStr` Tamir Duberstein
@ 2025-02-03 11:50 ` Tamir Duberstein
2025-02-03 11:50 ` [PATCH v8 4/4] rust: remove core::ffi::CStr reexport Tamir Duberstein
2025-02-18 17:05 ` [PATCH v8 0/4] rust: replace kernel::str::CStr w/ core::ffi::CStr Tamir Duberstein
4 siblings, 0 replies; 12+ messages in thread
From: Tamir Duberstein @ 2025-02-03 11:50 UTC (permalink / raw)
To: Michal Rostecki, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross
Cc: rust-for-linux, linux-kernel, Tamir Duberstein
C-String literals were added in Rust 1.77. Replace instances of
`kernel::c_str!` with C-String literals where possible and rename
`kernel::c_str!` to `c_str_avoid_literals` to clarify its intended use.
Closes: https://github.com/Rust-for-Linux/linux/issues/1075
Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
drivers/net/phy/ax88796b_rust.rs | 7 +++----
drivers/net/phy/qt2025.rs | 5 ++---
rust/kernel/devres.rs | 2 +-
rust/kernel/firmware.rs | 4 ++--
rust/kernel/kunit.rs | 7 ++++---
rust/kernel/net/phy.rs | 6 ++----
rust/kernel/platform.rs | 4 ++--
rust/kernel/str.rs | 37 +++++++++++++++++++++++-------------
rust/kernel/sync.rs | 4 ++--
rust/kernel/sync/lock/global.rs | 3 ++-
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 +--
14 files changed, 50 insertions(+), 42 deletions(-)
diff --git a/drivers/net/phy/ax88796b_rust.rs b/drivers/net/phy/ax88796b_rust.rs
index 8c7eb009d9fc..b07d36f45a43 100644
--- a/drivers/net/phy/ax88796b_rust.rs
+++ b/drivers/net/phy/ax88796b_rust.rs
@@ -5,7 +5,6 @@
//!
//! C version of this driver: [`drivers/net/phy/ax88796b.c`](./ax88796b.c)
use kernel::{
- c_str,
net::phy::{self, reg::C22, DeviceId, Driver},
prelude::*,
uapi,
@@ -41,7 +40,7 @@ fn asix_soft_reset(dev: &mut phy::Device) -> Result {
#[vtable]
impl Driver for PhyAX88772A {
const FLAGS: u32 = phy::flags::IS_INTERNAL;
- const NAME: &'static CStr = c_str!("Asix Electronics AX88772A");
+ const NAME: &'static CStr = c"Asix Electronics AX88772A";
const PHY_DEVICE_ID: DeviceId = DeviceId::new_with_exact_mask(0x003b1861);
// AX88772A is not working properly with some old switches (NETGEAR EN 108TP):
@@ -105,7 +104,7 @@ fn link_change_notify(dev: &mut phy::Device) {
#[vtable]
impl Driver for PhyAX88772C {
const FLAGS: u32 = phy::flags::IS_INTERNAL;
- const NAME: &'static CStr = c_str!("Asix Electronics AX88772C");
+ const NAME: &'static CStr = c"Asix Electronics AX88772C";
const PHY_DEVICE_ID: DeviceId = DeviceId::new_with_exact_mask(0x003b1881);
fn suspend(dev: &mut phy::Device) -> Result {
@@ -125,7 +124,7 @@ fn soft_reset(dev: &mut phy::Device) -> Result {
#[vtable]
impl Driver for PhyAX88796B {
- const NAME: &'static CStr = c_str!("Asix Electronics AX88796B");
+ const NAME: &'static CStr = c"Asix Electronics AX88796B";
const PHY_DEVICE_ID: DeviceId = DeviceId::new_with_model_mask(0x003b1841);
fn soft_reset(dev: &mut phy::Device) -> Result {
diff --git a/drivers/net/phy/qt2025.rs b/drivers/net/phy/qt2025.rs
index 1ab065798175..2f4ba85a0d73 100644
--- a/drivers/net/phy/qt2025.rs
+++ b/drivers/net/phy/qt2025.rs
@@ -9,7 +9,6 @@
//!
//! The QT2025 PHY integrates an Intel 8051 micro-controller.
-use kernel::c_str;
use kernel::error::code;
use kernel::firmware::Firmware;
use kernel::net::phy::{
@@ -36,7 +35,7 @@
#[vtable]
impl Driver for PhyQT2025 {
- const NAME: &'static CStr = c_str!("QT2025 10Gpbs SFP+");
+ const NAME: &'static CStr = c"QT2025 10Gpbs SFP+";
const PHY_DEVICE_ID: phy::DeviceId = phy::DeviceId::new_with_exact_mask(0x0043a400);
fn probe(dev: &mut phy::Device) -> Result<()> {
@@ -69,7 +68,7 @@ fn probe(dev: &mut phy::Device) -> Result<()> {
// The micro-controller will start running from the boot ROM.
dev.write(C45::new(Mmd::PCS, 0xe854), 0x00c0)?;
- let fw = Firmware::request(c_str!("qt2025-2.0.3.3.fw"), dev.as_ref())?;
+ let fw = Firmware::request(c"qt2025-2.0.3.3.fw", dev.as_ref())?;
if fw.data().len() > SZ_16K + SZ_8K {
return Err(code::EFBIG);
}
diff --git a/rust/kernel/devres.rs b/rust/kernel/devres.rs
index 942376f6f3af..68af33ca2f25 100644
--- a/rust/kernel/devres.rs
+++ b/rust/kernel/devres.rs
@@ -45,7 +45,7 @@ struct DevresInner<T> {
/// # Example
///
/// ```no_run
-/// # use kernel::{bindings, c_str, device::Device, devres::Devres, io::{Io, IoRaw}};
+/// # use kernel::{bindings, device::Device, devres::Devres, io::{Io, IoRaw}};
/// # use core::ops::Deref;
///
/// // See also [`pci::Bar`] for a real example.
diff --git a/rust/kernel/firmware.rs b/rust/kernel/firmware.rs
index c5162fdc95ff..e371df67261e 100644
--- a/rust/kernel/firmware.rs
+++ b/rust/kernel/firmware.rs
@@ -40,13 +40,13 @@ fn request_nowarn() -> Self {
/// # Examples
///
/// ```no_run
-/// # use kernel::{c_str, device::Device, firmware::Firmware};
+/// # use kernel::{device::Device, firmware::Firmware};
///
/// # fn no_run() -> Result<(), Error> {
/// # // SAFETY: *NOT* safe, just for the example to get an `ARef<Device>` instance
/// # let dev = unsafe { Device::get_device(core::ptr::null_mut()) };
///
-/// let fw = Firmware::request(c_str!("path/to/firmware.bin"), &dev)?;
+/// let fw = Firmware::request(c"path/to/firmware.bin", &dev)?;
/// let blob = fw.data();
///
/// # Ok(())
diff --git a/rust/kernel/kunit.rs b/rust/kernel/kunit.rs
index d858757aeace..527794dcc439 100644
--- a/rust/kernel/kunit.rs
+++ b/rust/kernel/kunit.rs
@@ -56,10 +56,11 @@ macro_rules! kunit_assert {
break 'out;
}
- static NAME: &'static $crate::str::CStr = $crate::c_str!($name);
- static FILE: &'static $crate::str::CStr = $crate::c_str!($file);
+ static NAME: &'static $crate::str::CStr = $crate::c_str_avoid_literals!($name);
+ static FILE: &'static $crate::str::CStr = $crate::c_str_avoid_literals!($file);
static LINE: i32 = core::line!() as i32 - $diff;
- static CONDITION: &'static $crate::str::CStr = $crate::c_str!(stringify!($condition));
+ static CONDITION: &'static $crate::str::CStr =
+ $crate::c_str_avoid_literals!(stringify!($condition));
// SAFETY: FFI call without safety requirements.
let kunit_test = unsafe { $crate::bindings::kunit_get_current_test() };
diff --git a/rust/kernel/net/phy.rs b/rust/kernel/net/phy.rs
index 73df0f55f94e..edd03e5b8b28 100644
--- a/rust/kernel/net/phy.rs
+++ b/rust/kernel/net/phy.rs
@@ -780,7 +780,6 @@ const fn as_int(&self) -> u32 {
///
/// ```
/// # mod module_phy_driver_sample {
-/// use kernel::c_str;
/// use kernel::net::phy::{self, DeviceId};
/// use kernel::prelude::*;
///
@@ -799,7 +798,7 @@ const fn as_int(&self) -> u32 {
///
/// #[vtable]
/// impl phy::Driver for PhySample {
-/// const NAME: &'static CStr = c_str!("PhySample");
+/// const NAME: &'static CStr = c"PhySample";
/// const PHY_DEVICE_ID: phy::DeviceId = phy::DeviceId::new_with_exact_mask(0x00000001);
/// }
/// # }
@@ -808,7 +807,6 @@ const fn as_int(&self) -> u32 {
/// This expands to the following code:
///
/// ```ignore
-/// use kernel::c_str;
/// use kernel::net::phy::{self, DeviceId};
/// use kernel::prelude::*;
///
@@ -828,7 +826,7 @@ const fn as_int(&self) -> u32 {
///
/// #[vtable]
/// impl phy::Driver for PhySample {
-/// const NAME: &'static CStr = c_str!("PhySample");
+/// const NAME: &'static CStr = c"PhySample";
/// const PHY_DEVICE_ID: phy::DeviceId = phy::DeviceId::new_with_exact_mask(0x00000001);
/// }
///
diff --git a/rust/kernel/platform.rs b/rust/kernel/platform.rs
index 50e6b0421813..99af0a16ae85 100644
--- a/rust/kernel/platform.rs
+++ b/rust/kernel/platform.rs
@@ -120,7 +120,7 @@ macro_rules! module_platform_driver {
/// # Example
///
///```
-/// # use kernel::{bindings, c_str, of, platform};
+/// # use kernel::{bindings, of, platform};
///
/// struct MyDriver;
///
@@ -129,7 +129,7 @@ macro_rules! module_platform_driver {
/// MODULE_OF_TABLE,
/// <MyDriver as platform::Driver>::IdInfo,
/// [
-/// (of::DeviceId::new(c_str!("test,device")), ())
+/// (of::DeviceId::new(c"test,device"), ())
/// ]
/// );
///
diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
index 13dd2f6cc572..4063d393ab58 100644
--- a/rust/kernel/str.rs
+++ b/rust/kernel/str.rs
@@ -273,14 +273,13 @@ pub trait CStrExt {
/// # Examples
///
/// ```
- /// # use kernel::c_str;
/// # use kernel::fmt;
/// # use kernel::str::CString;
- /// let penguin = c_str!("🐧");
+ /// let penguin = c"🐧";
/// let s = CString::try_from_fmt(fmt!("{}", penguin.display()))?;
/// assert_eq!(s.to_bytes_with_nul(), "\\xf0\\x9f\\x90\\xa7\0".as_bytes());
///
- /// let ascii = c_str!("so \"cool\"");
+ /// let ascii = c"so \"cool\"";
/// let s = CString::try_from_fmt(fmt!("{}", ascii.display()))?;
/// assert_eq!(s.to_bytes_with_nul(), "so \"cool\"\0".as_bytes());
/// # Ok::<(), kernel::error::Error>(())
@@ -368,25 +367,37 @@ fn as_ref(&self) -> &BStr {
}
}
-/// Creates a new [`CStr`] from a string literal.
+/// Creates a static C string wrapper at compile time.
///
-/// The string literal should not contain any `NUL` bytes.
+/// Rust supports C string literals since Rust 1.77, and they should be used instead of this macro
+/// where possible. This macro exists to allow static *non-literal* C strings to be created at
+/// compile time. This is most often used in other macros.
+///
+/// # Panics
+///
+/// This macro panics if the operand contains an interior `NUL` byte.
///
/// # Examples
///
/// ```
-/// # use kernel::c_str;
+/// # use kernel::c_str_avoid_literals;
/// # use kernel::str::CStr;
-/// const MY_CSTR: &CStr = c_str!("My awesome CStr!");
+/// const MY_CSTR: &CStr = c_str_avoid_literals!(concat!(file!(), ":", line!(), ": My CStr!"));
/// ```
#[macro_export]
-macro_rules! c_str {
+macro_rules! c_str_avoid_literals {
+ // NB: we could write `($str:lit) => compile_error!("use a C string literal instead");` here but
+ // that would trigger when the literal is at the top of several macro expansions. That would be
+ // too limiting to macro authors, so we rely on the name as a hint instead.
($str:expr) => {{
- const S: &str = concat!($str, "\0");
- const C: &$crate::str::CStr = match $crate::str::CStr::from_bytes_with_nul(S.as_bytes()) {
- Ok(v) => v,
- Err(_) => panic!("string contains interior NUL"),
- };
+ const S: &'static str = concat!($str, "\0");
+ const C: &'static $crate::str::CStr =
+ match $crate::str::CStr::from_bytes_with_nul(S.as_bytes()) {
+ Ok(v) => v,
+ Err(core::ffi::FromBytesWithNulError { .. }) => {
+ panic!("string contains interior NUL")
+ }
+ };
C
}};
}
diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs
index 3498fb344dc9..f86d109b4b89 100644
--- a/rust/kernel/sync.rs
+++ b/rust/kernel/sync.rs
@@ -62,9 +62,9 @@ macro_rules! static_lock_class {
#[macro_export]
macro_rules! optional_name {
() => {
- $crate::c_str!(::core::concat!(::core::file!(), ":", ::core::line!()))
+ $crate::c_str_avoid_literals!(::core::concat!(::core::file!(), ":", ::core::line!()))
};
($name:literal) => {
- $crate::c_str!($name)
+ $crate::c_str_avoid_literals!($name)
};
}
diff --git a/rust/kernel/sync/lock/global.rs b/rust/kernel/sync/lock/global.rs
index 6a28f5b60482..4a040c29faee 100644
--- a/rust/kernel/sync/lock/global.rs
+++ b/rust/kernel/sync/lock/global.rs
@@ -266,7 +266,8 @@ macro_rules! global_lock {
$pub enum $name {}
impl $crate::sync::lock::GlobalLockBackend for $name {
- const NAME: &'static $crate::str::CStr = $crate::c_str!(::core::stringify!($name));
+ const NAME: &'static $crate::str::CStr =
+ $crate::c_str_avoid_literals!(::core::stringify!($name));
type Item = $valuety;
type Backend = $crate::global_lock_inner!(backend $kind);
diff --git a/rust/macros/module.rs b/rust/macros/module.rs
index cdf94f4982df..a534fabd29d6 100644
--- a/rust/macros/module.rs
+++ b/rust/macros/module.rs
@@ -229,7 +229,7 @@ pub(crate) fn module(ts: TokenStream) -> TokenStream {
}};
impl kernel::ModuleMetadata for {type_} {{
- const NAME: &'static kernel::str::CStr = kernel::c_str!(\"{name}\");
+ const NAME: &'static kernel::str::CStr = c\"{name}\";
}}
// Double nested modules, since then nobody can access the public items inside.
diff --git a/samples/rust/rust_driver_pci.rs b/samples/rust/rust_driver_pci.rs
index 1fb6e44f3395..cb4a9247ff5e 100644
--- a/samples/rust/rust_driver_pci.rs
+++ b/samples/rust/rust_driver_pci.rs
@@ -4,7 +4,7 @@
//!
//! To make this driver probe, QEMU must be run with `-device pci-testdev`.
-use kernel::{bindings, c_str, devres::Devres, pci, prelude::*};
+use kernel::{bindings, devres::Devres, pci, prelude::*};
struct Regs;
@@ -73,7 +73,7 @@ fn probe(pdev: &mut pci::Device, info: &Self::IdInfo) -> Result<Pin<KBox<Self>>>
pdev.enable_device_mem()?;
pdev.set_master();
- let bar = pdev.iomap_region_sized::<{ Regs::END }>(0, c_str!("rust_driver_pci"))?;
+ let bar = pdev.iomap_region_sized::<{ Regs::END }>(0, c"rust_driver_pci")?;
let drvdata = KBox::new(
Self {
diff --git a/samples/rust/rust_driver_platform.rs b/samples/rust/rust_driver_platform.rs
index 8120609e2940..d2df2d0f8261 100644
--- a/samples/rust/rust_driver_platform.rs
+++ b/samples/rust/rust_driver_platform.rs
@@ -2,7 +2,7 @@
//! Rust Platform driver sample.
-use kernel::{c_str, of, platform, prelude::*};
+use kernel::{of, platform, prelude::*};
struct SampleDriver {
pdev: platform::Device,
@@ -14,7 +14,7 @@ struct SampleDriver {
OF_TABLE,
MODULE_OF_TABLE,
<SampleDriver as platform::Driver>::IdInfo,
- [(of::DeviceId::new(c_str!("test,rust-device")), Info(42))]
+ [(of::DeviceId::new(c"test,rust-device"), Info(42))]
);
impl platform::Driver for SampleDriver {
diff --git a/samples/rust/rust_misc_device.rs b/samples/rust/rust_misc_device.rs
index 40ad7266c225..9d85819a195b 100644
--- a/samples/rust/rust_misc_device.rs
+++ b/samples/rust/rust_misc_device.rs
@@ -97,7 +97,6 @@
use core::pin::Pin;
use kernel::{
- c_str,
device::Device,
fs::File,
ioctl::{_IO, _IOC_SIZE, _IOR, _IOW},
@@ -132,7 +131,7 @@ fn init(_module: &'static ThisModule) -> impl PinInit<Self, Error> {
pr_info!("Initialising Rust Misc Device Sample\n");
let options = MiscDeviceOptions {
- name: c_str!("rust-misc-device"),
+ name: c"rust-misc-device",
};
try_pin_init!(Self {
--
2.48.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v8 4/4] rust: remove core::ffi::CStr reexport
2025-02-03 11:50 [PATCH v8 0/4] rust: replace kernel::str::CStr w/ core::ffi::CStr Tamir Duberstein
` (2 preceding siblings ...)
2025-02-03 11:50 ` [PATCH v8 3/4] rust: replace `kernel::c_str!` with C-Strings Tamir Duberstein
@ 2025-02-03 11:50 ` Tamir Duberstein
2025-02-18 17:05 ` [PATCH v8 0/4] rust: replace kernel::str::CStr w/ core::ffi::CStr Tamir Duberstein
4 siblings, 0 replies; 12+ messages in thread
From: Tamir Duberstein @ 2025-02-03 11:50 UTC (permalink / raw)
To: Michal Rostecki, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross
Cc: rust-for-linux, linux-kernel, Tamir Duberstein
Clean up references to `kernel::str::CStr`.
Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
rust/kernel/device.rs | 3 +--
rust/kernel/driver.rs | 4 ++--
rust/kernel/error.rs | 6 ++----
rust/kernel/firmware.rs | 3 ++-
rust/kernel/kunit.rs | 6 +++---
rust/kernel/lib.rs | 2 +-
rust/kernel/miscdevice.rs | 3 +--
rust/kernel/of.rs | 3 ++-
rust/kernel/pci.rs | 3 +--
rust/kernel/platform.rs | 3 +--
rust/kernel/prelude.rs | 5 +----
rust/kernel/str.rs | 19 +++++++++----------
rust/kernel/sync/condvar.rs | 3 ++-
rust/kernel/sync/lock.rs | 4 ++--
rust/kernel/sync/lock/global.rs | 5 +++--
rust/kernel/sync/poll.rs | 1 +
rust/kernel/workqueue.rs | 1 +
rust/macros/module.rs | 2 +-
18 files changed, 36 insertions(+), 40 deletions(-)
diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs
index df4bfa5f51ea..a1123206819b 100644
--- a/rust/kernel/device.rs
+++ b/rust/kernel/device.rs
@@ -6,10 +6,9 @@
use crate::{
bindings,
- str::CStr,
types::{ARef, Opaque},
};
-use core::{fmt, ptr};
+use core::{ffi::CStr, fmt, ptr};
#[cfg(CONFIG_PRINTK)]
use crate::str::CStrExt as _;
diff --git a/rust/kernel/driver.rs b/rust/kernel/driver.rs
index 2a16d5e64e6c..0fbcdc226729 100644
--- a/rust/kernel/driver.rs
+++ b/rust/kernel/driver.rs
@@ -6,8 +6,8 @@
//! register using the [`Registration`] class.
use crate::error::{Error, Result};
-use crate::{device, init::PinInit, of, str::CStr, try_pin_init, types::Opaque, ThisModule};
-use core::pin::Pin;
+use crate::{device, init::PinInit, of, try_pin_init, types::Opaque, ThisModule};
+use core::{ffi::CStr, pin::Pin};
use macros::{pin_data, pinned_drop};
/// The [`RegistrationOps`] trait serves as generic interface for subsystems (e.g., PCI, Platform,
diff --git a/rust/kernel/error.rs b/rust/kernel/error.rs
index cff9204d9dd6..aa0786cee048 100644
--- a/rust/kernel/error.rs
+++ b/rust/kernel/error.rs
@@ -4,11 +4,9 @@
//!
//! C header: [`include/uapi/asm-generic/errno-base.h`](srctree/include/uapi/asm-generic/errno-base.h)
-use crate::{
- alloc::{layout::LayoutError, AllocError},
- str::CStr,
-};
+use crate::alloc::{layout::LayoutError, AllocError};
+use core::ffi::CStr;
use core::fmt;
use core::num::NonZeroI32;
use core::num::TryFromIntError;
diff --git a/rust/kernel/firmware.rs b/rust/kernel/firmware.rs
index e371df67261e..73eaa477756a 100644
--- a/rust/kernel/firmware.rs
+++ b/rust/kernel/firmware.rs
@@ -4,7 +4,8 @@
//!
//! C header: [`include/linux/firmware.h`](srctree/include/linux/firmware.h)
-use crate::{bindings, device::Device, error::Error, error::Result, str::CStr};
+use crate::{bindings, device::Device, error::Error, error::Result};
+use core::ffi::CStr;
use core::ptr::NonNull;
/// # Invariants
diff --git a/rust/kernel/kunit.rs b/rust/kernel/kunit.rs
index 527794dcc439..f4e0e58877a0 100644
--- a/rust/kernel/kunit.rs
+++ b/rust/kernel/kunit.rs
@@ -56,10 +56,10 @@ macro_rules! kunit_assert {
break 'out;
}
- static NAME: &'static $crate::str::CStr = $crate::c_str_avoid_literals!($name);
- static FILE: &'static $crate::str::CStr = $crate::c_str_avoid_literals!($file);
+ static NAME: &'static core::ffi::CStr = $crate::c_str_avoid_literals!($name);
+ static FILE: &'static core::ffi::CStr = $crate::c_str_avoid_literals!($file);
static LINE: i32 = core::line!() as i32 - $diff;
- static CONDITION: &'static $crate::str::CStr =
+ static CONDITION: &'static core::ffi::CStr =
$crate::c_str_avoid_literals!(stringify!($condition));
// SAFETY: FFI call without safety requirements.
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index 496ed32b0911..a129ef3292dc 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -132,7 +132,7 @@ fn init(module: &'static ThisModule) -> impl init::PinInit<Self, error::Error> {
/// Metadata attached to a [`Module`] or [`InPlaceModule`].
pub trait ModuleMetadata {
/// The name of the module as specified in the `module!` macro.
- const NAME: &'static crate::str::CStr;
+ const NAME: &'static core::ffi::CStr;
}
/// Equivalent to `THIS_MODULE` in the C API.
diff --git a/rust/kernel/miscdevice.rs b/rust/kernel/miscdevice.rs
index 78c150270080..6e6f70cb4981 100644
--- a/rust/kernel/miscdevice.rs
+++ b/rust/kernel/miscdevice.rs
@@ -16,10 +16,9 @@
fs::File,
prelude::*,
seq_file::SeqFile,
- str::CStr,
types::{ForeignOwnable, Opaque},
};
-use core::{marker::PhantomData, mem::MaybeUninit, pin::Pin};
+use core::{ffi::CStr, marker::PhantomData, mem::MaybeUninit, pin::Pin};
/// Options for creating a misc device.
#[derive(Copy, Clone)]
diff --git a/rust/kernel/of.rs b/rust/kernel/of.rs
index 12ea65df46de..087ac8e05551 100644
--- a/rust/kernel/of.rs
+++ b/rust/kernel/of.rs
@@ -2,7 +2,8 @@
//! Device Tree / Open Firmware abstractions.
-use crate::{bindings, device_id::RawDeviceId, prelude::*};
+use crate::{bindings, device_id::RawDeviceId};
+use core::ffi::CStr;
/// IdTable type for OF drivers.
pub type IdTable<T> = &'static dyn kernel::device_id::IdTable<DeviceId, T>;
diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
index 4c98b5b9aa1e..bec5b6bcb696 100644
--- a/rust/kernel/pci.rs
+++ b/rust/kernel/pci.rs
@@ -13,11 +13,10 @@
error::{to_result, Result},
io::Io,
io::IoRaw,
- str::CStr,
types::{ARef, ForeignOwnable, Opaque},
ThisModule,
};
-use core::{ops::Deref, ptr::addr_of_mut};
+use core::{ffi::CStr, ops::Deref, ptr::addr_of_mut};
use kernel::prelude::*;
/// An adapter for the registration of PCI drivers.
diff --git a/rust/kernel/platform.rs b/rust/kernel/platform.rs
index 99af0a16ae85..173eaeb90fb4 100644
--- a/rust/kernel/platform.rs
+++ b/rust/kernel/platform.rs
@@ -9,12 +9,11 @@
error::{to_result, Result},
of,
prelude::*,
- str::CStr,
types::{ARef, ForeignOwnable, Opaque},
ThisModule,
};
-use core::ptr::addr_of_mut;
+use core::{ffi::CStr, ptr::addr_of_mut};
/// An adapter for the registration of platform drivers.
pub struct Adapter<T: Driver>(T);
diff --git a/rust/kernel/prelude.rs b/rust/kernel/prelude.rs
index 96e7029c27da..21fa7b3a68c3 100644
--- a/rust/kernel/prelude.rs
+++ b/rust/kernel/prelude.rs
@@ -34,10 +34,7 @@
pub use super::error::{code::*, Error, Result};
-pub use super::{
- str::{CStr, CStrExt as _},
- ThisModule,
-};
+pub use super::{str::CStrExt as _, ThisModule};
pub use super::init::{InPlaceInit, InPlaceWrite, Init, PinInit};
diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
index 4063d393ab58..d25c3d698f25 100644
--- a/rust/kernel/str.rs
+++ b/rust/kernel/str.rs
@@ -3,6 +3,7 @@
//! String representations.
use crate::alloc::{flags::*, AllocError, KVec};
+use core::ffi::CStr;
use core::fmt::{self, Write};
use core::ops::{Deref, DerefMut};
@@ -177,8 +178,6 @@ macro_rules! b_str {
}};
}
-pub use core::ffi::CStr;
-
/// Returns a C pointer to the string.
// It is a free function rather than a method on an extension trait because:
//
@@ -381,7 +380,7 @@ fn as_ref(&self) -> &BStr {
///
/// ```
/// # use kernel::c_str_avoid_literals;
-/// # use kernel::str::CStr;
+/// # use core::ffi::CStr;
/// const MY_CSTR: &CStr = c_str_avoid_literals!(concat!(file!(), ":", line!(), ": My CStr!"));
/// ```
#[macro_export]
@@ -391,13 +390,13 @@ macro_rules! c_str_avoid_literals {
// too limiting to macro authors, so we rely on the name as a hint instead.
($str:expr) => {{
const S: &'static str = concat!($str, "\0");
- const C: &'static $crate::str::CStr =
- match $crate::str::CStr::from_bytes_with_nul(S.as_bytes()) {
- Ok(v) => v,
- Err(core::ffi::FromBytesWithNulError { .. }) => {
- panic!("string contains interior NUL")
- }
- };
+ const C: &'static core::ffi::CStr = match core::ffi::CStr::from_bytes_with_nul(S.as_bytes())
+ {
+ Ok(v) => v,
+ Err(core::ffi::FromBytesWithNulError { .. }) => {
+ panic!("string contains interior NUL")
+ }
+ };
C
}};
}
diff --git a/rust/kernel/sync/condvar.rs b/rust/kernel/sync/condvar.rs
index 7901aa040f0e..b23544e24a0e 100644
--- a/rust/kernel/sync/condvar.rs
+++ b/rust/kernel/sync/condvar.rs
@@ -10,11 +10,12 @@
ffi::{c_int, c_long},
init::PinInit,
pin_init,
- str::{CStr, CStrExt as _},
+ str::CStrExt as _,
task::{MAX_SCHEDULE_TIMEOUT, TASK_INTERRUPTIBLE, TASK_NORMAL, TASK_UNINTERRUPTIBLE},
time::Jiffies,
types::Opaque,
};
+use core::ffi::CStr;
use core::marker::PhantomPinned;
use core::ptr;
use macros::pin_data;
diff --git a/rust/kernel/sync/lock.rs b/rust/kernel/sync/lock.rs
index 37f1a6a53b12..711773c877cc 100644
--- a/rust/kernel/sync/lock.rs
+++ b/rust/kernel/sync/lock.rs
@@ -9,10 +9,10 @@
use crate::{
init::PinInit,
pin_init,
- str::{CStr, CStrExt as _},
+ str::CStrExt as _,
types::{NotThreadSafe, Opaque, ScopeGuard},
};
-use core::{cell::UnsafeCell, marker::PhantomPinned};
+use core::{cell::UnsafeCell, ffi::CStr, marker::PhantomPinned};
use macros::pin_data;
pub mod mutex;
diff --git a/rust/kernel/sync/lock/global.rs b/rust/kernel/sync/lock/global.rs
index 4a040c29faee..7d6d1abf9279 100644
--- a/rust/kernel/sync/lock/global.rs
+++ b/rust/kernel/sync/lock/global.rs
@@ -5,13 +5,14 @@
//! Support for defining statics containing locks.
use crate::{
- str::{CStr, CStrExt as _},
+ str::CStrExt as _,
sync::lock::{Backend, Guard, Lock},
sync::{LockClassKey, LockedBy},
types::Opaque,
};
use core::{
cell::UnsafeCell,
+ ffi::CStr,
marker::{PhantomData, PhantomPinned},
};
@@ -266,7 +267,7 @@ macro_rules! global_lock {
$pub enum $name {}
impl $crate::sync::lock::GlobalLockBackend for $name {
- const NAME: &'static $crate::str::CStr =
+ const NAME: &'static core::ffi::CStr =
$crate::c_str_avoid_literals!(::core::stringify!($name));
type Item = $valuety;
type Backend = $crate::global_lock_inner!(backend $kind);
diff --git a/rust/kernel/sync/poll.rs b/rust/kernel/sync/poll.rs
index d5f17153b424..3349f48725f2 100644
--- a/rust/kernel/sync/poll.rs
+++ b/rust/kernel/sync/poll.rs
@@ -11,6 +11,7 @@
sync::{CondVar, LockClassKey},
types::Opaque,
};
+use core::ffi::CStr;
use core::ops::Deref;
/// Creates a [`PollCondVar`] initialiser with the given name and a newly-created lock class.
diff --git a/rust/kernel/workqueue.rs b/rust/kernel/workqueue.rs
index 0cd100d2aefb..25723207c2be 100644
--- a/rust/kernel/workqueue.rs
+++ b/rust/kernel/workqueue.rs
@@ -135,6 +135,7 @@
use crate::alloc::{AllocError, Flags};
use crate::{prelude::*, sync::Arc, sync::LockClassKey, types::Opaque};
+use core::ffi::CStr;
use core::marker::PhantomData;
/// Creates a [`Work`] initialiser with the given name and a newly-created lock class.
diff --git a/rust/macros/module.rs b/rust/macros/module.rs
index a534fabd29d6..7f81cbbcc4ef 100644
--- a/rust/macros/module.rs
+++ b/rust/macros/module.rs
@@ -229,7 +229,7 @@ pub(crate) fn module(ts: TokenStream) -> TokenStream {
}};
impl kernel::ModuleMetadata for {type_} {{
- const NAME: &'static kernel::str::CStr = c\"{name}\";
+ const NAME: &'static core::ffi::CStr = c\"{name}\";
}}
// Double nested modules, since then nobody can access the public items inside.
--
2.48.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v8 0/4] rust: replace kernel::str::CStr w/ core::ffi::CStr
2025-02-03 11:50 [PATCH v8 0/4] rust: replace kernel::str::CStr w/ core::ffi::CStr Tamir Duberstein
` (3 preceding siblings ...)
2025-02-03 11:50 ` [PATCH v8 4/4] rust: remove core::ffi::CStr reexport Tamir Duberstein
@ 2025-02-18 17:05 ` Tamir Duberstein
2025-02-19 14:21 ` Alice Ryhl
4 siblings, 1 reply; 12+ messages in thread
From: Tamir Duberstein @ 2025-02-18 17:05 UTC (permalink / raw)
To: Michal Rostecki, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross
Cc: rust-for-linux, linux-kernel
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>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v8 0/4] rust: replace kernel::str::CStr w/ core::ffi::CStr
2025-02-18 17:05 ` [PATCH v8 0/4] rust: replace kernel::str::CStr w/ core::ffi::CStr Tamir Duberstein
@ 2025-02-19 14:21 ` Alice Ryhl
2025-02-19 14:32 ` Tamir Duberstein
2025-02-21 14:28 ` Gary Guo
0 siblings, 2 replies; 12+ messages in thread
From: Alice Ryhl @ 2025-02-19 14:21 UTC (permalink / raw)
To: Tamir Duberstein
Cc: Michal Rostecki, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, rust-for-linux, linux-kernel
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v8 0/4] rust: replace kernel::str::CStr w/ core::ffi::CStr
2025-02-19 14:21 ` Alice Ryhl
@ 2025-02-19 14:32 ` Tamir Duberstein
2025-02-20 19:48 ` Alice Ryhl
2025-02-21 14:28 ` Gary Guo
1 sibling, 1 reply; 12+ messages in thread
From: Tamir Duberstein @ 2025-02-19 14:32 UTC (permalink / raw)
To: Alice Ryhl
Cc: Michal Rostecki, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, rust-for-linux, linux-kernel
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?
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v8 0/4] rust: replace kernel::str::CStr w/ core::ffi::CStr
2025-02-19 14:32 ` Tamir Duberstein
@ 2025-02-20 19:48 ` Alice Ryhl
0 siblings, 0 replies; 12+ messages in thread
From: Alice Ryhl @ 2025-02-20 19:48 UTC (permalink / raw)
To: Tamir Duberstein
Cc: Michal Rostecki, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, rust-for-linux, linux-kernel
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v8 0/4] rust: replace kernel::str::CStr w/ core::ffi::CStr
2025-02-19 14:21 ` Alice Ryhl
2025-02-19 14:32 ` Tamir Duberstein
@ 2025-02-21 14:28 ` Gary Guo
2025-02-21 15:59 ` Tamir Duberstein
1 sibling, 1 reply; 12+ messages in thread
From: Gary Guo @ 2025-02-21 14:28 UTC (permalink / raw)
To: Alice Ryhl
Cc: Tamir Duberstein, Michal Rostecki, Miguel Ojeda, Alex Gaynor,
Boqun Feng, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, rust-for-linux, linux-kernel
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v8 0/4] rust: replace kernel::str::CStr w/ core::ffi::CStr
2025-02-21 14:28 ` Gary Guo
@ 2025-02-21 15:59 ` Tamir Duberstein
2025-03-14 14:15 ` Tamir Duberstein
0 siblings, 1 reply; 12+ messages in thread
From: Tamir Duberstein @ 2025-02-21 15:59 UTC (permalink / raw)
To: Gary Guo
Cc: Alice Ryhl, Michal Rostecki, Miguel Ojeda, Alex Gaynor,
Boqun Feng, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, rust-for-linux, linux-kernel
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.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v8 0/4] rust: replace kernel::str::CStr w/ core::ffi::CStr
2025-02-21 15:59 ` Tamir Duberstein
@ 2025-03-14 14:15 ` Tamir Duberstein
0 siblings, 0 replies; 12+ messages in thread
From: Tamir Duberstein @ 2025-03-14 14:15 UTC (permalink / raw)
To: Gary Guo
Cc: Alice Ryhl, Michal Rostecki, Miguel Ojeda, Alex Gaynor,
Boqun Feng, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, rust-for-linux, linux-kernel
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.
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2025-03-14 14:15 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-02-03 11:50 [PATCH v8 0/4] rust: replace kernel::str::CStr w/ core::ffi::CStr Tamir Duberstein
2025-02-03 11:50 ` [PATCH v8 1/4] rust: move BStr,CStr Display impls behind method Tamir Duberstein
2025-02-03 11:50 ` [PATCH v8 2/4] rust: replace `CStr` with `core::ffi::CStr` Tamir Duberstein
2025-02-03 11:50 ` [PATCH v8 3/4] rust: replace `kernel::c_str!` with C-Strings Tamir Duberstein
2025-02-03 11:50 ` [PATCH v8 4/4] rust: remove core::ffi::CStr reexport Tamir Duberstein
2025-02-18 17:05 ` [PATCH v8 0/4] rust: replace kernel::str::CStr w/ core::ffi::CStr Tamir Duberstein
2025-02-19 14:21 ` Alice Ryhl
2025-02-19 14:32 ` Tamir Duberstein
2025-02-20 19:48 ` Alice Ryhl
2025-02-21 14:28 ` Gary Guo
2025-02-21 15:59 ` Tamir Duberstein
2025-03-14 14:15 ` Tamir Duberstein
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).