* [PATCH v6 0/4] rust: replace kernel::str::CStr w/ core::ffi::CStr
@ 2025-02-02 12:20 Tamir Duberstein
2025-02-02 12:20 ` [PATCH v6 1/4] rust: move BStr,CStr Display impls behind method Tamir Duberstein
` (3 more replies)
0 siblings, 4 replies; 9+ messages in thread
From: Tamir Duberstein @ 2025-02-02 12:20 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 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/gpu/drm/drm_panic_qr.rs | 2 +-
drivers/net/phy/ax88796b_rust.rs | 7 +-
drivers/net/phy/qt2025.rs | 5 +-
rust/kernel/device.rs | 5 +-
rust/kernel/error.rs | 10 +-
rust/kernel/firmware.rs | 9 +-
rust/kernel/kunit.rs | 18 +-
rust/kernel/miscdevice.rs | 5 +-
rust/kernel/net/phy.rs | 8 +-
rust/kernel/prelude.rs | 2 +-
rust/kernel/seq_file.rs | 4 +-
rust/kernel/str.rs | 579 +++++++++++----------------------------
rust/kernel/sync.rs | 4 +-
rust/kernel/sync/condvar.rs | 4 +-
rust/kernel/sync/lock.rs | 5 +-
rust/kernel/sync/lock/global.rs | 7 +-
rust/kernel/sync/poll.rs | 1 +
rust/kernel/workqueue.rs | 3 +-
18 files changed, 210 insertions(+), 468 deletions(-)
---
base-commit: bdf3d0e99c106932167de56141be5092762bab48
change-id: 20250201-cstr-core-d4b9b69120cf
Best regards,
--
Tamir Duberstein <tamird@gmail.com>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v6 1/4] rust: move BStr,CStr Display impls behind method
2025-02-02 12:20 [PATCH v6 0/4] rust: replace kernel::str::CStr w/ core::ffi::CStr Tamir Duberstein
@ 2025-02-02 12:20 ` Tamir Duberstein
2025-02-02 12:20 ` [PATCH v6 2/4] rust: replace `CStr` with `core::ffi::CStr` Tamir Duberstein
` (2 subsequent siblings)
3 siblings, 0 replies; 9+ messages in thread
From: Tamir Duberstein @ 2025-02-02 12:20 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 | 120 +++++++++++++++++++++++++++++++++++----------------
2 files changed, 89 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..53450c956a6a 100644
--- a/rust/kernel/str.rs
+++ b/rust/kernel/str.rs
@@ -31,29 +31,76 @@ 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>(())
+/// ```
+///
+/// [`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 +115,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 +423,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 +461,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 +641,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 +658,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 +676,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] 9+ messages in thread
* [PATCH v6 2/4] rust: replace `CStr` with `core::ffi::CStr`
2025-02-02 12:20 [PATCH v6 0/4] rust: replace kernel::str::CStr w/ core::ffi::CStr Tamir Duberstein
2025-02-02 12:20 ` [PATCH v6 1/4] rust: move BStr,CStr Display impls behind method Tamir Duberstein
@ 2025-02-02 12:20 ` Tamir Duberstein
2025-02-02 12:54 ` Tamir Duberstein
2025-02-02 12:20 ` [PATCH v6 3/4] rust: replace `kernel::c_str!` with C-Strings Tamir Duberstein
2025-02-02 12:20 ` [PATCH v6 4/4] rust: remove core::ffi::CStr reexport Tamir Duberstein
3 siblings, 1 reply; 9+ messages in thread
From: Tamir Duberstein @ 2025-02-02 12:20 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.
`kernel::str::CStr::{from,as}_char_ptr` live on as free functions in
`kernel::str` to paper over the difference between `core::ffi::c_char`
and `kernel::ffi::char`; see the code comment on the latter type for
details.
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>
---
drivers/gpu/drm/drm_panic_qr.rs | 2 +-
rust/kernel/device.rs | 5 +-
rust/kernel/error.rs | 4 +-
rust/kernel/firmware.rs | 2 +-
rust/kernel/kunit.rs | 4 +-
rust/kernel/miscdevice.rs | 2 +-
rust/kernel/net/phy.rs | 2 +-
rust/kernel/prelude.rs | 5 +-
rust/kernel/seq_file.rs | 4 +-
rust/kernel/str.rs | 449 ++++++----------------------------------
rust/kernel/sync/condvar.rs | 2 +-
rust/kernel/sync/lock.rs | 2 +-
rust/kernel/sync/lock/global.rs | 2 +-
rust/kernel/workqueue.rs | 2 +-
14 files changed, 88 insertions(+), 399 deletions(-)
diff --git a/drivers/gpu/drm/drm_panic_qr.rs b/drivers/gpu/drm/drm_panic_qr.rs
index bcf248f69252..ef294017888e 100644
--- a/drivers/gpu/drm/drm_panic_qr.rs
+++ b/drivers/gpu/drm/drm_panic_qr.rs
@@ -958,7 +958,7 @@ fn draw_all(&mut self, data: impl Iterator<Item = u8>) {
} else {
// SAFETY: The caller ensures that `url` is a valid pointer to a
// nul-terminated string.
- let url_cstr: &CStr = unsafe { CStr::from_char_ptr(url) };
+ let url_cstr: &CStr = unsafe { kernel::str::from_char_ptr(url) };
let segments = &[
&Segment::Binary(url_cstr.as_bytes()),
&Segment::Numeric(&data_slice[0..data_len]),
diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs
index d5e6a19ff6b7..f97c409aaba9 100644
--- a/rust/kernel/device.rs
+++ b/rust/kernel/device.rs
@@ -10,9 +10,6 @@
};
use core::{fmt, ptr};
-#[cfg(CONFIG_PRINTK)]
-use crate::c_str;
-
/// A reference-counted device.
///
/// This structure represents the Rust abstraction for a C `struct device`. This implementation
@@ -175,7 +172,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(),
+ crate::str::as_char_ptr(c"%pA"),
&msg as *const _ as *const crate::ffi::c_void,
)
};
diff --git a/rust/kernel/error.rs b/rust/kernel/error.rs
index f6ecf09cb65f..51c5e0f0a0bc 100644
--- a/rust/kernel/error.rs
+++ b/rust/kernel/error.rs
@@ -164,7 +164,7 @@ pub fn name(&self) -> Option<&'static CStr> {
None
} else {
// SAFETY: The string returned by `errname` is static and `NUL`-terminated.
- Some(unsafe { CStr::from_char_ptr(ptr) })
+ Some(unsafe { crate::str::from_char_ptr(ptr) })
}
}
@@ -187,7 +187,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/firmware.rs b/rust/kernel/firmware.rs
index c5162fdc95ff..e75db4d825ce 100644
--- a/rust/kernel/firmware.rs
+++ b/rust/kernel/firmware.rs
@@ -61,7 +61,7 @@ fn request_internal(name: &CStr, dev: &Device, func: FwFunc) -> Result<Self> {
// SAFETY: `pfw` is a valid pointer to a NULL initialized `bindings::firmware` pointer.
// `name` and `dev` are valid as by their type invariants.
- let ret = unsafe { func.0(pfw as _, name.as_char_ptr(), dev.as_raw()) };
+ let ret = unsafe { func.0(pfw as _, crate::str::as_char_ptr(name), dev.as_raw()) };
if ret != 0 {
return Err(Error::from_errno(ret));
}
diff --git a/rust/kernel/kunit.rs b/rust/kernel/kunit.rs
index 630b947c708c..9f40ea744fc2 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(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(CONDITION),
expected_true: true,
});
diff --git a/rust/kernel/miscdevice.rs b/rust/kernel/miscdevice.rs
index b3a6cc50b240..2dd20e981e9b 100644
--- a/rust/kernel/miscdevice.rs
+++ b/rust/kernel/miscdevice.rs
@@ -31,7 +31,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(self.name);
result.fops = create_vtable::<T>();
result
}
diff --git a/rust/kernel/net/phy.rs b/rust/kernel/net/phy.rs
index d7da29f95e43..9e410de3c3a3 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(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/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..bb3c49fd4ef3 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, 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(),
+ crate::str::as_char_ptr(c"%pA"),
&args as *const _ as *const crate::ffi::c_void,
);
}
diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
index 53450c956a6a..2b63cfaaa981 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>(())
/// ```
///
@@ -119,11 +119,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 {
@@ -176,254 +176,10 @@ 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,
-
- /// Supplied bytes are not terminated by `NUL`.
- NotNulTerminated,
-}
-
-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
- }
-
- /// Wraps a raw C string pointer.
- ///
- /// # Safety
- ///
- /// `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) }
- }
-
- /// Creates a mutable [`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 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) }
- }
-
- /// 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()) }
- }
-
- /// 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)
- }
-
- /// Converts this [`CStr`] to its ASCII lower case equivalent in-place.
- ///
- /// ASCII letters 'A' to 'Z' are mapped to 'a' to 'z',
- /// but non-ASCII letters are unchanged.
- ///
- /// To return a new lowercased value without modifying the existing one, use
- /// [`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();
- }
-
- /// Converts this [`CStr`] to its ASCII upper case equivalent in-place.
- ///
- /// ASCII letters 'a' to 'z' are mapped to 'A' to 'Z',
- /// but non-ASCII letters are unchanged.
- ///
- /// To return a new uppercased value without modifying the existing one, use
- /// [`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();
- }
-
- /// Returns a copy of this [`CString`] where each character is mapped to its
- /// ASCII lower case equivalent.
- ///
- /// ASCII letters 'A' to 'Z' are mapped to 'a' to 'z',
- /// but non-ASCII letters are unchanged.
- ///
- /// 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)
- }
-
- /// Returns a copy of this [`CString`] where each character is mapped to its
- /// ASCII upper case equivalent.
- ///
- /// ASCII letters 'a' to 'z' are mapped to 'A' to 'Z',
- /// but non-ASCII letters are unchanged.
- ///
- /// 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)
- }
+pub use core::ffi::CStr;
+/// Extensions to [`CStr`].
+pub trait CStrExt {
/// 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.
@@ -439,114 +195,59 @@ 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>(())
/// ```
+ fn display(&self) -> Display<'_>;
+}
+
+impl CStrExt for CStr {
#[inline]
- pub fn display(&self) -> Display<'_> {
+ fn display(&self) -> Display<'_> {
Display {
- inner: self,
+ inner: BStr::from_bytes(self.to_bytes()),
escape_common: false,
}
}
}
-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("\"")
- }
-}
-
-impl AsRef<BStr> for CStr {
- #[inline]
- fn as_ref(&self) -> &BStr {
- BStr::from_bytes(self.as_bytes())
- }
-}
-
-impl Deref for CStr {
- type Target = BStr;
-
- #[inline]
- fn deref(&self) -> &Self::Target {
- self.as_ref()
- }
-}
-
-impl Index<ops::RangeFrom<usize>> for CStr {
- type Output = CStr;
-
- #[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..]) }
- }
-}
-
-impl Index<ops::RangeFull> for CStr {
- type Output = CStr;
-
- #[inline]
- fn index(&self, _index: ops::RangeFull) -> &Self::Output {
- self
- }
-}
-
-mod private {
- use core::ops;
-
- // Marker trait for index types that can be forward to `BStr`.
- pub trait CStrIndex {}
-
- impl CStrIndex for usize {}
- impl CStrIndex for ops::Range<usize> {}
- impl CStrIndex for ops::RangeInclusive<usize> {}
- impl CStrIndex for ops::RangeToInclusive<usize> {}
+/// Wraps a raw C string with a safe C string wrapper.
+///
+/// # Safety
+///
+/// * The memory pointed to by `ptr` must contain a valid nul terminator at the end of the string.
+///
+/// * `ptr` must be [valid] for reads of bytes up to and including the nul terminator. This means in
+/// particular:
+///
+/// * The entire memory range of this `CStr` must be contained within a single allocated object!
+/// * `ptr` must be non-null even for a zero-length cstr.
+///
+/// * The memory referenced by the returned `CStr` must not be mutated for the duration of lifetime
+/// `'a`.
+///
+/// * The nul terminator must be within `isize::MAX` from `ptr`
+// 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`.
+#[inline]
+pub unsafe fn from_char_ptr<'a>(ptr: *const crate::ffi::c_char) -> &'a CStr {
+ // SAFETY: The safety preconditions are the same as for `from_ptr`.
+ unsafe { CStr::from_ptr(ptr.cast()) }
}
-impl<Idx> Index<Idx> for CStr
-where
- Idx: private::CStrIndex,
- BStr: Index<Idx>,
-{
- type Output = <BStr as Index<Idx>>::Output;
-
- #[inline]
- fn index(&self, index: Idx) -> &Self::Output {
- &self.as_ref()[index]
- }
+/// Returns a C pointer to the string.
+// 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`. 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(c_str: &CStr) -> *const crate::ffi::c_char {
+ c_str.as_ptr().cast()
}
/// Creates a new [`CStr`] from a string literal.
@@ -629,25 +330,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]
@@ -663,14 +354,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]
@@ -850,14 +540,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));
@@ -915,9 +605,8 @@ fn deref(&self) -> &Self::Target {
impl DerefMut for CString {
fn deref_mut(&mut self) -> &mut Self::Target {
- // SAFETY: A `CString` is always NUL-terminated and contains no other
- // NUL bytes.
- unsafe { CStr::from_bytes_with_nul_unchecked_mut(self.buf.as_mut_slice()) }
+ // SAFETY: Properties of `bytes` guaranteed by the safety precondition.
+ unsafe { &mut *(self.buf.as_mut_slice() as *mut [u8] as *mut CStr) }
}
}
@@ -927,7 +616,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..7bf3df980433 100644
--- a/rust/kernel/sync/condvar.rs
+++ b/rust/kernel/sync/condvar.rs
@@ -107,7 +107,7 @@ pub fn new(name: &'static CStr, key: &'static LockClassKey) -> impl PinInit<Self
// SAFETY: `slot` is valid while the closure is called and both `name` and `key` have
// static lifetimes so they live indefinitely.
wait_queue_head <- Opaque::ffi_init(|slot| unsafe {
- bindings::__init_waitqueue_head(slot, name.as_char_ptr(), key.as_ptr())
+ bindings::__init_waitqueue_head(slot, crate::str::as_char_ptr(name), key.as_ptr())
}),
})
}
diff --git a/rust/kernel/sync/lock.rs b/rust/kernel/sync/lock.rs
index 41dcddac69e2..16bf3014f068 100644
--- a/rust/kernel/sync/lock.rs
+++ b/rust/kernel/sync/lock.rs
@@ -128,7 +128,7 @@ pub fn new(t: T, name: &'static CStr, key: &'static LockClassKey) -> impl PinIni
// SAFETY: `slot` is valid while the closure is called and both `name` and `key` have
// static lifetimes so they live indefinitely.
state <- Opaque::ffi_init(|slot| unsafe {
- B::init(slot, name.as_char_ptr(), key.as_ptr())
+ B::init(slot, crate::str::as_char_ptr(name), key.as_ptr())
}),
})
}
diff --git a/rust/kernel/sync/lock/global.rs b/rust/kernel/sync/lock/global.rs
index 480ee724e3cc..4c4d60a51c1f 100644
--- a/rust/kernel/sync/lock/global.rs
+++ b/rust/kernel/sync/lock/global.rs
@@ -69,7 +69,7 @@ pub unsafe fn init(&'static self) {
unsafe {
B::Backend::init(
self.inner.state.get(),
- B::NAME.as_char_ptr(),
+ crate::str::as_char_ptr(B::NAME),
B::get_lock_class().as_ptr(),
)
}
diff --git a/rust/kernel/workqueue.rs b/rust/kernel/workqueue.rs
index 1dcd53478edd..284b9a739c01 100644
--- a/rust/kernel/workqueue.rs
+++ b/rust/kernel/workqueue.rs
@@ -382,7 +382,7 @@ pub fn new(name: &'static CStr, key: &'static LockClassKey) -> impl PinInit<Self
slot,
Some(T::Pointer::run),
false,
- name.as_char_ptr(),
+ crate::str::as_char_ptr(name),
key.as_ptr(),
)
}
--
2.48.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v6 3/4] rust: replace `kernel::c_str!` with C-Strings
2025-02-02 12:20 [PATCH v6 0/4] rust: replace kernel::str::CStr w/ core::ffi::CStr Tamir Duberstein
2025-02-02 12:20 ` [PATCH v6 1/4] rust: move BStr,CStr Display impls behind method Tamir Duberstein
2025-02-02 12:20 ` [PATCH v6 2/4] rust: replace `CStr` with `core::ffi::CStr` Tamir Duberstein
@ 2025-02-02 12:20 ` Tamir Duberstein
2025-02-02 13:10 ` Dirk Behme
2025-02-02 12:20 ` [PATCH v6 4/4] rust: remove core::ffi::CStr reexport Tamir Duberstein
3 siblings, 1 reply; 9+ messages in thread
From: Tamir Duberstein @ 2025-02-02 12:20 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/firmware.rs | 4 ++--
rust/kernel/kunit.rs | 7 ++++---
rust/kernel/net/phy.rs | 6 ++----
rust/kernel/str.rs | 37 ++++++++++++++++++++++++-------------
rust/kernel/sync.rs | 4 ++--
rust/kernel/sync/lock/global.rs | 3 ++-
8 files changed, 41 insertions(+), 32 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/firmware.rs b/rust/kernel/firmware.rs
index e75db4d825ce..93e375e75ca0 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 9f40ea744fc2..3d812edec057 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 9e410de3c3a3..5fb9b0e1c9b1 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/str.rs b/rust/kernel/str.rs
index 2b63cfaaa981..20d852aa8b3c 100644
--- a/rust/kernel/str.rs
+++ b/rust/kernel/str.rs
@@ -190,14 +190,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>(())
@@ -250,25 +249,37 @@ pub const fn as_char_ptr(c_str: &CStr) -> *const crate::ffi::c_char {
c_str.as_ptr().cast()
}
-/// 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 1eab7ebf25fd..ec7f80a924c3 100644
--- a/rust/kernel/sync.rs
+++ b/rust/kernel/sync.rs
@@ -61,9 +61,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 4c4d60a51c1f..732dde45024b 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);
--
2.48.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v6 4/4] rust: remove core::ffi::CStr reexport
2025-02-02 12:20 [PATCH v6 0/4] rust: replace kernel::str::CStr w/ core::ffi::CStr Tamir Duberstein
` (2 preceding siblings ...)
2025-02-02 12:20 ` [PATCH v6 3/4] rust: replace `kernel::c_str!` with C-Strings Tamir Duberstein
@ 2025-02-02 12:20 ` Tamir Duberstein
3 siblings, 0 replies; 9+ messages in thread
From: Tamir Duberstein @ 2025-02-02 12:20 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/error.rs | 6 ++----
rust/kernel/firmware.rs | 3 ++-
rust/kernel/kunit.rs | 6 +++---
rust/kernel/miscdevice.rs | 3 +--
rust/kernel/prelude.rs | 5 +----
rust/kernel/str.rs | 19 +++++++++----------
rust/kernel/sync/condvar.rs | 2 +-
rust/kernel/sync/lock.rs | 3 +--
rust/kernel/sync/lock/global.rs | 4 ++--
rust/kernel/sync/poll.rs | 1 +
rust/kernel/workqueue.rs | 1 +
11 files changed, 24 insertions(+), 29 deletions(-)
diff --git a/rust/kernel/error.rs b/rust/kernel/error.rs
index 51c5e0f0a0bc..81972a8f695d 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 93e375e75ca0..86ed79cef211 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 3d812edec057..f83988959257 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/miscdevice.rs b/rust/kernel/miscdevice.rs
index 2dd20e981e9b..26623d2c43c0 100644
--- a/rust/kernel/miscdevice.rs
+++ b/rust/kernel/miscdevice.rs
@@ -13,10 +13,9 @@
error::{to_result, Error, Result, VTABLE_DEFAULT_ERROR},
ffi::{c_int, c_long, c_uint, c_ulong},
prelude::*,
- 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/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 20d852aa8b3c..c96ce2640ca1 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};
@@ -176,8 +177,6 @@ macro_rules! b_str {
}};
}
-pub use core::ffi::CStr;
-
/// Extensions to [`CStr`].
pub trait CStrExt {
/// Returns an object that implements [`Display`] for safely printing a [`CStr`] that may
@@ -263,7 +262,7 @@ pub const fn as_char_ptr(c_str: &CStr) -> *const crate::ffi::c_char {
///
/// ```
/// # 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]
@@ -273,13 +272,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 7bf3df980433..3b82acdcd56c 100644
--- a/rust/kernel/sync/condvar.rs
+++ b/rust/kernel/sync/condvar.rs
@@ -10,11 +10,11 @@
ffi::{c_int, c_long},
init::PinInit,
pin_init,
- str::CStr,
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 16bf3014f068..7b412f5049e5 100644
--- a/rust/kernel/sync/lock.rs
+++ b/rust/kernel/sync/lock.rs
@@ -9,10 +9,9 @@
use crate::{
init::PinInit,
pin_init,
- str::CStr,
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 732dde45024b..ee438654eb6c 100644
--- a/rust/kernel/sync/lock/global.rs
+++ b/rust/kernel/sync/lock/global.rs
@@ -5,13 +5,13 @@
//! Support for defining statics containing locks.
use crate::{
- str::CStr,
sync::lock::{Backend, Guard, Lock},
sync::{LockClassKey, LockedBy},
types::Opaque,
};
use core::{
cell::UnsafeCell,
+ ffi::CStr,
marker::{PhantomData, PhantomPinned},
};
@@ -266,7 +266,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 284b9a739c01..69bf6a1cba41 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.
--
2.48.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v6 2/4] rust: replace `CStr` with `core::ffi::CStr`
2025-02-02 12:20 ` [PATCH v6 2/4] rust: replace `CStr` with `core::ffi::CStr` Tamir Duberstein
@ 2025-02-02 12:54 ` Tamir Duberstein
2025-02-02 14:40 ` Tamir Duberstein
0 siblings, 1 reply; 9+ messages in thread
From: Tamir Duberstein @ 2025-02-02 12:54 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
On Sun, Feb 2, 2025 at 7:20 AM Tamir Duberstein <tamird@gmail.com> wrote:
>
> impl DerefMut for CString {
> fn deref_mut(&mut self) -> &mut Self::Target {
> - // SAFETY: A `CString` is always NUL-terminated and contains no other
> - // NUL bytes.
> - unsafe { CStr::from_bytes_with_nul_unchecked_mut(self.buf.as_mut_slice()) }
> + // SAFETY: Properties of `bytes` guaranteed by the safety precondition.
> + unsafe { &mut *(self.buf.as_mut_slice() as *mut [u8] as *mut CStr) }
> }
> }
Whoops, this safety comment is nonsense. I'll use the one from `impl
Deref` above in v7...or just remove this impl altogether. I'm not sure
what the use for `&mut CStr` would be.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v6 3/4] rust: replace `kernel::c_str!` with C-Strings
2025-02-02 12:20 ` [PATCH v6 3/4] rust: replace `kernel::c_str!` with C-Strings Tamir Duberstein
@ 2025-02-02 13:10 ` Dirk Behme
2025-02-02 14:18 ` Tamir Duberstein
0 siblings, 1 reply; 9+ messages in thread
From: Dirk Behme @ 2025-02-02 13:10 UTC (permalink / raw)
To: Tamir Duberstein, 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
Hi Tamir,
On 02.02.25 13:20, Tamir Duberstein wrote:
> 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/firmware.rs | 4 ++--
> rust/kernel/kunit.rs | 7 ++++---
> rust/kernel/net/phy.rs | 6 ++----
> rust/kernel/str.rs | 37 ++++++++++++++++++++++++-------------
> rust/kernel/sync.rs | 4 ++--
> rust/kernel/sync/lock/global.rs | 3 ++-
> 8 files changed, 41 insertions(+), 32 deletions(-)
Have you checked samples/rust/ [1] as well?
Best regards
Dirk
[1]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/samples/rust
> 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/firmware.rs b/rust/kernel/firmware.rs
> index e75db4d825ce..93e375e75ca0 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 9f40ea744fc2..3d812edec057 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 9e410de3c3a3..5fb9b0e1c9b1 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/str.rs b/rust/kernel/str.rs
> index 2b63cfaaa981..20d852aa8b3c 100644
> --- a/rust/kernel/str.rs
> +++ b/rust/kernel/str.rs
> @@ -190,14 +190,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>(())
> @@ -250,25 +249,37 @@ pub const fn as_char_ptr(c_str: &CStr) -> *const crate::ffi::c_char {
> c_str.as_ptr().cast()
> }
>
> -/// 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 1eab7ebf25fd..ec7f80a924c3 100644
> --- a/rust/kernel/sync.rs
> +++ b/rust/kernel/sync.rs
> @@ -61,9 +61,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 4c4d60a51c1f..732dde45024b 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);
>
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v6 3/4] rust: replace `kernel::c_str!` with C-Strings
2025-02-02 13:10 ` Dirk Behme
@ 2025-02-02 14:18 ` Tamir Duberstein
0 siblings, 0 replies; 9+ messages in thread
From: Tamir Duberstein @ 2025-02-02 14:18 UTC (permalink / raw)
To: Dirk Behme
Cc: Michal Rostecki, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, rust-for-linux, linux-kernel
On Sun, Feb 2, 2025 at 8:10 AM Dirk Behme <dirk.behme@gmail.com> wrote:
>
> Hi Tamir,
>
> On 02.02.25 13:20, Tamir Duberstein wrote:
> > 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/firmware.rs | 4 ++--
> > rust/kernel/kunit.rs | 7 ++++---
> > rust/kernel/net/phy.rs | 6 ++----
> > rust/kernel/str.rs | 37 ++++++++++++++++++++++++-------------
> > rust/kernel/sync.rs | 4 ++--
> > rust/kernel/sync/lock/global.rs | 3 ++-
> > 8 files changed, 41 insertions(+), 32 deletions(-)
>
>
> Have you checked samples/rust/ [1] as well?
>
> Best regards
>
> Dirk
Hi Dirk, I had, but I was working on rust-next rather than on
torvalds/master. I'll update the other samples in v7.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v6 2/4] rust: replace `CStr` with `core::ffi::CStr`
2025-02-02 12:54 ` Tamir Duberstein
@ 2025-02-02 14:40 ` Tamir Duberstein
0 siblings, 0 replies; 9+ messages in thread
From: Tamir Duberstein @ 2025-02-02 14:40 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
On Sun, Feb 2, 2025 at 7:54 AM Tamir Duberstein <tamird@gmail.com> wrote:
>
> On Sun, Feb 2, 2025 at 7:20 AM Tamir Duberstein <tamird@gmail.com> wrote:
> >
> > impl DerefMut for CString {
> > fn deref_mut(&mut self) -> &mut Self::Target {
> > - // SAFETY: A `CString` is always NUL-terminated and contains no other
> > - // NUL bytes.
> > - unsafe { CStr::from_bytes_with_nul_unchecked_mut(self.buf.as_mut_slice()) }
> > + // SAFETY: Properties of `bytes` guaranteed by the safety precondition.
> > + unsafe { &mut *(self.buf.as_mut_slice() as *mut [u8] as *mut CStr) }
> > }
> > }
>
> Whoops, this safety comment is nonsense. I'll use the one from `impl
> Deref` above in v7...or just remove this impl altogether. I'm not sure
> what the use for `&mut CStr` would be.
Ah, turns out Michal's patches removed functionality added in commit
a321f3ad0a5d ("rust: str: add {make,to}_{upper,lower}case() to
CString") which relied on this DerefMut impl. I've restored it all in
v7.
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2025-02-02 14:40 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-02-02 12:20 [PATCH v6 0/4] rust: replace kernel::str::CStr w/ core::ffi::CStr Tamir Duberstein
2025-02-02 12:20 ` [PATCH v6 1/4] rust: move BStr,CStr Display impls behind method Tamir Duberstein
2025-02-02 12:20 ` [PATCH v6 2/4] rust: replace `CStr` with `core::ffi::CStr` Tamir Duberstein
2025-02-02 12:54 ` Tamir Duberstein
2025-02-02 14:40 ` Tamir Duberstein
2025-02-02 12:20 ` [PATCH v6 3/4] rust: replace `kernel::c_str!` with C-Strings Tamir Duberstein
2025-02-02 13:10 ` Dirk Behme
2025-02-02 14:18 ` Tamir Duberstein
2025-02-02 12:20 ` [PATCH v6 4/4] rust: remove core::ffi::CStr reexport Tamir Duberstein
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.