* [PATCH v10 1/5] rust: retitle "Example" section as "Examples"
2025-05-24 20:33 [PATCH v10 0/5] rust: replace kernel::str::CStr w/ core::ffi::CStr Tamir Duberstein
@ 2025-05-24 20:33 ` Tamir Duberstein
2025-05-26 11:28 ` Benno Lossin
2025-05-26 16:15 ` Miguel Ojeda
2025-05-24 20:33 ` [PATCH v10 2/5] rust: support formatting of foreign types Tamir Duberstein
` (4 subsequent siblings)
5 siblings, 2 replies; 34+ messages in thread
From: Tamir Duberstein @ 2025-05-24 20:33 UTC (permalink / raw)
To: Michal Rostecki, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Brendan Higgins, David Gow, Rae Moar, Danilo Krummrich,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Greg Kroah-Hartman, Rafael J. Wysocki,
Luis Chamberlain, Russ Weight, FUJITA Tomonori, Rob Herring,
Saravana Kannan, Peter Zijlstra, Ingo Molnar, Will Deacon,
Waiman Long, Nathan Chancellor, Nick Desaulniers, Bill Wendling,
Justin Stitt, Andrew Lunn, Heiner Kallweit, Russell King,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Bjorn Helgaas, Arnd Bergmann, Jens Axboe, Benno Lossin,
Krzysztof Wilczyński, Benno Lossin
Cc: rust-for-linux, linux-kernel, linux-kselftest, kunit-dev,
dri-devel, netdev, devicetree, llvm, linux-pci, nouveau,
linux-block, Tamir Duberstein
This title is consistent with all other macros' documentation,
regardless of the number of examples contained in their "Examples"
sections.
Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
rust/macros/lib.rs | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/rust/macros/lib.rs b/rust/macros/lib.rs
index 9acaa68c974e..d31e50c446b0 100644
--- a/rust/macros/lib.rs
+++ b/rust/macros/lib.rs
@@ -263,7 +263,7 @@ pub fn concat_idents(ts: TokenStream) -> TokenStream {
/// literals (lifetimes and documentation strings are not supported). There is a difference in
/// supported modifiers as well.
///
-/// # Example
+/// # Examples
///
/// ```
/// # const binder_driver_return_protocol_BR_OK: u32 = 0;
--
2.49.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [PATCH v10 1/5] rust: retitle "Example" section as "Examples"
2025-05-24 20:33 ` [PATCH v10 1/5] rust: retitle "Example" section as "Examples" Tamir Duberstein
@ 2025-05-26 11:28 ` Benno Lossin
2025-05-26 16:15 ` Miguel Ojeda
1 sibling, 0 replies; 34+ messages in thread
From: Benno Lossin @ 2025-05-26 11:28 UTC (permalink / raw)
To: Tamir Duberstein, Michal Rostecki, Miguel Ojeda, Alex Gaynor,
Boqun Feng, Gary Guo, Björn Roy Baron, Andreas Hindborg,
Alice Ryhl, Trevor Gross, Brendan Higgins, David Gow, Rae Moar,
Danilo Krummrich, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter,
Greg Kroah-Hartman, Rafael J. Wysocki, Luis Chamberlain,
Russ Weight, FUJITA Tomonori, Rob Herring, Saravana Kannan,
Peter Zijlstra, Ingo Molnar, Will Deacon, Waiman Long,
Nathan Chancellor, Nick Desaulniers, Bill Wendling, Justin Stitt,
Andrew Lunn, Heiner Kallweit, Russell King, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Bjorn Helgaas,
Arnd Bergmann, Jens Axboe, Krzysztof Wilczyński
Cc: rust-for-linux, linux-kernel, linux-kselftest, kunit-dev,
dri-devel, netdev, devicetree, llvm, linux-pci, nouveau,
linux-block
On Sat May 24, 2025 at 10:33 PM CEST, Tamir Duberstein wrote:
> This title is consistent with all other macros' documentation,
> regardless of the number of examples contained in their "Examples"
> sections.
>
> Signed-off-by: Tamir Duberstein <tamird@gmail.com>
Reviewed-by: Benno Lossin <lossin@kernel.org>
---
Cheers,
Benno
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v10 1/5] rust: retitle "Example" section as "Examples"
2025-05-24 20:33 ` [PATCH v10 1/5] rust: retitle "Example" section as "Examples" Tamir Duberstein
2025-05-26 11:28 ` Benno Lossin
@ 2025-05-26 16:15 ` Miguel Ojeda
2025-05-26 20:41 ` Tamir Duberstein
1 sibling, 1 reply; 34+ messages in thread
From: Miguel Ojeda @ 2025-05-26 16:15 UTC (permalink / raw)
To: Tamir Duberstein, Patrick Miller, Hridesh MG
Cc: Michal Rostecki, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Brendan Higgins, David Gow, Rae Moar, Danilo Krummrich,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Greg Kroah-Hartman, Rafael J. Wysocki,
Luis Chamberlain, Russ Weight, FUJITA Tomonori, Rob Herring,
Saravana Kannan, Peter Zijlstra, Ingo Molnar, Will Deacon,
Waiman Long, Nathan Chancellor, Nick Desaulniers, Bill Wendling,
Justin Stitt, Andrew Lunn, Heiner Kallweit, Russell King,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Bjorn Helgaas, Arnd Bergmann, Jens Axboe, Benno Lossin,
Krzysztof Wilczyński, rust-for-linux, linux-kernel,
linux-kselftest, kunit-dev, dri-devel, netdev, devicetree, llvm,
linux-pci, nouveau, linux-block
On Sat, May 24, 2025 at 10:33 PM Tamir Duberstein <tamird@gmail.com> wrote:
>
> This title is consistent with all other macros' documentation,
> regardless of the number of examples contained in their "Examples"
> sections.
>
> Signed-off-by: Tamir Duberstein <tamird@gmail.com>
I was going to say that I could take this one independently, but we
already had #1 of:
https://lore.kernel.org/rust-for-linux/20240906164448.2268368-1-paddymills@proton.me/
I will take that one (which given the `checkpatch.pl` one got stalled,
I should have taken it separately as I mentioned at some point).
Patrick/Hridesh: there are new cases arriving (i.e. singular section
names), so it would be great if the `checkpatch.pl` patch discussion
could be restarted to see if we can land it, i.e. there is now even
more justification behind it just after some months. Thanks!
Cheers,
Miguel
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH v10 1/5] rust: retitle "Example" section as "Examples"
2025-05-26 16:15 ` Miguel Ojeda
@ 2025-05-26 20:41 ` Tamir Duberstein
0 siblings, 0 replies; 34+ messages in thread
From: Tamir Duberstein @ 2025-05-26 20:41 UTC (permalink / raw)
To: Miguel Ojeda
Cc: Patrick Miller, Hridesh MG, Michal Rostecki, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Brendan Higgins,
David Gow, Rae Moar, Danilo Krummrich, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
Greg Kroah-Hartman, Rafael J. Wysocki, Luis Chamberlain,
Russ Weight, FUJITA Tomonori, Rob Herring, Saravana Kannan,
Peter Zijlstra, Ingo Molnar, Will Deacon, Waiman Long,
Nathan Chancellor, Nick Desaulniers, Bill Wendling, Justin Stitt,
Andrew Lunn, Heiner Kallweit, Russell King, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Bjorn Helgaas,
Arnd Bergmann, Jens Axboe, Benno Lossin,
Krzysztof Wilczyński, rust-for-linux, linux-kernel,
linux-kselftest, kunit-dev, dri-devel, netdev, devicetree, llvm,
linux-pci, nouveau, linux-block
On Mon, May 26, 2025 at 12:15 PM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> On Sat, May 24, 2025 at 10:33 PM Tamir Duberstein <tamird@gmail.com> wrote:
> >
> > This title is consistent with all other macros' documentation,
> > regardless of the number of examples contained in their "Examples"
> > sections.
> >
> > Signed-off-by: Tamir Duberstein <tamird@gmail.com>
>
> I was going to say that I could take this one independently, but we
> already had #1 of:
>
> https://lore.kernel.org/rust-for-linux/20240906164448.2268368-1-paddymills@proton.me/
>
> I will take that one (which given the `checkpatch.pl` one got stalled,
> I should have taken it separately as I mentioned at some point).
Sounds good. I'll drop this one.
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH v10 2/5] rust: support formatting of foreign types
2025-05-24 20:33 [PATCH v10 0/5] rust: replace kernel::str::CStr w/ core::ffi::CStr Tamir Duberstein
2025-05-24 20:33 ` [PATCH v10 1/5] rust: retitle "Example" section as "Examples" Tamir Duberstein
@ 2025-05-24 20:33 ` Tamir Duberstein
2025-05-26 14:48 ` Benno Lossin
2025-05-24 20:33 ` [PATCH v10 3/5] rust: replace `CStr` with `core::ffi::CStr` Tamir Duberstein
` (3 subsequent siblings)
5 siblings, 1 reply; 34+ messages in thread
From: Tamir Duberstein @ 2025-05-24 20:33 UTC (permalink / raw)
To: Michal Rostecki, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Brendan Higgins, David Gow, Rae Moar, Danilo Krummrich,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Greg Kroah-Hartman, Rafael J. Wysocki,
Luis Chamberlain, Russ Weight, FUJITA Tomonori, Rob Herring,
Saravana Kannan, Peter Zijlstra, Ingo Molnar, Will Deacon,
Waiman Long, Nathan Chancellor, Nick Desaulniers, Bill Wendling,
Justin Stitt, Andrew Lunn, Heiner Kallweit, Russell King,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Bjorn Helgaas, Arnd Bergmann, Jens Axboe, Benno Lossin,
Krzysztof Wilczyński, Benno Lossin
Cc: rust-for-linux, linux-kernel, linux-kselftest, kunit-dev,
dri-devel, netdev, devicetree, llvm, linux-pci, nouveau,
linux-block, Tamir Duberstein
Introduce a `fmt!` macro which wraps all arguments in
`kernel::fmt::Adapter` This enables formatting of foreign types (like
`core::ffi::CStr`) that do not implement `fmt::Display` due to concerns
around lossy conversions which do not apply in the kernel.
Replace all direct calls to `format_args!` with `fmt!`.
In preparation for replacing our `CStr` with `core::ffi::CStr`, move its
`fmt::Display` implementation to `kernel::fmt::Adapter<&CStr>`.
Suggested-by: Alice Ryhl <aliceryhl@google.com>
Link: https://rust-for-linux.zulipchat.com/#narrow/channel/288089-General/topic/Custom.20formatting/with/516476467
Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
drivers/block/rnull.rs | 2 +-
rust/kernel/block/mq.rs | 2 +-
rust/kernel/device.rs | 2 +-
rust/kernel/fmt.rs | 77 +++++++++++++++++++++++++++++
rust/kernel/kunit.rs | 6 +--
rust/kernel/lib.rs | 1 +
rust/kernel/prelude.rs | 3 +-
rust/kernel/print.rs | 4 +-
rust/kernel/seq_file.rs | 2 +-
rust/kernel/str.rs | 23 ++++-----
rust/macros/fmt.rs | 118 ++++++++++++++++++++++++++++++++++++++++++++
rust/macros/lib.rs | 19 +++++++
scripts/rustdoc_test_gen.rs | 2 +-
13 files changed, 235 insertions(+), 26 deletions(-)
diff --git a/drivers/block/rnull.rs b/drivers/block/rnull.rs
index d07e76ae2c13..6366da12c5a5 100644
--- a/drivers/block/rnull.rs
+++ b/drivers/block/rnull.rs
@@ -51,7 +51,7 @@ fn init(_module: &'static ThisModule) -> impl PinInit<Self, Error> {
.logical_block_size(4096)?
.physical_block_size(4096)?
.rotational(false)
- .build(format_args!("rnullb{}", 0), tagset)
+ .build(fmt!("rnullb{}", 0), tagset)
})();
try_pin_init!(Self {
diff --git a/rust/kernel/block/mq.rs b/rust/kernel/block/mq.rs
index fb0f393c1cea..842be88aa1cf 100644
--- a/rust/kernel/block/mq.rs
+++ b/rust/kernel/block/mq.rs
@@ -82,7 +82,7 @@
//! Arc::pin_init(TagSet::new(1, 256, 1), flags::GFP_KERNEL)?;
//! let mut disk = gen_disk::GenDiskBuilder::new()
//! .capacity_sectors(4096)
-//! .build(format_args!("myblk"), tagset)?;
+//! .build(fmt!("myblk"), tagset)?;
//!
//! # Ok::<(), kernel::error::Error>(())
//! ```
diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs
index 5c372cf27ed0..99d99a76934c 100644
--- a/rust/kernel/device.rs
+++ b/rust/kernel/device.rs
@@ -240,7 +240,7 @@ impl DeviceContext for Normal {}
macro_rules! dev_printk {
($method:ident, $dev:expr, $($f:tt)*) => {
{
- ($dev).$method(core::format_args!($($f)*));
+ ($dev).$method($crate::prelude::fmt!($($f)*));
}
}
}
diff --git a/rust/kernel/fmt.rs b/rust/kernel/fmt.rs
new file mode 100644
index 000000000000..12b08debc3b3
--- /dev/null
+++ b/rust/kernel/fmt.rs
@@ -0,0 +1,77 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Formatting utilities.
+
+use core::fmt;
+
+/// Internal adapter used to route allow implementations of formatting traits for foreign types.
+///
+/// It is inserted automatically by the [`fmt!`] macro and is not meant to be used directly.
+///
+/// [`fmt!`]: crate::prelude::fmt!
+#[doc(hidden)]
+pub struct Adapter<T>(pub T);
+
+macro_rules! impl_fmt_adapter_forward {
+ ($($trait:ident),* $(,)?) => {
+ $(
+ impl<T: fmt::$trait> fmt::$trait for Adapter<T> {
+ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+ let Self(t) = self;
+ fmt::$trait::fmt(t, f)
+ }
+ }
+ )*
+ };
+}
+
+impl_fmt_adapter_forward!(Debug, LowerHex, UpperHex, Octal, Binary, Pointer, LowerExp, UpperExp);
+
+macro_rules! impl_display_forward {
+ ($(
+ $( { $($generics:tt)* } )? $ty:ty $( { where $($where:tt)* } )?
+ ),* $(,)?) => {
+ $(
+ impl$($($generics)*)? fmt::Display for Adapter<&$ty>
+ $(where $($where)*)? {
+ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+ let Self(t) = self;
+ fmt::Display::fmt(t, f)
+ }
+ }
+ )*
+ };
+}
+
+impl<T: ?Sized> fmt::Display for Adapter<&&T>
+where
+ for<'a> Adapter<&'a T>: fmt::Display,
+{
+ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+ let Self(t) = self;
+ Adapter::<&T>(**t).fmt(f)
+ }
+}
+
+impl_display_forward!(
+ bool,
+ char,
+ core::panic::PanicInfo<'_>,
+ crate::str::BStr,
+ fmt::Arguments<'_>,
+ i128,
+ i16,
+ i32,
+ i64,
+ i8,
+ isize,
+ str,
+ u128,
+ u16,
+ u32,
+ u64,
+ u8,
+ usize,
+ {<T: ?Sized>} crate::sync::Arc<T> {where crate::sync::Arc<T>: fmt::Display},
+ {<T: ?Sized>} crate::sync::UniqueArc<T> {where crate::sync::UniqueArc<T>: fmt::Display},
+);
diff --git a/rust/kernel/kunit.rs b/rust/kernel/kunit.rs
index 1604fb6a5b1b..c29e34192553 100644
--- a/rust/kernel/kunit.rs
+++ b/rust/kernel/kunit.rs
@@ -72,14 +72,14 @@ macro_rules! kunit_assert {
// mistake (it is hidden to prevent that).
//
// This mimics KUnit's failed assertion format.
- $crate::kunit::err(format_args!(
+ $crate::kunit::err($crate::prelude::fmt!(
" # {}: ASSERTION FAILED at {FILE}:{LINE}\n",
$name
));
- $crate::kunit::err(format_args!(
+ $crate::kunit::err($crate::prelude::fmt!(
" Expected {CONDITION} to be true, but is false\n"
));
- $crate::kunit::err(format_args!(
+ $crate::kunit::err($crate::prelude::fmt!(
" Failure not reported to KUnit since this is a non-KUnit task\n"
));
break 'out;
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index 6e9287136cac..ec48c818d512 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -66,6 +66,7 @@
pub mod faux;
#[cfg(CONFIG_RUST_FW_LOADER_ABSTRACTIONS)]
pub mod firmware;
+pub mod fmt;
pub mod fs;
pub mod init;
pub mod io;
diff --git a/rust/kernel/prelude.rs b/rust/kernel/prelude.rs
index baa774a351ce..ef1efcb9d945 100644
--- a/rust/kernel/prelude.rs
+++ b/rust/kernel/prelude.rs
@@ -17,7 +17,7 @@
pub use crate::alloc::{flags::*, Box, KBox, KVBox, KVVec, KVec, VBox, VVec, Vec};
#[doc(no_inline)]
-pub use macros::{export, module, vtable};
+pub use macros::{export, fmt, module, vtable};
pub use pin_init::{init, pin_data, pin_init, pinned_drop, InPlaceWrite, Init, PinInit, Zeroable};
@@ -26,7 +26,6 @@
// `super::std_vendor` is hidden, which makes the macro inline for some reason.
#[doc(no_inline)]
pub use super::dbg;
-pub use super::fmt;
pub use super::{dev_alert, dev_crit, dev_dbg, dev_emerg, dev_err, dev_info, dev_notice, dev_warn};
pub use super::{pr_alert, pr_crit, pr_debug, pr_emerg, pr_err, pr_info, pr_notice, pr_warn};
diff --git a/rust/kernel/print.rs b/rust/kernel/print.rs
index 9783d960a97a..0f5e15128005 100644
--- a/rust/kernel/print.rs
+++ b/rust/kernel/print.rs
@@ -149,7 +149,7 @@ macro_rules! print_macro (
// takes borrows on the arguments, but does not extend the scope of temporaries.
// Therefore, a `match` expression is used to keep them around, since
// the scrutinee is kept until the end of the `match`.
- match format_args!($($arg)+) {
+ match $crate::prelude::fmt!($($arg)+) {
// SAFETY: This hidden macro should only be called by the documented
// printing macros which ensure the format string is one of the fixed
// ones. All `__LOG_PREFIX`s are null-terminated as they are generated
@@ -168,7 +168,7 @@ macro_rules! print_macro (
// The `CONT` case.
($format_string:path, true, $($arg:tt)+) => (
$crate::print::call_printk_cont(
- format_args!($($arg)+),
+ $crate::prelude::fmt!($($arg)+),
);
);
);
diff --git a/rust/kernel/seq_file.rs b/rust/kernel/seq_file.rs
index 7a9403eb6e5b..627bc2f7b3d2 100644
--- a/rust/kernel/seq_file.rs
+++ b/rust/kernel/seq_file.rs
@@ -47,7 +47,7 @@ pub fn call_printf(&self, args: core::fmt::Arguments<'_>) {
#[macro_export]
macro_rules! seq_print {
($m:expr, $($arg:tt)+) => (
- $m.call_printf(format_args!($($arg)+))
+ $m.call_printf($crate::prelude::fmt!($($arg)+))
);
}
pub use seq_print;
diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
index 98d5c74ec4f7..302423ca5eb0 100644
--- a/rust/kernel/str.rs
+++ b/rust/kernel/str.rs
@@ -54,7 +54,7 @@ impl fmt::Display for BStr {
/// Formats printable ASCII characters, escaping the rest.
///
/// ```
- /// # use kernel::{fmt, b_str, str::{BStr, CString}};
+ /// # use kernel::{prelude::fmt, b_str, str::{BStr, CString}};
/// let ascii = b_str!("Hello, BStr!");
/// let s = CString::try_from_fmt(fmt!("{}", ascii))?;
/// assert_eq!(s.as_bytes(), "Hello, BStr!".as_bytes());
@@ -85,7 +85,7 @@ impl fmt::Debug for BStr {
/// escaping the rest.
///
/// ```
- /// # use kernel::{fmt, b_str, str::{BStr, CString}};
+ /// # use kernel::{prelude::fmt, b_str, str::{BStr, CString}};
/// // Embedded double quotes are escaped.
/// let ascii = b_str!("Hello, \"BStr\"!");
/// let s = CString::try_from_fmt(fmt!("{:?}", ascii))?;
@@ -424,12 +424,12 @@ pub fn to_ascii_uppercase(&self) -> Result<CString, AllocError> {
}
}
-impl fmt::Display for CStr {
+impl fmt::Display for crate::fmt::Adapter<&CStr> {
/// Formats printable ASCII characters, escaping the rest.
///
/// ```
/// # use kernel::c_str;
- /// # use kernel::fmt;
+ /// # use kernel::prelude::fmt;
/// # use kernel::str::CStr;
/// # use kernel::str::CString;
/// let penguin = c_str!("🐧");
@@ -442,7 +442,8 @@ impl fmt::Display for CStr {
/// # Ok::<(), kernel::error::Error>(())
/// ```
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
- for &c in self.as_bytes() {
+ let Self(cstr) = self;
+ for &c in cstr.as_bytes() {
if (0x20..0x7f).contains(&c) {
// Printable character.
f.write_char(c as char)?;
@@ -459,7 +460,7 @@ impl fmt::Debug for CStr {
///
/// ```
/// # use kernel::c_str;
- /// # use kernel::fmt;
+ /// # use kernel::prelude::fmt;
/// # use kernel::str::CStr;
/// # use kernel::str::CString;
/// let penguin = c_str!("🐧");
@@ -595,7 +596,7 @@ fn deref(&self) -> &str {
macro_rules! format {
($($f:tt)*) => ({
- &*String::from_fmt(kernel::fmt!($($f)*))
+ &*String::from_fmt(crate::prelude::fmt!($($f)*))
})
}
@@ -850,7 +851,7 @@ fn write_str(&mut self, s: &str) -> fmt::Result {
/// # Examples
///
/// ```
-/// use kernel::{str::CString, fmt};
+/// use kernel::{str::CString, prelude::fmt};
///
/// let s = CString::try_from_fmt(fmt!("{}{}{}", "abc", 10, 20))?;
/// assert_eq!(s.as_bytes_with_nul(), "abc1020\0".as_bytes());
@@ -940,9 +941,3 @@ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
fmt::Debug::fmt(&**self, f)
}
}
-
-/// A convenience alias for [`core::format_args`].
-#[macro_export]
-macro_rules! fmt {
- ($($f:tt)*) => ( core::format_args!($($f)*) )
-}
diff --git a/rust/macros/fmt.rs b/rust/macros/fmt.rs
new file mode 100644
index 000000000000..6b6bd9295d18
--- /dev/null
+++ b/rust/macros/fmt.rs
@@ -0,0 +1,118 @@
+// SPDX-License-Identifier: GPL-2.0
+
+use proc_macro::{Delimiter, Group, Ident, Punct, Spacing, Span, TokenStream, TokenTree};
+use std::collections::BTreeSet;
+
+/// Please see [`crate::fmt`] for documentation.
+pub(crate) fn fmt(input: TokenStream) -> TokenStream {
+ let mut input = input.into_iter();
+
+ let first_opt = input.next();
+ let first_owned_str;
+ let mut names = BTreeSet::new();
+ let first_lit = {
+ let Some((mut first_str, first_lit)) = (match first_opt.as_ref() {
+ Some(TokenTree::Literal(first_lit)) => {
+ first_owned_str = first_lit.to_string();
+ Some(first_owned_str.as_str()).and_then(|first| {
+ let first = first.strip_prefix('"')?;
+ let first = first.strip_suffix('"')?;
+ Some((first, first_lit))
+ })
+ }
+ _ => None,
+ }) else {
+ return first_opt.into_iter().chain(input).collect();
+ };
+ while let Some((_, rest)) = first_str.split_once('{') {
+ first_str = rest;
+ if let Some(rest) = first_str.strip_prefix('{') {
+ first_str = rest;
+ continue;
+ }
+ while let Some((name, rest)) = first_str.split_once('}') {
+ first_str = rest;
+ if let Some(rest) = first_str.strip_prefix('}') {
+ first_str = rest;
+ continue;
+ }
+ let name = name.split_once(':').map_or(name, |(name, _)| name);
+ if !name.is_empty() && !name.chars().all(|c| c.is_ascii_digit()) {
+ names.insert(name);
+ }
+ break;
+ }
+ }
+ first_lit
+ };
+
+ let first_span = first_lit.span();
+ let adapt = |expr| {
+ let mut borrow =
+ TokenStream::from_iter([TokenTree::Punct(Punct::new('&', Spacing::Alone))]);
+ borrow.extend(expr);
+ make_ident(first_span, ["kernel", "fmt", "Adapter"])
+ .chain([TokenTree::Group(Group::new(Delimiter::Parenthesis, borrow))])
+ };
+
+ let flush = |args: &mut TokenStream, current: &mut TokenStream| {
+ let current = std::mem::take(current);
+ if !current.is_empty() {
+ args.extend(adapt(current));
+ }
+ };
+
+ let mut args = TokenStream::from_iter(first_opt);
+ {
+ let mut current = TokenStream::new();
+ for tt in input {
+ match &tt {
+ TokenTree::Punct(p) => match p.as_char() {
+ ',' => {
+ flush(&mut args, &mut current);
+ &mut args
+ }
+ '=' => {
+ names.remove(current.to_string().as_str());
+ args.extend(std::mem::take(&mut current));
+ &mut args
+ }
+ _ => &mut current,
+ },
+ _ => &mut current,
+ }
+ .extend([tt]);
+ }
+ flush(&mut args, &mut current);
+ }
+
+ for name in names {
+ args.extend(
+ [
+ TokenTree::Punct(Punct::new(',', Spacing::Alone)),
+ TokenTree::Ident(Ident::new(name, first_span)),
+ TokenTree::Punct(Punct::new('=', Spacing::Alone)),
+ ]
+ .into_iter()
+ .chain(adapt(TokenTree::Ident(Ident::new(name, first_span)).into())),
+ );
+ }
+
+ TokenStream::from_iter(make_ident(first_span, ["core", "format_args"]).chain([
+ TokenTree::Punct(Punct::new('!', Spacing::Alone)),
+ TokenTree::Group(Group::new(Delimiter::Parenthesis, args)),
+ ]))
+}
+
+fn make_ident<'a, T: IntoIterator<Item = &'a str>>(
+ span: Span,
+ names: T,
+) -> impl Iterator<Item = TokenTree> + use<'a, T> {
+ names.into_iter().flat_map(move |name| {
+ [
+ TokenTree::Punct(Punct::new(':', Spacing::Joint)),
+ TokenTree::Punct(Punct::new(':', Spacing::Alone)),
+ TokenTree::Ident(Ident::new(name, span)),
+ ]
+ })
+}
diff --git a/rust/macros/lib.rs b/rust/macros/lib.rs
index d31e50c446b0..fa956eaa3ba7 100644
--- a/rust/macros/lib.rs
+++ b/rust/macros/lib.rs
@@ -10,6 +10,7 @@
mod quote;
mod concat_idents;
mod export;
+mod fmt;
mod helpers;
mod kunit;
mod module;
@@ -196,6 +197,24 @@ pub fn export(attr: TokenStream, ts: TokenStream) -> TokenStream {
export::export(attr, ts)
}
+/// Like [`core::format_args!`], but automatically wraps arguments in [`kernel::fmt::Adapter`].
+///
+/// This macro allows generating `core::fmt::Arguments` while ensuring that each argument is wrapped
+/// with `::kernel::fmt::Adapter`, which customizes formatting behavior for kernel logging.
+///
+/// Named arguments used in the format string (e.g. `{foo}`) are detected and resolved from local
+/// bindings. All positional and named arguments are automatically wrapped.
+///
+/// This macro is an implementation detail of other kernel logging macros like [`pr_info!`] and
+/// should not typically be used directly.
+///
+/// [`kernel::fmt::Adapter`]: ../kernel/fmt/struct.Adapter.html
+/// [`pr_info!`]: ../kernel/macro.pr_info.html
+#[proc_macro]
+pub fn fmt(input: TokenStream) -> TokenStream {
+ fmt::fmt(input)
+}
+
/// Concatenate two identifiers.
///
/// This is useful in macros that need to declare or reference items with names
diff --git a/scripts/rustdoc_test_gen.rs b/scripts/rustdoc_test_gen.rs
index ec8d70ac888b..22ed9ee14053 100644
--- a/scripts/rustdoc_test_gen.rs
+++ b/scripts/rustdoc_test_gen.rs
@@ -197,7 +197,7 @@ macro_rules! assert_eq {{
// This follows the syntax for declaring test metadata in the proposed KTAP v2 spec, which may
// be used for the proposed KUnit test attributes API. Thus hopefully this will make migration
// easier later on.
- kernel::kunit::info(format_args!(" # {kunit_name}.location: {real_path}:{line}\n"));
+ kernel::kunit::info(fmt!(" # {kunit_name}.location: {real_path}:{line}\n"));
/// The anchor where the test code body starts.
#[allow(unused)]
--
2.49.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [PATCH v10 2/5] rust: support formatting of foreign types
2025-05-24 20:33 ` [PATCH v10 2/5] rust: support formatting of foreign types Tamir Duberstein
@ 2025-05-26 14:48 ` Benno Lossin
2025-05-26 22:17 ` Tamir Duberstein
0 siblings, 1 reply; 34+ messages in thread
From: Benno Lossin @ 2025-05-26 14:48 UTC (permalink / raw)
To: Tamir Duberstein, Michal Rostecki, Miguel Ojeda, Alex Gaynor,
Boqun Feng, Gary Guo, Björn Roy Baron, Andreas Hindborg,
Alice Ryhl, Trevor Gross, Brendan Higgins, David Gow, Rae Moar,
Danilo Krummrich, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter,
Greg Kroah-Hartman, Rafael J. Wysocki, Luis Chamberlain,
Russ Weight, FUJITA Tomonori, Rob Herring, Saravana Kannan,
Peter Zijlstra, Ingo Molnar, Will Deacon, Waiman Long,
Nathan Chancellor, Nick Desaulniers, Bill Wendling, Justin Stitt,
Andrew Lunn, Heiner Kallweit, Russell King, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Bjorn Helgaas,
Arnd Bergmann, Jens Axboe, Krzysztof Wilczyński
Cc: rust-for-linux, linux-kernel, linux-kselftest, kunit-dev,
dri-devel, netdev, devicetree, llvm, linux-pci, nouveau,
linux-block
On Sat May 24, 2025 at 10:33 PM CEST, Tamir Duberstein wrote:
> Introduce a `fmt!` macro which wraps all arguments in
> `kernel::fmt::Adapter` This enables formatting of foreign types (like
> `core::ffi::CStr`) that do not implement `fmt::Display` due to concerns
> around lossy conversions which do not apply in the kernel.
>
> Replace all direct calls to `format_args!` with `fmt!`.
>
> In preparation for replacing our `CStr` with `core::ffi::CStr`, move its
> `fmt::Display` implementation to `kernel::fmt::Adapter<&CStr>`.
>
> Suggested-by: Alice Ryhl <aliceryhl@google.com>
> Link: https://rust-for-linux.zulipchat.com/#narrow/channel/288089-General/topic/Custom.20formatting/with/516476467
> Signed-off-by: Tamir Duberstein <tamird@gmail.com>
> ---
> drivers/block/rnull.rs | 2 +-
> rust/kernel/block/mq.rs | 2 +-
> rust/kernel/device.rs | 2 +-
> rust/kernel/fmt.rs | 77 +++++++++++++++++++++++++++++
> rust/kernel/kunit.rs | 6 +--
> rust/kernel/lib.rs | 1 +
> rust/kernel/prelude.rs | 3 +-
> rust/kernel/print.rs | 4 +-
> rust/kernel/seq_file.rs | 2 +-
> rust/kernel/str.rs | 23 ++++-----
> rust/macros/fmt.rs | 118 ++++++++++++++++++++++++++++++++++++++++++++
> rust/macros/lib.rs | 19 +++++++
> scripts/rustdoc_test_gen.rs | 2 +-
> 13 files changed, 235 insertions(+), 26 deletions(-)
Can you split this into creating the proc-macro, forwarding the display
impls and replacing all the uses with the proc macro?
> diff --git a/drivers/block/rnull.rs b/drivers/block/rnull.rs
> index d07e76ae2c13..6366da12c5a5 100644
> --- a/drivers/block/rnull.rs
> +++ b/drivers/block/rnull.rs
> @@ -51,7 +51,7 @@ fn init(_module: &'static ThisModule) -> impl PinInit<Self, Error> {
> .logical_block_size(4096)?
> .physical_block_size(4096)?
> .rotational(false)
> - .build(format_args!("rnullb{}", 0), tagset)
> + .build(fmt!("rnullb{}", 0), tagset)
> })();
>
> try_pin_init!(Self {
> diff --git a/rust/kernel/block/mq.rs b/rust/kernel/block/mq.rs
> index fb0f393c1cea..842be88aa1cf 100644
> --- a/rust/kernel/block/mq.rs
> +++ b/rust/kernel/block/mq.rs
> @@ -82,7 +82,7 @@
> //! Arc::pin_init(TagSet::new(1, 256, 1), flags::GFP_KERNEL)?;
> //! let mut disk = gen_disk::GenDiskBuilder::new()
> //! .capacity_sectors(4096)
> -//! .build(format_args!("myblk"), tagset)?;
> +//! .build(fmt!("myblk"), tagset)?;
> //!
> //! # Ok::<(), kernel::error::Error>(())
> //! ```
> diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs
> index 5c372cf27ed0..99d99a76934c 100644
> --- a/rust/kernel/device.rs
> +++ b/rust/kernel/device.rs
> @@ -240,7 +240,7 @@ impl DeviceContext for Normal {}
> macro_rules! dev_printk {
> ($method:ident, $dev:expr, $($f:tt)*) => {
> {
> - ($dev).$method(core::format_args!($($f)*));
> + ($dev).$method($crate::prelude::fmt!($($f)*));
> }
> }
> }
> diff --git a/rust/kernel/fmt.rs b/rust/kernel/fmt.rs
> new file mode 100644
> index 000000000000..12b08debc3b3
> --- /dev/null
> +++ b/rust/kernel/fmt.rs
> @@ -0,0 +1,77 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Formatting utilities.
> +
> +use core::fmt;
> +
> +/// Internal adapter used to route allow implementations of formatting traits for foreign types.
> +///
> +/// It is inserted automatically by the [`fmt!`] macro and is not meant to be used directly.
> +///
> +/// [`fmt!`]: crate::prelude::fmt!
> +#[doc(hidden)]
> +pub struct Adapter<T>(pub T);
> +
> +macro_rules! impl_fmt_adapter_forward {
> + ($($trait:ident),* $(,)?) => {
> + $(
> + impl<T: fmt::$trait> fmt::$trait for Adapter<T> {
> + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
> + let Self(t) = self;
> + fmt::$trait::fmt(t, f)
> + }
> + }
> + )*
> + };
> +}
> +
> +impl_fmt_adapter_forward!(Debug, LowerHex, UpperHex, Octal, Binary, Pointer, LowerExp, UpperExp);
> +
> +macro_rules! impl_display_forward {
> + ($(
> + $( { $($generics:tt)* } )? $ty:ty $( { where $($where:tt)* } )?
You don't need `{}` around the `where` clause, as a `where` keyword can
follow a `ty` fragment.
> + ),* $(,)?) => {
> + $(
> + impl$($($generics)*)? fmt::Display for Adapter<&$ty>
> + $(where $($where)*)? {
> + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
> + let Self(t) = self;
> + fmt::Display::fmt(t, f)
> + }
> + }
> + )*
> + };
> +}
> +
> +impl<T: ?Sized> fmt::Display for Adapter<&&T>
> +where
> + for<'a> Adapter<&'a T>: fmt::Display,
> +{
> + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
> + let Self(t) = self;
> + Adapter::<&T>(**t).fmt(f)
> + }
> +}
> +
> +impl_display_forward!(
> + bool,
> + char,
> + core::panic::PanicInfo<'_>,
> + crate::str::BStr,
> + fmt::Arguments<'_>,
> + i128,
> + i16,
> + i32,
> + i64,
> + i8,
> + isize,
> + str,
> + u128,
> + u16,
> + u32,
> + u64,
> + u8,
> + usize,
> + {<T: ?Sized>} crate::sync::Arc<T> {where crate::sync::Arc<T>: fmt::Display},
> + {<T: ?Sized>} crate::sync::UniqueArc<T> {where crate::sync::UniqueArc<T>: fmt::Display},
> +);
If we use `{}` instead of `()`, then we can format the contents
differently:
impl_display_forward! {
i8, i16, i32, i64, i128, isize,
u8, u16, u32, u64, u128, usize,
bool, char, str,
crate::str::BStr,
fmt::Arguments<'_>,
core::panic::PanicInfo<'_>,
{<T: ?Sized>} crate::sync::Arc<T> {where Self: fmt::Display},
{<T: ?Sized>} crate::sync::UniqueArc<T> {where Self: fmt::Display},
}
> diff --git a/rust/macros/fmt.rs b/rust/macros/fmt.rs
> new file mode 100644
> index 000000000000..6b6bd9295d18
> --- /dev/null
> +++ b/rust/macros/fmt.rs
> @@ -0,0 +1,118 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +use proc_macro::{Delimiter, Group, Ident, Punct, Spacing, Span, TokenStream, TokenTree};
> +use std::collections::BTreeSet;
> +
> +/// Please see [`crate::fmt`] for documentation.
> +pub(crate) fn fmt(input: TokenStream) -> TokenStream {
> + let mut input = input.into_iter();
> +
> + let first_opt = input.next();
> + let first_owned_str;
> + let mut names = BTreeSet::new();
> + let first_lit = {
> + let Some((mut first_str, first_lit)) = (match first_opt.as_ref() {
> + Some(TokenTree::Literal(first_lit)) => {
> + first_owned_str = first_lit.to_string();
> + Some(first_owned_str.as_str()).and_then(|first| {
> + let first = first.strip_prefix('"')?;
> + let first = first.strip_suffix('"')?;
> + Some((first, first_lit))
> + })
> + }
> + _ => None,
> + }) else {
> + return first_opt.into_iter().chain(input).collect();
> + };
This usage of let-else + match is pretty confusing and could just be a
single match statement.
> + while let Some((_, rest)) = first_str.split_once('{') {
> + first_str = rest;
> + if let Some(rest) = first_str.strip_prefix('{') {
> + first_str = rest;
> + continue;
> + }
> + while let Some((name, rest)) = first_str.split_once('}') {
> + first_str = rest;
> + if let Some(rest) = first_str.strip_prefix('}') {
This doesn't make sense, we've matched a `{`, some text and a `}`. You
can't escape a `}` that is associated to a `{`.
> + first_str = rest;
> + continue;
> + }
> + let name = name.split_once(':').map_or(name, |(name, _)| name);
> + if !name.is_empty() && !name.chars().all(|c| c.is_ascii_digit()) {
> + names.insert(name);
> + }
> + break;
> + }
> + }
> + first_lit
`first_lit` is not modified, so could we just the code above it into a
block instead of keeping it in the expr for `first_lit`?
> + };
> +
> + let first_span = first_lit.span();
> + let adapt = |expr| {
> + let mut borrow =
> + TokenStream::from_iter([TokenTree::Punct(Punct::new('&', Spacing::Alone))]);
> + borrow.extend(expr);
> + make_ident(first_span, ["kernel", "fmt", "Adapter"])
> + .chain([TokenTree::Group(Group::new(Delimiter::Parenthesis, borrow))])
This should be fine with using `quote!`:
quote!(::kernel::fmt::Adapter(&#expr))
> + };
> +
> + let flush = |args: &mut TokenStream, current: &mut TokenStream| {
> + let current = std::mem::take(current);
> + if !current.is_empty() {
> + args.extend(adapt(current));
> + }
> + };
> +
> + let mut args = TokenStream::from_iter(first_opt);
> + {
> + let mut current = TokenStream::new();
> + for tt in input {
> + match &tt {
> + TokenTree::Punct(p) => match p.as_char() {
> + ',' => {
> + flush(&mut args, &mut current);
> + &mut args
> + }
> + '=' => {
> + names.remove(current.to_string().as_str());
> + args.extend(std::mem::take(&mut current));
> + &mut args
> + }
> + _ => &mut current,
> + },
> + _ => &mut current,
> + }
> + .extend([tt]);
> + }
This doesn't handle the following code correctly ):
let mut a = 0;
pr_info!("{a:?}", a = a = a);
Looks like we'll have to remember what "kind" of an equals we parsed...
> + flush(&mut args, &mut current);
> + }
> +
> + for name in names {
> + args.extend(
> + [
> + TokenTree::Punct(Punct::new(',', Spacing::Alone)),
> + TokenTree::Ident(Ident::new(name, first_span)),
> + TokenTree::Punct(Punct::new('=', Spacing::Alone)),
> + ]
> + .into_iter()
> + .chain(adapt(TokenTree::Ident(Ident::new(name, first_span)).into())),
> + );
This can probably be:
let name = Ident::new(name, first_span);
let value = adapt(name.clone());
args.extend(quote!(, #name = #value));
> + }
> +
> + TokenStream::from_iter(make_ident(first_span, ["core", "format_args"]).chain([
> + TokenTree::Punct(Punct::new('!', Spacing::Alone)),
> + TokenTree::Group(Group::new(Delimiter::Parenthesis, args)),
> + ]))
This can be:
quote!(::core::format_args!(#args))
(not sure if you need `#(#args)*`)
> +}
> +
> +fn make_ident<'a, T: IntoIterator<Item = &'a str>>(
> + span: Span,
> + names: T,
> +) -> impl Iterator<Item = TokenTree> + use<'a, T> {
> + names.into_iter().flat_map(move |name| {
> + [
> + TokenTree::Punct(Punct::new(':', Spacing::Joint)),
> + TokenTree::Punct(Punct::new(':', Spacing::Alone)),
> + TokenTree::Ident(Ident::new(name, span)),
> + ]
> + })
> +}
> diff --git a/rust/macros/lib.rs b/rust/macros/lib.rs
> index d31e50c446b0..fa956eaa3ba7 100644
> --- a/rust/macros/lib.rs
> +++ b/rust/macros/lib.rs
> @@ -10,6 +10,7 @@
> mod quote;
> mod concat_idents;
> mod export;
> +mod fmt;
> mod helpers;
> mod kunit;
> mod module;
> @@ -196,6 +197,24 @@ pub fn export(attr: TokenStream, ts: TokenStream) -> TokenStream {
> export::export(attr, ts)
> }
>
> +/// Like [`core::format_args!`], but automatically wraps arguments in [`kernel::fmt::Adapter`].
> +///
> +/// This macro allows generating `core::fmt::Arguments` while ensuring that each argument is wrapped
> +/// with `::kernel::fmt::Adapter`, which customizes formatting behavior for kernel logging.
> +///
> +/// Named arguments used in the format string (e.g. `{foo}`) are detected and resolved from local
> +/// bindings. All positional and named arguments are automatically wrapped.
> +///
> +/// This macro is an implementation detail of other kernel logging macros like [`pr_info!`] and
> +/// should not typically be used directly.
> +///
> +/// [`kernel::fmt::Adapter`]: ../kernel/fmt/struct.Adapter.html
> +/// [`pr_info!`]: ../kernel/macro.pr_info.html
> +#[proc_macro]
> +pub fn fmt(input: TokenStream) -> TokenStream {
I'm wondering if we should name this `format_args` instead in order to
better communicate that it's a replacement for `core::format_args!`.
---
Cheers,
Benno
> + fmt::fmt(input)
> +}
> +
> /// Concatenate two identifiers.
> ///
> /// This is useful in macros that need to declare or reference items with names
> diff --git a/scripts/rustdoc_test_gen.rs b/scripts/rustdoc_test_gen.rs
> index ec8d70ac888b..22ed9ee14053 100644
> --- a/scripts/rustdoc_test_gen.rs
> +++ b/scripts/rustdoc_test_gen.rs
> @@ -197,7 +197,7 @@ macro_rules! assert_eq {{
> // This follows the syntax for declaring test metadata in the proposed KTAP v2 spec, which may
> // be used for the proposed KUnit test attributes API. Thus hopefully this will make migration
> // easier later on.
> - kernel::kunit::info(format_args!(" # {kunit_name}.location: {real_path}:{line}\n"));
> + kernel::kunit::info(fmt!(" # {kunit_name}.location: {real_path}:{line}\n"));
>
> /// The anchor where the test code body starts.
> #[allow(unused)]
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH v10 2/5] rust: support formatting of foreign types
2025-05-26 14:48 ` Benno Lossin
@ 2025-05-26 22:17 ` Tamir Duberstein
2025-05-26 23:01 ` Benno Lossin
2025-05-27 12:44 ` Alice Ryhl
0 siblings, 2 replies; 34+ messages in thread
From: Tamir Duberstein @ 2025-05-26 22:17 UTC (permalink / raw)
To: Benno Lossin
Cc: Michal Rostecki, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Brendan Higgins, David Gow, Rae Moar, Danilo Krummrich,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Greg Kroah-Hartman, Rafael J. Wysocki,
Luis Chamberlain, Russ Weight, FUJITA Tomonori, Rob Herring,
Saravana Kannan, Peter Zijlstra, Ingo Molnar, Will Deacon,
Waiman Long, Nathan Chancellor, Nick Desaulniers, Bill Wendling,
Justin Stitt, Andrew Lunn, Heiner Kallweit, Russell King,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Bjorn Helgaas, Arnd Bergmann, Jens Axboe,
Krzysztof Wilczyński, rust-for-linux, linux-kernel,
linux-kselftest, kunit-dev, dri-devel, netdev, devicetree, llvm,
linux-pci, nouveau, linux-block
On Mon, May 26, 2025 at 10:48 AM Benno Lossin <lossin@kernel.org> wrote:
>
> On Sat May 24, 2025 at 10:33 PM CEST, Tamir Duberstein wrote:
> > Introduce a `fmt!` macro which wraps all arguments in
> > `kernel::fmt::Adapter` This enables formatting of foreign types (like
> > `core::ffi::CStr`) that do not implement `fmt::Display` due to concerns
> > around lossy conversions which do not apply in the kernel.
> >
> > Replace all direct calls to `format_args!` with `fmt!`.
> >
> > In preparation for replacing our `CStr` with `core::ffi::CStr`, move its
> > `fmt::Display` implementation to `kernel::fmt::Adapter<&CStr>`.
> >
> > Suggested-by: Alice Ryhl <aliceryhl@google.com>
> > Link: https://rust-for-linux.zulipchat.com/#narrow/channel/288089-General/topic/Custom.20formatting/with/516476467
> > Signed-off-by: Tamir Duberstein <tamird@gmail.com>
> > ---
> > drivers/block/rnull.rs | 2 +-
> > rust/kernel/block/mq.rs | 2 +-
> > rust/kernel/device.rs | 2 +-
> > rust/kernel/fmt.rs | 77 +++++++++++++++++++++++++++++
> > rust/kernel/kunit.rs | 6 +--
> > rust/kernel/lib.rs | 1 +
> > rust/kernel/prelude.rs | 3 +-
> > rust/kernel/print.rs | 4 +-
> > rust/kernel/seq_file.rs | 2 +-
> > rust/kernel/str.rs | 23 ++++-----
> > rust/macros/fmt.rs | 118 ++++++++++++++++++++++++++++++++++++++++++++
> > rust/macros/lib.rs | 19 +++++++
> > scripts/rustdoc_test_gen.rs | 2 +-
> > 13 files changed, 235 insertions(+), 26 deletions(-)
>
> Can you split this into creating the proc-macro, forwarding the display
> impls and replacing all the uses with the proc macro?
Can you help me understand why that's better?
>
> > diff --git a/drivers/block/rnull.rs b/drivers/block/rnull.rs
> > index d07e76ae2c13..6366da12c5a5 100644
> > --- a/drivers/block/rnull.rs
> > +++ b/drivers/block/rnull.rs
> > @@ -51,7 +51,7 @@ fn init(_module: &'static ThisModule) -> impl PinInit<Self, Error> {
> > .logical_block_size(4096)?
> > .physical_block_size(4096)?
> > .rotational(false)
> > - .build(format_args!("rnullb{}", 0), tagset)
> > + .build(fmt!("rnullb{}", 0), tagset)
> > })();
> >
> > try_pin_init!(Self {
> > diff --git a/rust/kernel/block/mq.rs b/rust/kernel/block/mq.rs
> > index fb0f393c1cea..842be88aa1cf 100644
> > --- a/rust/kernel/block/mq.rs
> > +++ b/rust/kernel/block/mq.rs
> > @@ -82,7 +82,7 @@
> > //! Arc::pin_init(TagSet::new(1, 256, 1), flags::GFP_KERNEL)?;
> > //! let mut disk = gen_disk::GenDiskBuilder::new()
> > //! .capacity_sectors(4096)
> > -//! .build(format_args!("myblk"), tagset)?;
> > +//! .build(fmt!("myblk"), tagset)?;
> > //!
> > //! # Ok::<(), kernel::error::Error>(())
> > //! ```
> > diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs
> > index 5c372cf27ed0..99d99a76934c 100644
> > --- a/rust/kernel/device.rs
> > +++ b/rust/kernel/device.rs
> > @@ -240,7 +240,7 @@ impl DeviceContext for Normal {}
> > macro_rules! dev_printk {
> > ($method:ident, $dev:expr, $($f:tt)*) => {
> > {
> > - ($dev).$method(core::format_args!($($f)*));
> > + ($dev).$method($crate::prelude::fmt!($($f)*));
> > }
> > }
> > }
> > diff --git a/rust/kernel/fmt.rs b/rust/kernel/fmt.rs
> > new file mode 100644
> > index 000000000000..12b08debc3b3
> > --- /dev/null
> > +++ b/rust/kernel/fmt.rs
> > @@ -0,0 +1,77 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +//! Formatting utilities.
> > +
> > +use core::fmt;
> > +
> > +/// Internal adapter used to route allow implementations of formatting traits for foreign types.
> > +///
> > +/// It is inserted automatically by the [`fmt!`] macro and is not meant to be used directly.
> > +///
> > +/// [`fmt!`]: crate::prelude::fmt!
> > +#[doc(hidden)]
> > +pub struct Adapter<T>(pub T);
> > +
> > +macro_rules! impl_fmt_adapter_forward {
> > + ($($trait:ident),* $(,)?) => {
> > + $(
> > + impl<T: fmt::$trait> fmt::$trait for Adapter<T> {
> > + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
> > + let Self(t) = self;
> > + fmt::$trait::fmt(t, f)
> > + }
> > + }
> > + )*
> > + };
> > +}
> > +
> > +impl_fmt_adapter_forward!(Debug, LowerHex, UpperHex, Octal, Binary, Pointer, LowerExp, UpperExp);
> > +
> > +macro_rules! impl_display_forward {
> > + ($(
> > + $( { $($generics:tt)* } )? $ty:ty $( { where $($where:tt)* } )?
>
> You don't need `{}` around the `where` clause, as a `where` keyword can
> follow a `ty` fragment.
This doesn't work:
```
error: local ambiguity when calling macro `impl_display_forward`:
multiple parsing options: built-in NTs tt ('r#where') or 2 other
options.
--> rust/kernel/fmt.rs:75:78
|
75 | {<T: ?Sized>} crate::sync::Arc<T> where crate::sync::Arc<T>:
fmt::Display,
|
^
```
>
> > + ),* $(,)?) => {
> > + $(
> > + impl$($($generics)*)? fmt::Display for Adapter<&$ty>
> > + $(where $($where)*)? {
> > + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
> > + let Self(t) = self;
> > + fmt::Display::fmt(t, f)
> > + }
> > + }
> > + )*
> > + };
> > +}
> > +
> > +impl<T: ?Sized> fmt::Display for Adapter<&&T>
> > +where
> > + for<'a> Adapter<&'a T>: fmt::Display,
> > +{
> > + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
> > + let Self(t) = self;
> > + Adapter::<&T>(**t).fmt(f)
> > + }
> > +}
> > +
> > +impl_display_forward!(
> > + bool,
> > + char,
> > + core::panic::PanicInfo<'_>,
> > + crate::str::BStr,
> > + fmt::Arguments<'_>,
> > + i128,
> > + i16,
> > + i32,
> > + i64,
> > + i8,
> > + isize,
> > + str,
> > + u128,
> > + u16,
> > + u32,
> > + u64,
> > + u8,
> > + usize,
> > + {<T: ?Sized>} crate::sync::Arc<T> {where crate::sync::Arc<T>: fmt::Display},
> > + {<T: ?Sized>} crate::sync::UniqueArc<T> {where crate::sync::UniqueArc<T>: fmt::Display},
> > +);
>
> If we use `{}` instead of `()`, then we can format the contents
> differently:
>
> impl_display_forward! {
> i8, i16, i32, i64, i128, isize,
> u8, u16, u32, u64, u128, usize,
> bool, char, str,
> crate::str::BStr,
> fmt::Arguments<'_>,
> core::panic::PanicInfo<'_>,
> {<T: ?Sized>} crate::sync::Arc<T> {where Self: fmt::Display},
> {<T: ?Sized>} crate::sync::UniqueArc<T> {where Self: fmt::Display},
> }
Is that formatting better? rustfmt refuses to touch it either way.
>
> > diff --git a/rust/macros/fmt.rs b/rust/macros/fmt.rs
> > new file mode 100644
> > index 000000000000..6b6bd9295d18
> > --- /dev/null
> > +++ b/rust/macros/fmt.rs
> > @@ -0,0 +1,118 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +use proc_macro::{Delimiter, Group, Ident, Punct, Spacing, Span, TokenStream, TokenTree};
> > +use std::collections::BTreeSet;
> > +
> > +/// Please see [`crate::fmt`] for documentation.
> > +pub(crate) fn fmt(input: TokenStream) -> TokenStream {
> > + let mut input = input.into_iter();
> > +
> > + let first_opt = input.next();
> > + let first_owned_str;
> > + let mut names = BTreeSet::new();
> > + let first_lit = {
> > + let Some((mut first_str, first_lit)) = (match first_opt.as_ref() {
> > + Some(TokenTree::Literal(first_lit)) => {
> > + first_owned_str = first_lit.to_string();
> > + Some(first_owned_str.as_str()).and_then(|first| {
> > + let first = first.strip_prefix('"')?;
> > + let first = first.strip_suffix('"')?;
> > + Some((first, first_lit))
> > + })
> > + }
> > + _ => None,
> > + }) else {
> > + return first_opt.into_iter().chain(input).collect();
> > + };
>
> This usage of let-else + match is pretty confusing and could just be a
> single match statement.
I don't think so. Can you try rewriting it into the form you like?
>
> > + while let Some((_, rest)) = first_str.split_once('{') {
> > + first_str = rest;
> > + if let Some(rest) = first_str.strip_prefix('{') {
> > + first_str = rest;
> > + continue;
> > + }
> > + while let Some((name, rest)) = first_str.split_once('}') {
> > + first_str = rest;
> > + if let Some(rest) = first_str.strip_prefix('}') {
>
> This doesn't make sense, we've matched a `{`, some text and a `}`. You
> can't escape a `}` that is associated to a `{`.
Sure, but such input would be malformed, so I don't think it's
necessary to handle it "perfectly". We'll get a nice error from
format_args anyhow.
https://play.rust-lang.org/?version=stable&mode=debug&edition=2024&gist=5f529d93da7cf46b3c99ba7772623e33
>
> > + first_str = rest;
> > + continue;
> > + }
> > + let name = name.split_once(':').map_or(name, |(name, _)| name);
> > + if !name.is_empty() && !name.chars().all(|c| c.is_ascii_digit()) {
> > + names.insert(name);
> > + }
> > + break;
> > + }
> > + }
> > + first_lit
>
> `first_lit` is not modified, so could we just the code above it into a
> block instead of keeping it in the expr for `first_lit`?
As above, can you suggest the alternate form you like better? The
gymnastics here are all in service of being able to let malformed
input fall through to core::format_args which will do the hard work of
producing good diagnostics.
>
> > + };
> > +
> > + let first_span = first_lit.span();
> > + let adapt = |expr| {
> > + let mut borrow =
> > + TokenStream::from_iter([TokenTree::Punct(Punct::new('&', Spacing::Alone))]);
> > + borrow.extend(expr);
> > + make_ident(first_span, ["kernel", "fmt", "Adapter"])
> > + .chain([TokenTree::Group(Group::new(Delimiter::Parenthesis, borrow))])
>
> This should be fine with using `quote!`:
>
> quote!(::kernel::fmt::Adapter(&#expr))
Yeah, I have a local commit that uses quote_spanned to remove all the
manual constructions.
>
> > + };
> > +
> > + let flush = |args: &mut TokenStream, current: &mut TokenStream| {
> > + let current = std::mem::take(current);
> > + if !current.is_empty() {
> > + args.extend(adapt(current));
> > + }
> > + };
> > +
> > + let mut args = TokenStream::from_iter(first_opt);
> > + {
> > + let mut current = TokenStream::new();
> > + for tt in input {
> > + match &tt {
> > + TokenTree::Punct(p) => match p.as_char() {
> > + ',' => {
> > + flush(&mut args, &mut current);
> > + &mut args
> > + }
> > + '=' => {
> > + names.remove(current.to_string().as_str());
> > + args.extend(std::mem::take(&mut current));
> > + &mut args
> > + }
> > + _ => &mut current,
> > + },
> > + _ => &mut current,
> > + }
> > + .extend([tt]);
> > + }
>
> This doesn't handle the following code correctly ):
>
> let mut a = 0;
> pr_info!("{a:?}", a = a = a);
>
> Looks like we'll have to remember what "kind" of an equals we parsed...
Hmm, good point. Maybe we can just avoid dealing with `=` at all until
we hit the `,` and just split on the leftmost `=`. WDYT? I'll have
that in v11.
>
> > + flush(&mut args, &mut current);
> > + }
> > +
> > + for name in names {
> > + args.extend(
> > + [
> > + TokenTree::Punct(Punct::new(',', Spacing::Alone)),
> > + TokenTree::Ident(Ident::new(name, first_span)),
> > + TokenTree::Punct(Punct::new('=', Spacing::Alone)),
> > + ]
> > + .into_iter()
> > + .chain(adapt(TokenTree::Ident(Ident::new(name, first_span)).into())),
> > + );
>
> This can probably be:
>
> let name = Ident::new(name, first_span);
> let value = adapt(name.clone());
> args.extend(quote!(, #name = #value));
Indeed, see above - manual construction will be gone in v11.
>
> > + }
> > +
> > + TokenStream::from_iter(make_ident(first_span, ["core", "format_args"]).chain([
> > + TokenTree::Punct(Punct::new('!', Spacing::Alone)),
> > + TokenTree::Group(Group::new(Delimiter::Parenthesis, args)),
> > + ]))
>
> This can be:
>
> quote!(::core::format_args!(#args))
>
> (not sure if you need `#(#args)*`)
>
> > +}
> > +
> > +fn make_ident<'a, T: IntoIterator<Item = &'a str>>(
> > + span: Span,
> > + names: T,
> > +) -> impl Iterator<Item = TokenTree> + use<'a, T> {
> > + names.into_iter().flat_map(move |name| {
> > + [
> > + TokenTree::Punct(Punct::new(':', Spacing::Joint)),
> > + TokenTree::Punct(Punct::new(':', Spacing::Alone)),
> > + TokenTree::Ident(Ident::new(name, span)),
> > + ]
> > + })
> > +}
> > diff --git a/rust/macros/lib.rs b/rust/macros/lib.rs
> > index d31e50c446b0..fa956eaa3ba7 100644
> > --- a/rust/macros/lib.rs
> > +++ b/rust/macros/lib.rs
> > @@ -10,6 +10,7 @@
> > mod quote;
> > mod concat_idents;
> > mod export;
> > +mod fmt;
> > mod helpers;
> > mod kunit;
> > mod module;
> > @@ -196,6 +197,24 @@ pub fn export(attr: TokenStream, ts: TokenStream) -> TokenStream {
> > export::export(attr, ts)
> > }
> >
> > +/// Like [`core::format_args!`], but automatically wraps arguments in [`kernel::fmt::Adapter`].
> > +///
> > +/// This macro allows generating `core::fmt::Arguments` while ensuring that each argument is wrapped
> > +/// with `::kernel::fmt::Adapter`, which customizes formatting behavior for kernel logging.
> > +///
> > +/// Named arguments used in the format string (e.g. `{foo}`) are detected and resolved from local
> > +/// bindings. All positional and named arguments are automatically wrapped.
> > +///
> > +/// This macro is an implementation detail of other kernel logging macros like [`pr_info!`] and
> > +/// should not typically be used directly.
> > +///
> > +/// [`kernel::fmt::Adapter`]: ../kernel/fmt/struct.Adapter.html
> > +/// [`pr_info!`]: ../kernel/macro.pr_info.html
> > +#[proc_macro]
> > +pub fn fmt(input: TokenStream) -> TokenStream {
>
> I'm wondering if we should name this `format_args` instead in order to
> better communicate that it's a replacement for `core::format_args!`.
Unfortunately that introduces ambiguity in cases where
kernel::prelude::* is imported because core::format_args is in core's
prelude.
>
> ---
> Cheers,
> Benno
>
> > + fmt::fmt(input)
> > +}
> > +
> > /// Concatenate two identifiers.
> > ///
> > /// This is useful in macros that need to declare or reference items with names
> > diff --git a/scripts/rustdoc_test_gen.rs b/scripts/rustdoc_test_gen.rs
> > index ec8d70ac888b..22ed9ee14053 100644
> > --- a/scripts/rustdoc_test_gen.rs
> > +++ b/scripts/rustdoc_test_gen.rs
> > @@ -197,7 +197,7 @@ macro_rules! assert_eq {{
> > // This follows the syntax for declaring test metadata in the proposed KTAP v2 spec, which may
> > // be used for the proposed KUnit test attributes API. Thus hopefully this will make migration
> > // easier later on.
> > - kernel::kunit::info(format_args!(" # {kunit_name}.location: {real_path}:{line}\n"));
> > + kernel::kunit::info(fmt!(" # {kunit_name}.location: {real_path}:{line}\n"));
> >
> > /// The anchor where the test code body starts.
> > #[allow(unused)]
>
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH v10 2/5] rust: support formatting of foreign types
2025-05-26 22:17 ` Tamir Duberstein
@ 2025-05-26 23:01 ` Benno Lossin
2025-05-27 15:02 ` Tamir Duberstein
2025-05-27 12:44 ` Alice Ryhl
1 sibling, 1 reply; 34+ messages in thread
From: Benno Lossin @ 2025-05-26 23:01 UTC (permalink / raw)
To: Tamir Duberstein
Cc: Michal Rostecki, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Brendan Higgins, David Gow, Rae Moar, Danilo Krummrich,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Greg Kroah-Hartman, Rafael J. Wysocki,
Luis Chamberlain, Russ Weight, FUJITA Tomonori, Rob Herring,
Saravana Kannan, Peter Zijlstra, Ingo Molnar, Will Deacon,
Waiman Long, Nathan Chancellor, Nick Desaulniers, Bill Wendling,
Justin Stitt, Andrew Lunn, Heiner Kallweit, Russell King,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Bjorn Helgaas, Arnd Bergmann, Jens Axboe,
Krzysztof Wilczyński, rust-for-linux, linux-kernel,
linux-kselftest, kunit-dev, dri-devel, netdev, devicetree, llvm,
linux-pci, nouveau, linux-block
On Tue May 27, 2025 at 12:17 AM CEST, Tamir Duberstein wrote:
> On Mon, May 26, 2025 at 10:48 AM Benno Lossin <lossin@kernel.org> wrote:
>> On Sat May 24, 2025 at 10:33 PM CEST, Tamir Duberstein wrote:
>> > Introduce a `fmt!` macro which wraps all arguments in
>> > `kernel::fmt::Adapter` This enables formatting of foreign types (like
>> > `core::ffi::CStr`) that do not implement `fmt::Display` due to concerns
>> > around lossy conversions which do not apply in the kernel.
>> >
>> > Replace all direct calls to `format_args!` with `fmt!`.
>> >
>> > In preparation for replacing our `CStr` with `core::ffi::CStr`, move its
>> > `fmt::Display` implementation to `kernel::fmt::Adapter<&CStr>`.
>> >
>> > Suggested-by: Alice Ryhl <aliceryhl@google.com>
>> > Link: https://rust-for-linux.zulipchat.com/#narrow/channel/288089-General/topic/Custom.20formatting/with/516476467
>> > Signed-off-by: Tamir Duberstein <tamird@gmail.com>
>> > ---
>> > drivers/block/rnull.rs | 2 +-
>> > rust/kernel/block/mq.rs | 2 +-
>> > rust/kernel/device.rs | 2 +-
>> > rust/kernel/fmt.rs | 77 +++++++++++++++++++++++++++++
>> > rust/kernel/kunit.rs | 6 +--
>> > rust/kernel/lib.rs | 1 +
>> > rust/kernel/prelude.rs | 3 +-
>> > rust/kernel/print.rs | 4 +-
>> > rust/kernel/seq_file.rs | 2 +-
>> > rust/kernel/str.rs | 23 ++++-----
>> > rust/macros/fmt.rs | 118 ++++++++++++++++++++++++++++++++++++++++++++
>> > rust/macros/lib.rs | 19 +++++++
>> > scripts/rustdoc_test_gen.rs | 2 +-
>> > 13 files changed, 235 insertions(+), 26 deletions(-)
>>
>> Can you split this into creating the proc-macro, forwarding the display
>> impls and replacing all the uses with the proc macro?
>
> Can you help me understand why that's better?
It makes reviewing significantly easier.
>> > +macro_rules! impl_display_forward {
>> > + ($(
>> > + $( { $($generics:tt)* } )? $ty:ty $( { where $($where:tt)* } )?
>>
>> You don't need `{}` around the `where` clause, as a `where` keyword can
>> follow a `ty` fragment.
>
> This doesn't work:
> ```
> error: local ambiguity when calling macro `impl_display_forward`:
> multiple parsing options: built-in NTs tt ('r#where') or 2 other
> options.
> --> rust/kernel/fmt.rs:75:78
> |
> 75 | {<T: ?Sized>} crate::sync::Arc<T> where crate::sync::Arc<T>:
> fmt::Display,
> |
> ^
> ```
Ah right that's a shame, forgot about the `tt`s at the end...
>> > +impl_display_forward!(
>> > + bool,
>> > + char,
>> > + core::panic::PanicInfo<'_>,
>> > + crate::str::BStr,
>> > + fmt::Arguments<'_>,
>> > + i128,
>> > + i16,
>> > + i32,
>> > + i64,
>> > + i8,
>> > + isize,
>> > + str,
>> > + u128,
>> > + u16,
>> > + u32,
>> > + u64,
>> > + u8,
>> > + usize,
>> > + {<T: ?Sized>} crate::sync::Arc<T> {where crate::sync::Arc<T>: fmt::Display},
>> > + {<T: ?Sized>} crate::sync::UniqueArc<T> {where crate::sync::UniqueArc<T>: fmt::Display},
>> > +);
>>
>> If we use `{}` instead of `()`, then we can format the contents
>> differently:
>>
>> impl_display_forward! {
>> i8, i16, i32, i64, i128, isize,
>> u8, u16, u32, u64, u128, usize,
>> bool, char, str,
>> crate::str::BStr,
>> fmt::Arguments<'_>,
>> core::panic::PanicInfo<'_>,
>> {<T: ?Sized>} crate::sync::Arc<T> {where Self: fmt::Display},
>> {<T: ?Sized>} crate::sync::UniqueArc<T> {where Self: fmt::Display},
>> }
>
> Is that formatting better? rustfmt refuses to touch it either way.
Yeah rustfmt doesn't touch macro parameters enclosed in `{}`. I think
it's better.
>> > +/// Please see [`crate::fmt`] for documentation.
>> > +pub(crate) fn fmt(input: TokenStream) -> TokenStream {
>> > + let mut input = input.into_iter();
>> > +
>> > + let first_opt = input.next();
>> > + let first_owned_str;
>> > + let mut names = BTreeSet::new();
>> > + let first_lit = {
>> > + let Some((mut first_str, first_lit)) = (match first_opt.as_ref() {
>> > + Some(TokenTree::Literal(first_lit)) => {
>> > + first_owned_str = first_lit.to_string();
>> > + Some(first_owned_str.as_str()).and_then(|first| {
>> > + let first = first.strip_prefix('"')?;
>> > + let first = first.strip_suffix('"')?;
>> > + Some((first, first_lit))
>> > + })
>> > + }
>> > + _ => None,
>> > + }) else {
>> > + return first_opt.into_iter().chain(input).collect();
>> > + };
>>
>> This usage of let-else + match is pretty confusing and could just be a
>> single match statement.
>
> I don't think so. Can you try rewriting it into the form you like?
let (mut first_str, first_lit) match first_opt.as_ref() {
Some(TokenTree::Literal(lit)) if lit.to_string().starts_with('"') => {
let contents = lit.to_string();
let contents = contents.strip_prefix('"').unwrap().strip_suffix('"').unwrap();
((contents, lit))
}
_ => return first_opt.into_iter().chain(input).collect(),
};
>> > + while let Some((_, rest)) = first_str.split_once('{') {
>> > + first_str = rest;
>> > + if let Some(rest) = first_str.strip_prefix('{') {
>> > + first_str = rest;
>> > + continue;
>> > + }
>> > + while let Some((name, rest)) = first_str.split_once('}') {
>> > + first_str = rest;
>> > + if let Some(rest) = first_str.strip_prefix('}') {
>>
>> This doesn't make sense, we've matched a `{`, some text and a `}`. You
>> can't escape a `}` that is associated to a `{`.
>
> Sure, but such input would be malformed, so I don't think it's
> necessary to handle it "perfectly". We'll get a nice error from
> format_args anyhow.
My suggestion in this case would be to just remove this if-let. The
search for `{` above would skip the `}` if it's correct.
> https://play.rust-lang.org/?version=stable&mode=debug&edition=2024&gist=5f529d93da7cf46b3c99ba7772623e33
Yes it will error like that, but if we do the replacement only when the
syntax is correct, there also will be compile errors because of a
missing `Display` impl, or is that not the case?
I'm a bit concerned about the ergonomics that this change will
introduce, but I guess there really isn't anything that we can do about
except not do it.
>> > + first_str = rest;
>> > + continue;
>> > + }
>> > + let name = name.split_once(':').map_or(name, |(name, _)| name);
>> > + if !name.is_empty() && !name.chars().all(|c| c.is_ascii_digit()) {
>> > + names.insert(name);
>> > + }
>> > + break;
>> > + }
>> > + }
>> > + first_lit
>>
>> `first_lit` is not modified, so could we just the code above it into a
>> block instead of keeping it in the expr for `first_lit`?
>
> As above, can you suggest the alternate form you like better? The
> gymnastics here are all in service of being able to let malformed
> input fall through to core::format_args which will do the hard work of
> producing good diagnostics.
I don't see how this is hard, just do:
let (first_str, first_lit) = ...;
while ...
>> > + };
>> > +
>> > + let first_span = first_lit.span();
>> > + let adapt = |expr| {
>> > + let mut borrow =
>> > + TokenStream::from_iter([TokenTree::Punct(Punct::new('&', Spacing::Alone))]);
>> > + borrow.extend(expr);
>> > + make_ident(first_span, ["kernel", "fmt", "Adapter"])
>> > + .chain([TokenTree::Group(Group::new(Delimiter::Parenthesis, borrow))])
>>
>> This should be fine with using `quote!`:
>>
>> quote!(::kernel::fmt::Adapter(&#expr))
>
> Yeah, I have a local commit that uses quote_spanned to remove all the
> manual constructions.
I don't think that you need `quote_spanned` here at all. If you do, then
let me know, something weird with spans is going on then.
>> > + };
>> > +
>> > + let flush = |args: &mut TokenStream, current: &mut TokenStream| {
>> > + let current = std::mem::take(current);
>> > + if !current.is_empty() {
>> > + args.extend(adapt(current));
>> > + }
>> > + };
>> > +
>> > + let mut args = TokenStream::from_iter(first_opt);
>> > + {
>> > + let mut current = TokenStream::new();
>> > + for tt in input {
>> > + match &tt {
>> > + TokenTree::Punct(p) => match p.as_char() {
>> > + ',' => {
>> > + flush(&mut args, &mut current);
>> > + &mut args
>> > + }
>> > + '=' => {
>> > + names.remove(current.to_string().as_str());
>> > + args.extend(std::mem::take(&mut current));
>> > + &mut args
>> > + }
>> > + _ => &mut current,
>> > + },
>> > + _ => &mut current,
>> > + }
>> > + .extend([tt]);
>> > + }
>>
>> This doesn't handle the following code correctly ):
>>
>> let mut a = 0;
>> pr_info!("{a:?}", a = a = a);
>>
>> Looks like we'll have to remember what "kind" of an equals we parsed...
>
> Hmm, good point. Maybe we can just avoid dealing with `=` at all until
> we hit the `,` and just split on the leftmost `=`. WDYT? I'll have
> that in v11.
Sounds good, if there is no `=`, then ignore it.
>> > +/// Like [`core::format_args!`], but automatically wraps arguments in [`kernel::fmt::Adapter`].
>> > +///
>> > +/// This macro allows generating `core::fmt::Arguments` while ensuring that each argument is wrapped
>> > +/// with `::kernel::fmt::Adapter`, which customizes formatting behavior for kernel logging.
>> > +///
>> > +/// Named arguments used in the format string (e.g. `{foo}`) are detected and resolved from local
>> > +/// bindings. All positional and named arguments are automatically wrapped.
>> > +///
>> > +/// This macro is an implementation detail of other kernel logging macros like [`pr_info!`] and
>> > +/// should not typically be used directly.
>> > +///
>> > +/// [`kernel::fmt::Adapter`]: ../kernel/fmt/struct.Adapter.html
>> > +/// [`pr_info!`]: ../kernel/macro.pr_info.html
>> > +#[proc_macro]
>> > +pub fn fmt(input: TokenStream) -> TokenStream {
>>
>> I'm wondering if we should name this `format_args` instead in order to
>> better communicate that it's a replacement for `core::format_args!`.
>
> Unfortunately that introduces ambiguity in cases where
> kernel::prelude::* is imported because core::format_args is in core's
> prelude.
Ahh that's unfortunate.
---
Cheers,
Benno
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH v10 2/5] rust: support formatting of foreign types
2025-05-26 23:01 ` Benno Lossin
@ 2025-05-27 15:02 ` Tamir Duberstein
2025-05-27 20:49 ` Benno Lossin
0 siblings, 1 reply; 34+ messages in thread
From: Tamir Duberstein @ 2025-05-27 15:02 UTC (permalink / raw)
To: Benno Lossin
Cc: Michal Rostecki, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Brendan Higgins, David Gow, Rae Moar, Danilo Krummrich,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Greg Kroah-Hartman, Rafael J. Wysocki,
Luis Chamberlain, Russ Weight, FUJITA Tomonori, Rob Herring,
Saravana Kannan, Peter Zijlstra, Ingo Molnar, Will Deacon,
Waiman Long, Nathan Chancellor, Nick Desaulniers, Bill Wendling,
Justin Stitt, Andrew Lunn, Heiner Kallweit, Russell King,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Bjorn Helgaas, Arnd Bergmann, Jens Axboe,
Krzysztof Wilczyński, rust-for-linux, linux-kernel,
linux-kselftest, kunit-dev, dri-devel, netdev, devicetree, llvm,
linux-pci, nouveau, linux-block
On Mon, May 26, 2025 at 7:01 PM Benno Lossin <lossin@kernel.org> wrote:
>
> On Tue May 27, 2025 at 12:17 AM CEST, Tamir Duberstein wrote:
> > On Mon, May 26, 2025 at 10:48 AM Benno Lossin <lossin@kernel.org> wrote:
> >> On Sat May 24, 2025 at 10:33 PM CEST, Tamir Duberstein wrote:
> >> > Introduce a `fmt!` macro which wraps all arguments in
> >> > `kernel::fmt::Adapter` This enables formatting of foreign types (like
> >> > `core::ffi::CStr`) that do not implement `fmt::Display` due to concerns
> >> > around lossy conversions which do not apply in the kernel.
> >> >
> >> > Replace all direct calls to `format_args!` with `fmt!`.
> >> >
> >> > In preparation for replacing our `CStr` with `core::ffi::CStr`, move its
> >> > `fmt::Display` implementation to `kernel::fmt::Adapter<&CStr>`.
> >> >
> >> > Suggested-by: Alice Ryhl <aliceryhl@google.com>
> >> > Link: https://rust-for-linux.zulipchat.com/#narrow/channel/288089-General/topic/Custom.20formatting/with/516476467
> >> > Signed-off-by: Tamir Duberstein <tamird@gmail.com>
> >> > ---
> >> > drivers/block/rnull.rs | 2 +-
> >> > rust/kernel/block/mq.rs | 2 +-
> >> > rust/kernel/device.rs | 2 +-
> >> > rust/kernel/fmt.rs | 77 +++++++++++++++++++++++++++++
> >> > rust/kernel/kunit.rs | 6 +--
> >> > rust/kernel/lib.rs | 1 +
> >> > rust/kernel/prelude.rs | 3 +-
> >> > rust/kernel/print.rs | 4 +-
> >> > rust/kernel/seq_file.rs | 2 +-
> >> > rust/kernel/str.rs | 23 ++++-----
> >> > rust/macros/fmt.rs | 118 ++++++++++++++++++++++++++++++++++++++++++++
> >> > rust/macros/lib.rs | 19 +++++++
> >> > scripts/rustdoc_test_gen.rs | 2 +-
> >> > 13 files changed, 235 insertions(+), 26 deletions(-)
> >>
> >> Can you split this into creating the proc-macro, forwarding the display
> >> impls and replacing all the uses with the proc macro?
> >
> > Can you help me understand why that's better?
>
> It makes reviewing significantly easier.
>
> >> > +macro_rules! impl_display_forward {
> >> > + ($(
> >> > + $( { $($generics:tt)* } )? $ty:ty $( { where $($where:tt)* } )?
> >>
> >> You don't need `{}` around the `where` clause, as a `where` keyword can
> >> follow a `ty` fragment.
> >
> > This doesn't work:
> > ```
> > error: local ambiguity when calling macro `impl_display_forward`:
> > multiple parsing options: built-in NTs tt ('r#where') or 2 other
> > options.
> > --> rust/kernel/fmt.rs:75:78
> > |
> > 75 | {<T: ?Sized>} crate::sync::Arc<T> where crate::sync::Arc<T>:
> > fmt::Display,
> > |
> > ^
> > ```
>
> Ah right that's a shame, forgot about the `tt`s at the end...
>
> >> > +impl_display_forward!(
> >> > + bool,
> >> > + char,
> >> > + core::panic::PanicInfo<'_>,
> >> > + crate::str::BStr,
> >> > + fmt::Arguments<'_>,
> >> > + i128,
> >> > + i16,
> >> > + i32,
> >> > + i64,
> >> > + i8,
> >> > + isize,
> >> > + str,
> >> > + u128,
> >> > + u16,
> >> > + u32,
> >> > + u64,
> >> > + u8,
> >> > + usize,
> >> > + {<T: ?Sized>} crate::sync::Arc<T> {where crate::sync::Arc<T>: fmt::Display},
> >> > + {<T: ?Sized>} crate::sync::UniqueArc<T> {where crate::sync::UniqueArc<T>: fmt::Display},
> >> > +);
> >>
> >> If we use `{}` instead of `()`, then we can format the contents
> >> differently:
> >>
> >> impl_display_forward! {
> >> i8, i16, i32, i64, i128, isize,
> >> u8, u16, u32, u64, u128, usize,
> >> bool, char, str,
> >> crate::str::BStr,
> >> fmt::Arguments<'_>,
> >> core::panic::PanicInfo<'_>,
> >> {<T: ?Sized>} crate::sync::Arc<T> {where Self: fmt::Display},
> >> {<T: ?Sized>} crate::sync::UniqueArc<T> {where Self: fmt::Display},
> >> }
> >
> > Is that formatting better? rustfmt refuses to touch it either way.
>
> Yeah rustfmt doesn't touch macro parameters enclosed in `{}`. I think
> it's better.
OK, but why? This seems entirely subjective.
> >> > +/// Please see [`crate::fmt`] for documentation.
> >> > +pub(crate) fn fmt(input: TokenStream) -> TokenStream {
> >> > + let mut input = input.into_iter();
> >> > +
> >> > + let first_opt = input.next();
> >> > + let first_owned_str;
> >> > + let mut names = BTreeSet::new();
> >> > + let first_lit = {
> >> > + let Some((mut first_str, first_lit)) = (match first_opt.as_ref() {
> >> > + Some(TokenTree::Literal(first_lit)) => {
> >> > + first_owned_str = first_lit.to_string();
> >> > + Some(first_owned_str.as_str()).and_then(|first| {
> >> > + let first = first.strip_prefix('"')?;
> >> > + let first = first.strip_suffix('"')?;
> >> > + Some((first, first_lit))
> >> > + })
> >> > + }
> >> > + _ => None,
> >> > + }) else {
> >> > + return first_opt.into_iter().chain(input).collect();
> >> > + };
> >>
> >> This usage of let-else + match is pretty confusing and could just be a
> >> single match statement.
> >
> > I don't think so. Can you try rewriting it into the form you like?
>
> let (mut first_str, first_lit) match first_opt.as_ref() {
> Some(TokenTree::Literal(lit)) if lit.to_string().starts_with('"') => {
> let contents = lit.to_string();
> let contents = contents.strip_prefix('"').unwrap().strip_suffix('"').unwrap();
> ((contents, lit))
> }
> _ => return first_opt.into_iter().chain(input).collect(),
> };
What happens if the invocation is utterly malformed, e.g.
`fmt!("hello)`? You're unwrapping here, which I intentionally avoid.
>
> >> > + while let Some((_, rest)) = first_str.split_once('{') {
> >> > + first_str = rest;
> >> > + if let Some(rest) = first_str.strip_prefix('{') {
> >> > + first_str = rest;
> >> > + continue;
> >> > + }
> >> > + while let Some((name, rest)) = first_str.split_once('}') {
> >> > + first_str = rest;
> >> > + if let Some(rest) = first_str.strip_prefix('}') {
> >>
> >> This doesn't make sense, we've matched a `{`, some text and a `}`. You
> >> can't escape a `}` that is associated to a `{`.
> >
> > Sure, but such input would be malformed, so I don't think it's
> > necessary to handle it "perfectly". We'll get a nice error from
> > format_args anyhow.
>
> My suggestion in this case would be to just remove this if-let. The
> search for `{` above would skip the `}` if it's correct.
>
> > https://play.rust-lang.org/?version=stable&mode=debug&edition=2024&gist=5f529d93da7cf46b3c99ba7772623e33
Makes sense to me.
>
> Yes it will error like that, but if we do the replacement only when the
> syntax is correct, there also will be compile errors because of a
> missing `Display` impl, or is that not the case?
I'm not sure - I would guess syntax errors "mask" typeck errors.
>
> I'm a bit concerned about the ergonomics that this change will
> introduce, but I guess there really isn't anything that we can do about
> except not do it.
>
> >> > + first_str = rest;
> >> > + continue;
> >> > + }
> >> > + let name = name.split_once(':').map_or(name, |(name, _)| name);
> >> > + if !name.is_empty() && !name.chars().all(|c| c.is_ascii_digit()) {
> >> > + names.insert(name);
> >> > + }
> >> > + break;
> >> > + }
> >> > + }
> >> > + first_lit
> >>
> >> `first_lit` is not modified, so could we just the code above it into a
> >> block instead of keeping it in the expr for `first_lit`?
> >
> > As above, can you suggest the alternate form you like better? The
> > gymnastics here are all in service of being able to let malformed
> > input fall through to core::format_args which will do the hard work of
> > producing good diagnostics.
>
> I don't see how this is hard, just do:
>
> let (first_str, first_lit) = ...;
It requires you to unwrap, like you did above, which is what I'm
trying to avoid.
>
> while ...
>
> >> > + };
> >> > +
> >> > + let first_span = first_lit.span();
> >> > + let adapt = |expr| {
> >> > + let mut borrow =
> >> > + TokenStream::from_iter([TokenTree::Punct(Punct::new('&', Spacing::Alone))]);
> >> > + borrow.extend(expr);
> >> > + make_ident(first_span, ["kernel", "fmt", "Adapter"])
> >> > + .chain([TokenTree::Group(Group::new(Delimiter::Parenthesis, borrow))])
> >>
> >> This should be fine with using `quote!`:
> >>
> >> quote!(::kernel::fmt::Adapter(&#expr))
> >
> > Yeah, I have a local commit that uses quote_spanned to remove all the
> > manual constructions.
>
> I don't think that you need `quote_spanned` here at all. If you do, then
> let me know, something weird with spans is going on then.
You need to give idents a span, so each of `kernel`, `fmt`, and
`adapter` need a span. I *could* use `quote!` and get whatever span it
uses (mixed_site) but I'd rather retain control.
>
> >> > + };
> >> > +
> >> > + let flush = |args: &mut TokenStream, current: &mut TokenStream| {
> >> > + let current = std::mem::take(current);
> >> > + if !current.is_empty() {
> >> > + args.extend(adapt(current));
> >> > + }
> >> > + };
> >> > +
> >> > + let mut args = TokenStream::from_iter(first_opt);
> >> > + {
> >> > + let mut current = TokenStream::new();
> >> > + for tt in input {
> >> > + match &tt {
> >> > + TokenTree::Punct(p) => match p.as_char() {
> >> > + ',' => {
> >> > + flush(&mut args, &mut current);
> >> > + &mut args
> >> > + }
> >> > + '=' => {
> >> > + names.remove(current.to_string().as_str());
> >> > + args.extend(std::mem::take(&mut current));
> >> > + &mut args
> >> > + }
> >> > + _ => &mut current,
> >> > + },
> >> > + _ => &mut current,
> >> > + }
> >> > + .extend([tt]);
> >> > + }
> >>
> >> This doesn't handle the following code correctly ):
> >>
> >> let mut a = 0;
> >> pr_info!("{a:?}", a = a = a);
> >>
> >> Looks like we'll have to remember what "kind" of an equals we parsed...
> >
> > Hmm, good point. Maybe we can just avoid dealing with `=` at all until
> > we hit the `,` and just split on the leftmost `=`. WDYT? I'll have
> > that in v11.
>
> Sounds good, if there is no `=`, then ignore it.
>
> >> > +/// Like [`core::format_args!`], but automatically wraps arguments in [`kernel::fmt::Adapter`].
> >> > +///
> >> > +/// This macro allows generating `core::fmt::Arguments` while ensuring that each argument is wrapped
> >> > +/// with `::kernel::fmt::Adapter`, which customizes formatting behavior for kernel logging.
> >> > +///
> >> > +/// Named arguments used in the format string (e.g. `{foo}`) are detected and resolved from local
> >> > +/// bindings. All positional and named arguments are automatically wrapped.
> >> > +///
> >> > +/// This macro is an implementation detail of other kernel logging macros like [`pr_info!`] and
> >> > +/// should not typically be used directly.
> >> > +///
> >> > +/// [`kernel::fmt::Adapter`]: ../kernel/fmt/struct.Adapter.html
> >> > +/// [`pr_info!`]: ../kernel/macro.pr_info.html
> >> > +#[proc_macro]
> >> > +pub fn fmt(input: TokenStream) -> TokenStream {
> >>
> >> I'm wondering if we should name this `format_args` instead in order to
> >> better communicate that it's a replacement for `core::format_args!`.
> >
> > Unfortunately that introduces ambiguity in cases where
> > kernel::prelude::* is imported because core::format_args is in core's
> > prelude.
>
> Ahh that's unfortunate.
>
> ---
> Cheers,
> Benno
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH v10 2/5] rust: support formatting of foreign types
2025-05-27 15:02 ` Tamir Duberstein
@ 2025-05-27 20:49 ` Benno Lossin
2025-05-29 22:07 ` Tamir Duberstein
0 siblings, 1 reply; 34+ messages in thread
From: Benno Lossin @ 2025-05-27 20:49 UTC (permalink / raw)
To: Tamir Duberstein
Cc: Michal Rostecki, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Brendan Higgins, David Gow, Rae Moar, Danilo Krummrich,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Greg Kroah-Hartman, Rafael J. Wysocki,
Luis Chamberlain, Russ Weight, FUJITA Tomonori, Rob Herring,
Saravana Kannan, Peter Zijlstra, Ingo Molnar, Will Deacon,
Waiman Long, Nathan Chancellor, Nick Desaulniers, Bill Wendling,
Justin Stitt, Andrew Lunn, Heiner Kallweit, Russell King,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Bjorn Helgaas, Arnd Bergmann, Jens Axboe,
Krzysztof Wilczyński, rust-for-linux, linux-kernel,
linux-kselftest, kunit-dev, dri-devel, netdev, devicetree, llvm,
linux-pci, nouveau, linux-block
On Tue May 27, 2025 at 5:02 PM CEST, Tamir Duberstein wrote:
> On Mon, May 26, 2025 at 7:01 PM Benno Lossin <lossin@kernel.org> wrote:
>> On Tue May 27, 2025 at 12:17 AM CEST, Tamir Duberstein wrote:
>> > On Mon, May 26, 2025 at 10:48 AM Benno Lossin <lossin@kernel.org> wrote:
>> >> On Sat May 24, 2025 at 10:33 PM CEST, Tamir Duberstein wrote:
>> >> > +impl_display_forward!(
>> >> > + bool,
>> >> > + char,
>> >> > + core::panic::PanicInfo<'_>,
>> >> > + crate::str::BStr,
>> >> > + fmt::Arguments<'_>,
>> >> > + i128,
>> >> > + i16,
>> >> > + i32,
>> >> > + i64,
>> >> > + i8,
>> >> > + isize,
>> >> > + str,
>> >> > + u128,
>> >> > + u16,
>> >> > + u32,
>> >> > + u64,
>> >> > + u8,
>> >> > + usize,
>> >> > + {<T: ?Sized>} crate::sync::Arc<T> {where crate::sync::Arc<T>: fmt::Display},
>> >> > + {<T: ?Sized>} crate::sync::UniqueArc<T> {where crate::sync::UniqueArc<T>: fmt::Display},
>> >> > +);
>> >>
>> >> If we use `{}` instead of `()`, then we can format the contents
>> >> differently:
>> >>
>> >> impl_display_forward! {
>> >> i8, i16, i32, i64, i128, isize,
>> >> u8, u16, u32, u64, u128, usize,
>> >> bool, char, str,
>> >> crate::str::BStr,
>> >> fmt::Arguments<'_>,
>> >> core::panic::PanicInfo<'_>,
>> >> {<T: ?Sized>} crate::sync::Arc<T> {where Self: fmt::Display},
>> >> {<T: ?Sized>} crate::sync::UniqueArc<T> {where Self: fmt::Display},
>> >> }
>> >
>> > Is that formatting better? rustfmt refuses to touch it either way.
>>
>> Yeah rustfmt doesn't touch macro parameters enclosed in `{}`. I think
>> it's better.
>
> OK, but why? This seems entirely subjective.
If more types are added to the list, it will grow over one screen size.
With my formatting, leaving related types on a single line, that will
only happen much later.
>> >> > +/// Please see [`crate::fmt`] for documentation.
>> >> > +pub(crate) fn fmt(input: TokenStream) -> TokenStream {
>> >> > + let mut input = input.into_iter();
>> >> > +
>> >> > + let first_opt = input.next();
>> >> > + let first_owned_str;
>> >> > + let mut names = BTreeSet::new();
>> >> > + let first_lit = {
>> >> > + let Some((mut first_str, first_lit)) = (match first_opt.as_ref() {
>> >> > + Some(TokenTree::Literal(first_lit)) => {
>> >> > + first_owned_str = first_lit.to_string();
>> >> > + Some(first_owned_str.as_str()).and_then(|first| {
>> >> > + let first = first.strip_prefix('"')?;
>> >> > + let first = first.strip_suffix('"')?;
>> >> > + Some((first, first_lit))
>> >> > + })
>> >> > + }
>> >> > + _ => None,
>> >> > + }) else {
>> >> > + return first_opt.into_iter().chain(input).collect();
>> >> > + };
>> >>
>> >> This usage of let-else + match is pretty confusing and could just be a
>> >> single match statement.
>> >
>> > I don't think so. Can you try rewriting it into the form you like?
>>
>> let (mut first_str, first_lit) match first_opt.as_ref() {
>> Some(TokenTree::Literal(lit)) if lit.to_string().starts_with('"') => {
>> let contents = lit.to_string();
>> let contents = contents.strip_prefix('"').unwrap().strip_suffix('"').unwrap();
>> ((contents, lit))
>> }
>> _ => return first_opt.into_iter().chain(input).collect(),
>> };
>
> What happens if the invocation is utterly malformed, e.g.
> `fmt!("hello)`? You're unwrapping here, which I intentionally avoid.
That example won't even survive lexing (macros always will get valid
rust tokens as input). If a literal begins with a `"`, it also will end
with one AFAIK.
>> Yes it will error like that, but if we do the replacement only when the
>> syntax is correct, there also will be compile errors because of a
>> missing `Display` impl, or is that not the case?
>
> I'm not sure - I would guess syntax errors "mask" typeck errors.
I checked and it seems to be so, that's good.
>> >> > + first_str = rest;
>> >> > + continue;
>> >> > + }
>> >> > + let name = name.split_once(':').map_or(name, |(name, _)| name);
>> >> > + if !name.is_empty() && !name.chars().all(|c| c.is_ascii_digit()) {
>> >> > + names.insert(name);
>> >> > + }
>> >> > + break;
>> >> > + }
>> >> > + }
>> >> > + first_lit
>> >>
>> >> `first_lit` is not modified, so could we just the code above it into a
>> >> block instead of keeping it in the expr for `first_lit`?
>> >
>> > As above, can you suggest the alternate form you like better? The
>> > gymnastics here are all in service of being able to let malformed
>> > input fall through to core::format_args which will do the hard work of
>> > producing good diagnostics.
>>
>> I don't see how this is hard, just do:
>>
>> let (first_str, first_lit) = ...;
>
> It requires you to unwrap, like you did above, which is what I'm
> trying to avoid.
How so? What do you need to unwrap?
>> >> > + };
>> >> > +
>> >> > + let first_span = first_lit.span();
>> >> > + let adapt = |expr| {
>> >> > + let mut borrow =
>> >> > + TokenStream::from_iter([TokenTree::Punct(Punct::new('&', Spacing::Alone))]);
>> >> > + borrow.extend(expr);
>> >> > + make_ident(first_span, ["kernel", "fmt", "Adapter"])
>> >> > + .chain([TokenTree::Group(Group::new(Delimiter::Parenthesis, borrow))])
>> >>
>> >> This should be fine with using `quote!`:
>> >>
>> >> quote!(::kernel::fmt::Adapter(&#expr))
>> >
>> > Yeah, I have a local commit that uses quote_spanned to remove all the
>> > manual constructions.
>>
>> I don't think that you need `quote_spanned` here at all. If you do, then
>> let me know, something weird with spans is going on then.
>
> You need to give idents a span, so each of `kernel`, `fmt`, and
> `adapter` need a span. I *could* use `quote!` and get whatever span it
> uses (mixed_site) but I'd rather retain control.
Please use `quote!` if it works. No need to make this more complex than
it already is. If it doesn't work then that's another story.
---
Cheers,
Benno
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH v10 2/5] rust: support formatting of foreign types
2025-05-27 20:49 ` Benno Lossin
@ 2025-05-29 22:07 ` Tamir Duberstein
0 siblings, 0 replies; 34+ messages in thread
From: Tamir Duberstein @ 2025-05-29 22:07 UTC (permalink / raw)
To: Benno Lossin
Cc: Michal Rostecki, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Brendan Higgins, David Gow, Rae Moar, Danilo Krummrich,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Greg Kroah-Hartman, Rafael J. Wysocki,
Luis Chamberlain, Russ Weight, FUJITA Tomonori, Rob Herring,
Saravana Kannan, Peter Zijlstra, Ingo Molnar, Will Deacon,
Waiman Long, Nathan Chancellor, Nick Desaulniers, Bill Wendling,
Justin Stitt, Andrew Lunn, Heiner Kallweit, Russell King,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Bjorn Helgaas, Arnd Bergmann, Jens Axboe,
Krzysztof Wilczyński, rust-for-linux, linux-kernel,
linux-kselftest, kunit-dev, dri-devel, netdev, devicetree, llvm,
linux-pci, nouveau, linux-block
On Tue, May 27, 2025 at 4:49 PM Benno Lossin <lossin@kernel.org> wrote:
>
> On Tue May 27, 2025 at 5:02 PM CEST, Tamir Duberstein wrote:
> > On Mon, May 26, 2025 at 7:01 PM Benno Lossin <lossin@kernel.org> wrote:
> >> On Tue May 27, 2025 at 12:17 AM CEST, Tamir Duberstein wrote:
> >> > On Mon, May 26, 2025 at 10:48 AM Benno Lossin <lossin@kernel.org> wrote:
> >> >> On Sat May 24, 2025 at 10:33 PM CEST, Tamir Duberstein wrote:
> >> >> > +impl_display_forward!(
> >> >> > + bool,
> >> >> > + char,
> >> >> > + core::panic::PanicInfo<'_>,
> >> >> > + crate::str::BStr,
> >> >> > + fmt::Arguments<'_>,
> >> >> > + i128,
> >> >> > + i16,
> >> >> > + i32,
> >> >> > + i64,
> >> >> > + i8,
> >> >> > + isize,
> >> >> > + str,
> >> >> > + u128,
> >> >> > + u16,
> >> >> > + u32,
> >> >> > + u64,
> >> >> > + u8,
> >> >> > + usize,
> >> >> > + {<T: ?Sized>} crate::sync::Arc<T> {where crate::sync::Arc<T>: fmt::Display},
> >> >> > + {<T: ?Sized>} crate::sync::UniqueArc<T> {where crate::sync::UniqueArc<T>: fmt::Display},
> >> >> > +);
> >> >>
> >> >> If we use `{}` instead of `()`, then we can format the contents
> >> >> differently:
> >> >>
> >> >> impl_display_forward! {
> >> >> i8, i16, i32, i64, i128, isize,
> >> >> u8, u16, u32, u64, u128, usize,
> >> >> bool, char, str,
> >> >> crate::str::BStr,
> >> >> fmt::Arguments<'_>,
> >> >> core::panic::PanicInfo<'_>,
> >> >> {<T: ?Sized>} crate::sync::Arc<T> {where Self: fmt::Display},
> >> >> {<T: ?Sized>} crate::sync::UniqueArc<T> {where Self: fmt::Display},
> >> >> }
> >> >
> >> > Is that formatting better? rustfmt refuses to touch it either way.
> >>
> >> Yeah rustfmt doesn't touch macro parameters enclosed in `{}`. I think
> >> it's better.
> >
> > OK, but why? This seems entirely subjective.
>
> If more types are added to the list, it will grow over one screen size.
> With my formatting, leaving related types on a single line, that will
> only happen much later.
>
> >> >> > +/// Please see [`crate::fmt`] for documentation.
> >> >> > +pub(crate) fn fmt(input: TokenStream) -> TokenStream {
> >> >> > + let mut input = input.into_iter();
> >> >> > +
> >> >> > + let first_opt = input.next();
> >> >> > + let first_owned_str;
> >> >> > + let mut names = BTreeSet::new();
> >> >> > + let first_lit = {
> >> >> > + let Some((mut first_str, first_lit)) = (match first_opt.as_ref() {
> >> >> > + Some(TokenTree::Literal(first_lit)) => {
> >> >> > + first_owned_str = first_lit.to_string();
> >> >> > + Some(first_owned_str.as_str()).and_then(|first| {
> >> >> > + let first = first.strip_prefix('"')?;
> >> >> > + let first = first.strip_suffix('"')?;
> >> >> > + Some((first, first_lit))
> >> >> > + })
> >> >> > + }
> >> >> > + _ => None,
> >> >> > + }) else {
> >> >> > + return first_opt.into_iter().chain(input).collect();
> >> >> > + };
> >> >>
> >> >> This usage of let-else + match is pretty confusing and could just be a
> >> >> single match statement.
> >> >
> >> > I don't think so. Can you try rewriting it into the form you like?
> >>
> >> let (mut first_str, first_lit) match first_opt.as_ref() {
> >> Some(TokenTree::Literal(lit)) if lit.to_string().starts_with('"') => {
> >> let contents = lit.to_string();
> >> let contents = contents.strip_prefix('"').unwrap().strip_suffix('"').unwrap();
> >> ((contents, lit))
> >> }
> >> _ => return first_opt.into_iter().chain(input).collect(),
> >> };
> >
> > What happens if the invocation is utterly malformed, e.g.
> > `fmt!("hello)`? You're unwrapping here, which I intentionally avoid.
>
> That example won't even survive lexing (macros always will get valid
> rust tokens as input). If a literal begins with a `"`, it also will end
> with one AFAIK.
>
> >> Yes it will error like that, but if we do the replacement only when the
> >> syntax is correct, there also will be compile errors because of a
> >> missing `Display` impl, or is that not the case?
> >
> > I'm not sure - I would guess syntax errors "mask" typeck errors.
>
> I checked and it seems to be so, that's good.
👍
>
> >> >> > + first_str = rest;
> >> >> > + continue;
> >> >> > + }
> >> >> > + let name = name.split_once(':').map_or(name, |(name, _)| name);
> >> >> > + if !name.is_empty() && !name.chars().all(|c| c.is_ascii_digit()) {
> >> >> > + names.insert(name);
> >> >> > + }
> >> >> > + break;
> >> >> > + }
> >> >> > + }
> >> >> > + first_lit
> >> >>
> >> >> `first_lit` is not modified, so could we just the code above it into a
> >> >> block instead of keeping it in the expr for `first_lit`?
> >> >
> >> > As above, can you suggest the alternate form you like better? The
> >> > gymnastics here are all in service of being able to let malformed
> >> > input fall through to core::format_args which will do the hard work of
> >> > producing good diagnostics.
> >>
> >> I don't see how this is hard, just do:
> >>
> >> let (first_str, first_lit) = ...;
> >
> > It requires you to unwrap, like you did above, which is what I'm
> > trying to avoid.
>
> How so? What do you need to unwrap?
I was referring to your unwraps above.
> >> >> > + };
> >> >> > +
> >> >> > + let first_span = first_lit.span();
> >> >> > + let adapt = |expr| {
> >> >> > + let mut borrow =
> >> >> > + TokenStream::from_iter([TokenTree::Punct(Punct::new('&', Spacing::Alone))]);
> >> >> > + borrow.extend(expr);
> >> >> > + make_ident(first_span, ["kernel", "fmt", "Adapter"])
> >> >> > + .chain([TokenTree::Group(Group::new(Delimiter::Parenthesis, borrow))])
> >> >>
> >> >> This should be fine with using `quote!`:
> >> >>
> >> >> quote!(::kernel::fmt::Adapter(&#expr))
> >> >
> >> > Yeah, I have a local commit that uses quote_spanned to remove all the
> >> > manual constructions.
> >>
> >> I don't think that you need `quote_spanned` here at all. If you do, then
> >> let me know, something weird with spans is going on then.
> >
> > You need to give idents a span, so each of `kernel`, `fmt`, and
> > `adapter` need a span. I *could* use `quote!` and get whatever span it
> > uses (mixed_site) but I'd rather retain control.
>
> Please use `quote!` if it works. No need to make this more complex than
> it already is. If it doesn't work then that's another story.
Let's adjudicate that on v11, where you can see the code.
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v10 2/5] rust: support formatting of foreign types
2025-05-26 22:17 ` Tamir Duberstein
2025-05-26 23:01 ` Benno Lossin
@ 2025-05-27 12:44 ` Alice Ryhl
2025-05-27 14:57 ` Tamir Duberstein
1 sibling, 1 reply; 34+ messages in thread
From: Alice Ryhl @ 2025-05-27 12:44 UTC (permalink / raw)
To: Tamir Duberstein
Cc: Benno Lossin, Michal Rostecki, Miguel Ojeda, Alex Gaynor,
Boqun Feng, Gary Guo, Björn Roy Baron, Andreas Hindborg,
Trevor Gross, Brendan Higgins, David Gow, Rae Moar,
Danilo Krummrich, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter,
Greg Kroah-Hartman, Rafael J. Wysocki, Luis Chamberlain,
Russ Weight, FUJITA Tomonori, Rob Herring, Saravana Kannan,
Peter Zijlstra, Ingo Molnar, Will Deacon, Waiman Long,
Nathan Chancellor, Nick Desaulniers, Bill Wendling, Justin Stitt,
Andrew Lunn, Heiner Kallweit, Russell King, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Bjorn Helgaas,
Arnd Bergmann, Jens Axboe, Krzysztof Wilczyński,
rust-for-linux, linux-kernel, linux-kselftest, kunit-dev,
dri-devel, netdev, devicetree, llvm, linux-pci, nouveau,
linux-block
On Tue, May 27, 2025 at 12:18 AM Tamir Duberstein <tamird@gmail.com> wrote:
> > > +}
> > > +
> > > +fn make_ident<'a, T: IntoIterator<Item = &'a str>>(
> > > + span: Span,
> > > + names: T,
> > > +) -> impl Iterator<Item = TokenTree> + use<'a, T> {
> > > + names.into_iter().flat_map(move |name| {
> > > + [
> > > + TokenTree::Punct(Punct::new(':', Spacing::Joint)),
> > > + TokenTree::Punct(Punct::new(':', Spacing::Alone)),
> > > + TokenTree::Ident(Ident::new(name, span)),
> > > + ]
> > > + })
> > > +}
> > > diff --git a/rust/macros/lib.rs b/rust/macros/lib.rs
> > > index d31e50c446b0..fa956eaa3ba7 100644
> > > --- a/rust/macros/lib.rs
> > > +++ b/rust/macros/lib.rs
> > > @@ -10,6 +10,7 @@
> > > mod quote;
> > > mod concat_idents;
> > > mod export;
> > > +mod fmt;
> > > mod helpers;
> > > mod kunit;
> > > mod module;
> > > @@ -196,6 +197,24 @@ pub fn export(attr: TokenStream, ts: TokenStream) -> TokenStream {
> > > export::export(attr, ts)
> > > }
> > >
> > > +/// Like [`core::format_args!`], but automatically wraps arguments in [`kernel::fmt::Adapter`].
> > > +///
> > > +/// This macro allows generating `core::fmt::Arguments` while ensuring that each argument is wrapped
> > > +/// with `::kernel::fmt::Adapter`, which customizes formatting behavior for kernel logging.
> > > +///
> > > +/// Named arguments used in the format string (e.g. `{foo}`) are detected and resolved from local
> > > +/// bindings. All positional and named arguments are automatically wrapped.
> > > +///
> > > +/// This macro is an implementation detail of other kernel logging macros like [`pr_info!`] and
> > > +/// should not typically be used directly.
> > > +///
> > > +/// [`kernel::fmt::Adapter`]: ../kernel/fmt/struct.Adapter.html
> > > +/// [`pr_info!`]: ../kernel/macro.pr_info.html
> > > +#[proc_macro]
> > > +pub fn fmt(input: TokenStream) -> TokenStream {
> >
> > I'm wondering if we should name this `format_args` instead in order to
> > better communicate that it's a replacement for `core::format_args!`.
>
> Unfortunately that introduces ambiguity in cases where
> kernel::prelude::* is imported because core::format_args is in core's
> prelude.
I'm pretty sure that glob imports are higher priority than the core
prelude? Or is this because there are macros that now incorrectly use
kernel::prelude::format_args when they should use the one from core?
Alice
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH v10 2/5] rust: support formatting of foreign types
2025-05-27 12:44 ` Alice Ryhl
@ 2025-05-27 14:57 ` Tamir Duberstein
0 siblings, 0 replies; 34+ messages in thread
From: Tamir Duberstein @ 2025-05-27 14:57 UTC (permalink / raw)
To: Alice Ryhl
Cc: Benno Lossin, Michal Rostecki, Miguel Ojeda, Alex Gaynor,
Boqun Feng, Gary Guo, Björn Roy Baron, Andreas Hindborg,
Trevor Gross, Brendan Higgins, David Gow, Rae Moar,
Danilo Krummrich, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter,
Greg Kroah-Hartman, Rafael J. Wysocki, Luis Chamberlain,
Russ Weight, FUJITA Tomonori, Rob Herring, Saravana Kannan,
Peter Zijlstra, Ingo Molnar, Will Deacon, Waiman Long,
Nathan Chancellor, Nick Desaulniers, Bill Wendling, Justin Stitt,
Andrew Lunn, Heiner Kallweit, Russell King, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Bjorn Helgaas,
Arnd Bergmann, Jens Axboe, Krzysztof Wilczyński,
rust-for-linux, linux-kernel, linux-kselftest, kunit-dev,
dri-devel, netdev, devicetree, llvm, linux-pci, nouveau,
linux-block
On Tue, May 27, 2025 at 8:44 AM Alice Ryhl <aliceryhl@google.com> wrote:
>
> On Tue, May 27, 2025 at 12:18 AM Tamir Duberstein <tamird@gmail.com> wrote:
> > > > +}
> > > > +
> > > > +fn make_ident<'a, T: IntoIterator<Item = &'a str>>(
> > > > + span: Span,
> > > > + names: T,
> > > > +) -> impl Iterator<Item = TokenTree> + use<'a, T> {
> > > > + names.into_iter().flat_map(move |name| {
> > > > + [
> > > > + TokenTree::Punct(Punct::new(':', Spacing::Joint)),
> > > > + TokenTree::Punct(Punct::new(':', Spacing::Alone)),
> > > > + TokenTree::Ident(Ident::new(name, span)),
> > > > + ]
> > > > + })
> > > > +}
> > > > diff --git a/rust/macros/lib.rs b/rust/macros/lib.rs
> > > > index d31e50c446b0..fa956eaa3ba7 100644
> > > > --- a/rust/macros/lib.rs
> > > > +++ b/rust/macros/lib.rs
> > > > @@ -10,6 +10,7 @@
> > > > mod quote;
> > > > mod concat_idents;
> > > > mod export;
> > > > +mod fmt;
> > > > mod helpers;
> > > > mod kunit;
> > > > mod module;
> > > > @@ -196,6 +197,24 @@ pub fn export(attr: TokenStream, ts: TokenStream) -> TokenStream {
> > > > export::export(attr, ts)
> > > > }
> > > >
> > > > +/// Like [`core::format_args!`], but automatically wraps arguments in [`kernel::fmt::Adapter`].
> > > > +///
> > > > +/// This macro allows generating `core::fmt::Arguments` while ensuring that each argument is wrapped
> > > > +/// with `::kernel::fmt::Adapter`, which customizes formatting behavior for kernel logging.
> > > > +///
> > > > +/// Named arguments used in the format string (e.g. `{foo}`) are detected and resolved from local
> > > > +/// bindings. All positional and named arguments are automatically wrapped.
> > > > +///
> > > > +/// This macro is an implementation detail of other kernel logging macros like [`pr_info!`] and
> > > > +/// should not typically be used directly.
> > > > +///
> > > > +/// [`kernel::fmt::Adapter`]: ../kernel/fmt/struct.Adapter.html
> > > > +/// [`pr_info!`]: ../kernel/macro.pr_info.html
> > > > +#[proc_macro]
> > > > +pub fn fmt(input: TokenStream) -> TokenStream {
> > >
> > > I'm wondering if we should name this `format_args` instead in order to
> > > better communicate that it's a replacement for `core::format_args!`.
> >
> > Unfortunately that introduces ambiguity in cases where
> > kernel::prelude::* is imported because core::format_args is in core's
> > prelude.
>
> I'm pretty sure that glob imports are higher priority than the core
> prelude? Or is this because there are macros that now incorrectly use
> kernel::prelude::format_args when they should use the one from core?
compiler says no, e.g.:
error[E0659]: `format_args` is ambiguous
--> rust/doctests_kernel_generated.rs:8783:25
|
8783 | kernel::kunit::info(format_args!(" #
rust_doctest_kernel_workqueue_rs_3.location:
rust/kernel/workqueue.rs:77\n"));
| ^^^^^^^^^^^ ambiguous name
|
= note: ambiguous because of a conflict between a name from a
glob import and an outer scope during import or macro resolution
= note: `format_args` could refer to a macro from prelude
note: `format_args` could also refer to the macro imported here
--> rust/doctests_kernel_generated.rs:8772:9
|
8772 | use kernel::prelude::*;
| ^^^^^^^^^^^^^^^^^^
= help: consider adding an explicit import of `format_args` to disambiguate
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH v10 3/5] rust: replace `CStr` with `core::ffi::CStr`
2025-05-24 20:33 [PATCH v10 0/5] rust: replace kernel::str::CStr w/ core::ffi::CStr Tamir Duberstein
2025-05-24 20:33 ` [PATCH v10 1/5] rust: retitle "Example" section as "Examples" Tamir Duberstein
2025-05-24 20:33 ` [PATCH v10 2/5] rust: support formatting of foreign types Tamir Duberstein
@ 2025-05-24 20:33 ` Tamir Duberstein
2025-05-26 14:56 ` Benno Lossin
2025-05-24 20:33 ` [PATCH v10 4/5] rust: replace `kernel::c_str!` with C-Strings Tamir Duberstein
` (2 subsequent siblings)
5 siblings, 1 reply; 34+ messages in thread
From: Tamir Duberstein @ 2025-05-24 20:33 UTC (permalink / raw)
To: Michal Rostecki, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Brendan Higgins, David Gow, Rae Moar, Danilo Krummrich,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Greg Kroah-Hartman, Rafael J. Wysocki,
Luis Chamberlain, Russ Weight, FUJITA Tomonori, Rob Herring,
Saravana Kannan, Peter Zijlstra, Ingo Molnar, Will Deacon,
Waiman Long, Nathan Chancellor, Nick Desaulniers, Bill Wendling,
Justin Stitt, Andrew Lunn, Heiner Kallweit, Russell King,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Bjorn Helgaas, Arnd Bergmann, Jens Axboe, Benno Lossin,
Krzysztof Wilczyński, Benno Lossin
Cc: rust-for-linux, linux-kernel, linux-kselftest, kunit-dev,
dri-devel, netdev, devicetree, llvm, linux-pci, nouveau,
linux-block, 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>
---
drivers/gpu/drm/drm_panic_qr.rs | 2 +-
rust/kernel/device.rs | 4 +-
rust/kernel/error.rs | 4 +-
rust/kernel/firmware.rs | 11 +-
rust/kernel/kunit.rs | 6 +-
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 | 358 +++++++++-------------------------------
rust/kernel/sync/condvar.rs | 2 +-
rust/kernel/sync/lock.rs | 2 +-
rust/kernel/sync/lock/global.rs | 2 +-
14 files changed, 112 insertions(+), 294 deletions(-)
diff --git a/drivers/gpu/drm/drm_panic_qr.rs b/drivers/gpu/drm/drm_panic_qr.rs
index f2a99681b998..d8192a9bef63 100644
--- a/drivers/gpu/drm/drm_panic_qr.rs
+++ b/drivers/gpu/drm/drm_panic_qr.rs
@@ -922,7 +922,7 @@ fn draw_all(&mut self, data: impl Iterator<Item = u8>) {
// nul-terminated string.
let url_cstr: &CStr = unsafe { CStr::from_char_ptr(url) };
let segments = &[
- &Segment::Binary(url_cstr.as_bytes()),
+ &Segment::Binary(url_cstr.to_bytes()),
&Segment::Numeric(&data_slice[0..data_len]),
];
match EncodedMsg::new(segments, tmp_slice) {
diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs
index 99d99a76934c..9074322c79e8 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 3dee3139fcd4..933c048c04f1 100644
--- a/rust/kernel/error.rs
+++ b/rust/kernel/error.rs
@@ -164,6 +164,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) })
}
@@ -188,7 +190,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 2494c96e105f..582ab648b14c 100644
--- a/rust/kernel/firmware.rs
+++ b/rust/kernel/firmware.rs
@@ -4,7 +4,14 @@
//!
//! C header: [`include/linux/firmware.h`](srctree/include/linux/firmware.h)
-use crate::{bindings, device::Device, error::Error, error::Result, ffi, str::CStr};
+use crate::{
+ bindings,
+ device::Device,
+ error::Error,
+ error::Result,
+ ffi,
+ str::{CStr, CStrExt as _},
+};
use core::ptr::NonNull;
/// # Invariants
@@ -290,7 +297,7 @@ const fn push_module_name(self) -> Self {
let module_name = this.module_name;
if !this.module_name.is_empty() {
- this = this.push_internal(module_name.as_bytes_with_nul());
+ this = this.push_internal(module_name.to_bytes_with_nul());
if N != 0 {
// Re-use the space taken by the NULL terminator and swap it with the '.' separator.
diff --git a/rust/kernel/kunit.rs b/rust/kernel/kunit.rs
index c29e34192553..e5621d596ed3 100644
--- a/rust/kernel/kunit.rs
+++ b/rust/kernel/kunit.rs
@@ -100,12 +100,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,
});
@@ -175,7 +175,7 @@ pub const fn kunit_case(
) -> kernel::bindings::kunit_case {
kernel::bindings::kunit_case {
run_case: Some(run_case),
- name: name.as_char_ptr(),
+ name: kernel::str::as_char_ptr_in_const_context(name),
attr: kernel::bindings::kunit_attributes {
speed: kernel::bindings::kunit_speed_KUNIT_SPEED_NORMAL,
},
diff --git a/rust/kernel/miscdevice.rs b/rust/kernel/miscdevice.rs
index b4c5f74de23d..d684ec4ef4d0 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 = MiscdeviceVTable::<T>::build();
result
}
diff --git a/rust/kernel/net/phy.rs b/rust/kernel/net/phy.rs
index a59469c785e3..652e060e47bd 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).cast_mut(),
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 ef1efcb9d945..97e8bcf73669 100644
--- a/rust/kernel/prelude.rs
+++ b/rust/kernel/prelude.rs
@@ -35,7 +35,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;
diff --git a/rust/kernel/seq_file.rs b/rust/kernel/seq_file.rs
index 627bc2f7b3d2..1011743dd0ce 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)]
@@ -36,7 +36,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 302423ca5eb0..586644912414 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, Index};
use crate::error::{code::*, Error};
@@ -57,11 +57,11 @@ impl fmt::Display for BStr {
/// # use kernel::{prelude::fmt, b_str, str::{BStr, CString}};
/// 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\\xa6\\x80".as_bytes());
+ /// assert_eq!(s.to_bytes(), "\\xf0\\x9f\\xa6\\x80".as_bytes());
/// # Ok::<(), kernel::error::Error>(())
/// ```
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
@@ -89,11 +89,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 {
@@ -175,55 +175,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
@@ -231,54 +195,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.
@@ -287,77 +206,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.
///
@@ -368,11 +226,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.
///
@@ -383,11 +237,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.
@@ -398,13 +248,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.
@@ -415,13 +259,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>;
}
impl fmt::Display for crate::fmt::Adapter<&CStr> {
@@ -434,16 +272,16 @@ impl fmt::Display for crate::fmt::Adapter<&CStr> {
/// # 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());
+ /// 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))?;
- /// 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 fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
let Self(cstr) = self;
- for &c in cstr.as_bytes() {
+ for &c in cstr.to_bytes() {
if (0x20..0x7f).contains(&c) {
// Printable character.
f.write_char(c as char)?;
@@ -455,98 +293,75 @@ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
}
}
-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::prelude::fmt;
- /// # use kernel::str::CStr;
- /// # 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{c:02x}")?,
- }
- }
- 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;
-
+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,15 +445,6 @@ 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();
@@ -854,11 +660,11 @@ fn write_str(&mut self, s: &str) -> fmt::Result {
/// use kernel::{str::CString, prelude::fmt};
///
/// 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 +734,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 caebf03f553b..0b6bc7f2878d 100644
--- a/rust/kernel/sync/condvar.rs
+++ b/rust/kernel/sync/condvar.rs
@@ -8,7 +8,7 @@
use super::{lock::Backend, lock::Guard, LockClassKey};
use crate::{
ffi::{c_int, c_long},
- str::CStr,
+ str::{CStr, CStrExt as _},
task::{
MAX_SCHEDULE_TIMEOUT, TASK_FREEZABLE, TASK_INTERRUPTIBLE, TASK_NORMAL, TASK_UNINTERRUPTIBLE,
},
diff --git a/rust/kernel/sync/lock.rs b/rust/kernel/sync/lock.rs
index e82fa5be289c..a777a22976e0 100644
--- a/rust/kernel/sync/lock.rs
+++ b/rust/kernel/sync/lock.rs
@@ -7,7 +7,7 @@
use super::LockClassKey;
use crate::{
- str::CStr,
+ str::{CStr, CStrExt as _},
types::{NotThreadSafe, Opaque, ScopeGuard},
};
use core::{cell::UnsafeCell, marker::PhantomPinned, pin::Pin};
diff --git a/rust/kernel/sync/lock/global.rs b/rust/kernel/sync/lock/global.rs
index d65f94b5caf2..79d0ef7fda86 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.49.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [PATCH v10 3/5] rust: replace `CStr` with `core::ffi::CStr`
2025-05-24 20:33 ` [PATCH v10 3/5] rust: replace `CStr` with `core::ffi::CStr` Tamir Duberstein
@ 2025-05-26 14:56 ` Benno Lossin
2025-05-26 22:24 ` Tamir Duberstein
0 siblings, 1 reply; 34+ messages in thread
From: Benno Lossin @ 2025-05-26 14:56 UTC (permalink / raw)
To: Tamir Duberstein, Michal Rostecki, Miguel Ojeda, Alex Gaynor,
Boqun Feng, Gary Guo, Björn Roy Baron, Andreas Hindborg,
Alice Ryhl, Trevor Gross, Brendan Higgins, David Gow, Rae Moar,
Danilo Krummrich, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter,
Greg Kroah-Hartman, Rafael J. Wysocki, Luis Chamberlain,
Russ Weight, FUJITA Tomonori, Rob Herring, Saravana Kannan,
Peter Zijlstra, Ingo Molnar, Will Deacon, Waiman Long,
Nathan Chancellor, Nick Desaulniers, Bill Wendling, Justin Stitt,
Andrew Lunn, Heiner Kallweit, Russell King, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Bjorn Helgaas,
Arnd Bergmann, Jens Axboe, Krzysztof Wilczyński
Cc: rust-for-linux, linux-kernel, linux-kselftest, kunit-dev,
dri-devel, netdev, devicetree, llvm, linux-pci, nouveau,
linux-block
On Sat May 24, 2025 at 10:33 PM CEST, Tamir Duberstein wrote:
> `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.
What's this supposed to mean?
> 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.
Similarly this, the message should explain the motivation for the
change, the change itself and can include additional information.
>
> Signed-off-by: Tamir Duberstein <tamird@gmail.com>
> ---
> drivers/gpu/drm/drm_panic_qr.rs | 2 +-
> rust/kernel/device.rs | 4 +-
> rust/kernel/error.rs | 4 +-
> rust/kernel/firmware.rs | 11 +-
> rust/kernel/kunit.rs | 6 +-
> 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 | 358 +++++++++-------------------------------
> rust/kernel/sync/condvar.rs | 2 +-
> rust/kernel/sync/lock.rs | 2 +-
> rust/kernel/sync/lock/global.rs | 2 +-
> 14 files changed, 112 insertions(+), 294 deletions(-)
I'm a bit confused by some of the diffs here, they seem pretty messy,
any chance that they can be improved?
---
Cheers,
Benno
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v10 3/5] rust: replace `CStr` with `core::ffi::CStr`
2025-05-26 14:56 ` Benno Lossin
@ 2025-05-26 22:24 ` Tamir Duberstein
2025-05-26 23:03 ` Benno Lossin
0 siblings, 1 reply; 34+ messages in thread
From: Tamir Duberstein @ 2025-05-26 22:24 UTC (permalink / raw)
To: Benno Lossin
Cc: Michal Rostecki, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Brendan Higgins, David Gow, Rae Moar, Danilo Krummrich,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Greg Kroah-Hartman, Rafael J. Wysocki,
Luis Chamberlain, Russ Weight, FUJITA Tomonori, Rob Herring,
Saravana Kannan, Peter Zijlstra, Ingo Molnar, Will Deacon,
Waiman Long, Nathan Chancellor, Nick Desaulniers, Bill Wendling,
Justin Stitt, Andrew Lunn, Heiner Kallweit, Russell King,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Bjorn Helgaas, Arnd Bergmann, Jens Axboe,
Krzysztof Wilczyński, rust-for-linux, linux-kernel,
linux-kselftest, kunit-dev, dri-devel, netdev, devicetree, llvm,
linux-pci, nouveau, linux-block
On Mon, May 26, 2025 at 10:56 AM Benno Lossin <lossin@kernel.org> wrote:
>
> On Sat May 24, 2025 at 10:33 PM CEST, Tamir Duberstein wrote:
> > `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.
>
> What's this supposed to mean?
It means that kernel::str::CStr was introduced before core::ffi:CStr
was available. I didn't check this before, but it is indeed true - see
https://github.com/Rust-for-Linux/linux/commit/faa3cbcca03d0dec8f8e43f1d8d5c0860d98a23f.
>
> > 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.
>
> Similarly this, the message should explain the motivation for the
> change, the change itself and can include additional information.
The motivation is implied (that using standard types is preferable to
having custom ones; this is also implicit rather than explicit in
https://github.com/Rust-for-Linux/linux/issues/1075), but I can
sharpen it.
>
> >
> > Signed-off-by: Tamir Duberstein <tamird@gmail.com>
> > ---
> > drivers/gpu/drm/drm_panic_qr.rs | 2 +-
> > rust/kernel/device.rs | 4 +-
> > rust/kernel/error.rs | 4 +-
> > rust/kernel/firmware.rs | 11 +-
> > rust/kernel/kunit.rs | 6 +-
> > 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 | 358 +++++++++-------------------------------
> > rust/kernel/sync/condvar.rs | 2 +-
> > rust/kernel/sync/lock.rs | 2 +-
> > rust/kernel/sync/lock/global.rs | 2 +-
> > 14 files changed, 112 insertions(+), 294 deletions(-)
>
> I'm a bit confused by some of the diffs here, they seem pretty messy,
> any chance that they can be improved?
I'm open to suggestions. I think the confusion arises from git trying
to keep code from moving; fundamentally much of the change is moving
methods to an extension trait, which means git has to choose between
keeping the documentation where it is, or keeping the implementation
where it is. If I use `--patience` then everything moves together, but
then the diffstat swells. Thoughts?
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v10 3/5] rust: replace `CStr` with `core::ffi::CStr`
2025-05-26 22:24 ` Tamir Duberstein
@ 2025-05-26 23:03 ` Benno Lossin
2025-05-27 15:31 ` Tamir Duberstein
0 siblings, 1 reply; 34+ messages in thread
From: Benno Lossin @ 2025-05-26 23:03 UTC (permalink / raw)
To: Tamir Duberstein
Cc: Michal Rostecki, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Brendan Higgins, David Gow, Rae Moar, Danilo Krummrich,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Greg Kroah-Hartman, Rafael J. Wysocki,
Luis Chamberlain, Russ Weight, FUJITA Tomonori, Rob Herring,
Saravana Kannan, Peter Zijlstra, Ingo Molnar, Will Deacon,
Waiman Long, Nathan Chancellor, Nick Desaulniers, Bill Wendling,
Justin Stitt, Andrew Lunn, Heiner Kallweit, Russell King,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Bjorn Helgaas, Arnd Bergmann, Jens Axboe,
Krzysztof Wilczyński, rust-for-linux, linux-kernel,
linux-kselftest, kunit-dev, dri-devel, netdev, devicetree, llvm,
linux-pci, nouveau, linux-block
On Tue May 27, 2025 at 12:24 AM CEST, Tamir Duberstein wrote:
> On Mon, May 26, 2025 at 10:56 AM Benno Lossin <lossin@kernel.org> wrote:
>>
>> On Sat May 24, 2025 at 10:33 PM CEST, Tamir Duberstein wrote:
>> > `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.
>>
>> What's this supposed to mean?
>
> It means that kernel::str::CStr was introduced before core::ffi:CStr
> was available. I didn't check this before, but it is indeed true - see
> https://github.com/Rust-for-Linux/linux/commit/faa3cbcca03d0dec8f8e43f1d8d5c0860d98a23f.
I see, then just write that and mention the commit.
>> > 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.
>>
>> Similarly this, the message should explain the motivation for the
>> change, the change itself and can include additional information.
>
> The motivation is implied (that using standard types is preferable to
> having custom ones; this is also implicit rather than explicit in
> https://github.com/Rust-for-Linux/linux/issues/1075), but I can
> sharpen it.
Please add this information to the commit message.
>> > Signed-off-by: Tamir Duberstein <tamird@gmail.com>
>> > ---
>> > drivers/gpu/drm/drm_panic_qr.rs | 2 +-
>> > rust/kernel/device.rs | 4 +-
>> > rust/kernel/error.rs | 4 +-
>> > rust/kernel/firmware.rs | 11 +-
>> > rust/kernel/kunit.rs | 6 +-
>> > 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 | 358 +++++++++-------------------------------
>> > rust/kernel/sync/condvar.rs | 2 +-
>> > rust/kernel/sync/lock.rs | 2 +-
>> > rust/kernel/sync/lock/global.rs | 2 +-
>> > 14 files changed, 112 insertions(+), 294 deletions(-)
>>
>> I'm a bit confused by some of the diffs here, they seem pretty messy,
>> any chance that they can be improved?
>
> I'm open to suggestions. I think the confusion arises from git trying
> to keep code from moving; fundamentally much of the change is moving
> methods to an extension trait, which means git has to choose between
> keeping the documentation where it is, or keeping the implementation
> where it is. If I use `--patience` then everything moves together, but
> then the diffstat swells. Thoughts?
I haven't viewed this patch with color-moved (since I haven't applied it
locally), I can try that first and see if it helps.
---
Cheers,
Benno
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v10 3/5] rust: replace `CStr` with `core::ffi::CStr`
2025-05-26 23:03 ` Benno Lossin
@ 2025-05-27 15:31 ` Tamir Duberstein
0 siblings, 0 replies; 34+ messages in thread
From: Tamir Duberstein @ 2025-05-27 15:31 UTC (permalink / raw)
To: Benno Lossin
Cc: Michal Rostecki, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Brendan Higgins, David Gow, Rae Moar, Danilo Krummrich,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Greg Kroah-Hartman, Rafael J. Wysocki,
Luis Chamberlain, Russ Weight, FUJITA Tomonori, Rob Herring,
Saravana Kannan, Peter Zijlstra, Ingo Molnar, Will Deacon,
Waiman Long, Nathan Chancellor, Nick Desaulniers, Bill Wendling,
Justin Stitt, Andrew Lunn, Heiner Kallweit, Russell King,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Bjorn Helgaas, Arnd Bergmann, Jens Axboe,
Krzysztof Wilczyński, rust-for-linux, linux-kernel,
linux-kselftest, kunit-dev, dri-devel, netdev, devicetree, llvm,
linux-pci, nouveau, linux-block
On Mon, May 26, 2025 at 7:03 PM Benno Lossin <lossin@kernel.org> wrote:
>
> On Tue May 27, 2025 at 12:24 AM CEST, Tamir Duberstein wrote:
> > On Mon, May 26, 2025 at 10:56 AM Benno Lossin <lossin@kernel.org> wrote:
> >>
> >> On Sat May 24, 2025 at 10:33 PM CEST, Tamir Duberstein wrote:
> >> > `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.
> >>
> >> What's this supposed to mean?
> >
> > It means that kernel::str::CStr was introduced before core::ffi:CStr
> > was available. I didn't check this before, but it is indeed true - see
> > https://github.com/Rust-for-Linux/linux/commit/faa3cbcca03d0dec8f8e43f1d8d5c0860d98a23f.
>
> I see, then just write that and mention the commit.
👍
> >> > 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.
> >>
> >> Similarly this, the message should explain the motivation for the
> >> change, the change itself and can include additional information.
> >
> > The motivation is implied (that using standard types is preferable to
> > having custom ones; this is also implicit rather than explicit in
> > https://github.com/Rust-for-Linux/linux/issues/1075), but I can
> > sharpen it.
>
> Please add this information to the commit message.
👍
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH v10 4/5] rust: replace `kernel::c_str!` with C-Strings
2025-05-24 20:33 [PATCH v10 0/5] rust: replace kernel::str::CStr w/ core::ffi::CStr Tamir Duberstein
` (2 preceding siblings ...)
2025-05-24 20:33 ` [PATCH v10 3/5] rust: replace `CStr` with `core::ffi::CStr` Tamir Duberstein
@ 2025-05-24 20:33 ` Tamir Duberstein
2025-05-26 15:04 ` Benno Lossin
2025-05-24 20:33 ` [PATCH v10 5/5] rust: remove core::ffi::CStr reexport Tamir Duberstein
2025-05-28 10:38 ` [PATCH v10 0/5] rust: replace kernel::str::CStr w/ core::ffi::CStr Alice Ryhl
5 siblings, 1 reply; 34+ messages in thread
From: Tamir Duberstein @ 2025-05-24 20:33 UTC (permalink / raw)
To: Michal Rostecki, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Brendan Higgins, David Gow, Rae Moar, Danilo Krummrich,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Greg Kroah-Hartman, Rafael J. Wysocki,
Luis Chamberlain, Russ Weight, FUJITA Tomonori, Rob Herring,
Saravana Kannan, Peter Zijlstra, Ingo Molnar, Will Deacon,
Waiman Long, Nathan Chancellor, Nick Desaulniers, Bill Wendling,
Justin Stitt, Andrew Lunn, Heiner Kallweit, Russell King,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Bjorn Helgaas, Arnd Bergmann, Jens Axboe, Benno Lossin,
Krzysztof Wilczyński, Benno Lossin
Cc: rust-for-linux, linux-kernel, linux-kselftest, kunit-dev,
dri-devel, netdev, devicetree, llvm, linux-pci, nouveau,
linux-block, 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/gpu/nova-core/driver.rs | 2 +-
drivers/net/phy/ax88796b_rust.rs | 7 +++----
drivers/net/phy/qt2025.rs | 5 ++---
rust/kernel/devres.rs | 2 +-
rust/kernel/firmware.rs | 6 +++---
rust/kernel/kunit.rs | 7 ++++---
rust/kernel/net/phy.rs | 6 ++----
rust/kernel/platform.rs | 4 ++--
rust/kernel/str.rs | 38 ++++++++++++++++++++++++------------
rust/kernel/sync.rs | 7 +++----
rust/kernel/sync/lock/global.rs | 3 ++-
rust/macros/kunit.rs | 6 +++---
rust/macros/module.rs | 2 +-
samples/rust/rust_driver_faux.rs | 4 ++--
samples/rust/rust_driver_pci.rs | 4 ++--
samples/rust/rust_driver_platform.rs | 4 ++--
samples/rust/rust_misc_device.rs | 3 +--
17 files changed, 59 insertions(+), 51 deletions(-)
diff --git a/drivers/gpu/nova-core/driver.rs b/drivers/gpu/nova-core/driver.rs
index a08fb6599267..776970049974 100644
--- a/drivers/gpu/nova-core/driver.rs
+++ b/drivers/gpu/nova-core/driver.rs
@@ -33,7 +33,7 @@ fn probe(pdev: &pci::Device<Core>, _info: &Self::IdInfo) -> Result<Pin<KBox<Self
pdev.enable_device_mem()?;
pdev.set_master();
- let bar = pdev.iomap_region_sized::<BAR0_SIZE>(0, c_str!("nova-core/bar0"))?;
+ let bar = pdev.iomap_region_sized::<BAR0_SIZE>(0, c"nova-core/bar0")?;
let this = KBox::pin_init(
try_pin_init!(Self {
diff --git a/drivers/net/phy/ax88796b_rust.rs b/drivers/net/phy/ax88796b_rust.rs
index bc73ebccc2aa..2d24628a4e58 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 0b9400dcb4c1..9ccc75f70219 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 ddb1ce4a78d9..8735b227f0d3 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 582ab648b14c..09fd3a27bcf0 100644
--- a/rust/kernel/firmware.rs
+++ b/rust/kernel/firmware.rs
@@ -51,13 +51,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(())
@@ -203,7 +203,7 @@ macro_rules! module_firmware {
($($builder:tt)*) => {
const _: () = {
const __MODULE_FIRMWARE_PREFIX: &'static $crate::str::CStr = if cfg!(MODULE) {
- $crate::c_str!("")
+ c""
} else {
<LocalModule as $crate::ModuleMetadata>::NAME
};
diff --git a/rust/kernel/kunit.rs b/rust/kernel/kunit.rs
index e5621d596ed3..09148e982f48 100644
--- a/rust/kernel/kunit.rs
+++ b/rust/kernel/kunit.rs
@@ -58,9 +58,10 @@ macro_rules! kunit_assert {
break 'out;
}
- static FILE: &'static $crate::str::CStr = $crate::c_str!($file);
+ 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() };
@@ -222,7 +223,7 @@ pub const fn kunit_case_null() -> kernel::bindings::kunit_case {
/// }
///
/// static mut KUNIT_TEST_CASES: [kernel::bindings::kunit_case; 2] = [
-/// kernel::kunit::kunit_case(kernel::c_str!("name"), test_fn),
+/// kernel::kunit::kunit_case(c"name", test_fn),
/// kernel::kunit::kunit_case_null(),
/// ];
/// kernel::kunit_unsafe_test_suite!(suite_name, KUNIT_TEST_CASES);
diff --git a/rust/kernel/net/phy.rs b/rust/kernel/net/phy.rs
index 652e060e47bd..8129419a3931 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 fd4a494f30e8..7163fc468b32 100644
--- a/rust/kernel/platform.rs
+++ b/rust/kernel/platform.rs
@@ -124,7 +124,7 @@ macro_rules! module_platform_driver {
/// # Example
///
///```
-/// # use kernel::{bindings, c_str, device::Core, of, platform};
+/// # use kernel::{bindings, device::Core, of, platform};
///
/// struct MyDriver;
///
@@ -133,7 +133,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 586644912414..0385d927fcd5 100644
--- a/rust/kernel/str.rs
+++ b/rust/kernel/str.rs
@@ -266,15 +266,14 @@ impl fmt::Display for crate::fmt::Adapter<&CStr> {
/// Formats printable ASCII characters, escaping the rest.
///
/// ```
- /// # use kernel::c_str;
/// # use kernel::prelude::fmt;
/// # use kernel::str::CStr;
/// # use kernel::str::CString;
- /// let penguin = c_str!("🐧");
+ /// let penguin = c"🐧";
/// let s = CString::try_from_fmt(fmt!("{}", penguin))?;
/// 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))?;
/// assert_eq!(s.to_bytes_with_nul(), "so \"cool\"\0".as_bytes());
/// # Ok::<(), kernel::error::Error>(())
@@ -365,25 +364,38 @@ 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(err) => {
+ let _: core::ffi::FromBytesWithNulError = err;
+ panic!("string contains interior NUL")
+ }
+ };
C
}};
}
diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs
index 36a719015583..424864fb448f 100644
--- a/rust/kernel/sync.rs
+++ b/rust/kernel/sync.rs
@@ -41,7 +41,6 @@ impl LockClassKey {
///
/// # Example
/// ```
- /// # use kernel::c_str;
/// # use kernel::alloc::KBox;
/// # use kernel::types::ForeignOwnable;
/// # use kernel::sync::{LockClassKey, SpinLock};
@@ -53,7 +52,7 @@ impl LockClassKey {
/// {
/// stack_pin_init!(let num: SpinLock<u32> = SpinLock::new(
/// 0,
- /// c_str!("my_spinlock"),
+ /// c"my_spinlock",
/// // SAFETY: `key_ptr` is returned by the above `into_foreign()`, whose
/// // `from_foreign()` has not yet been called.
/// unsafe { <Pin<KBox<LockClassKey>> as ForeignOwnable>::borrow(key_ptr) }
@@ -106,9 +105,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 79d0ef7fda86..0ca23b12427c 100644
--- a/rust/kernel/sync/lock/global.rs
+++ b/rust/kernel/sync/lock/global.rs
@@ -267,7 +267,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/kunit.rs b/rust/macros/kunit.rs
index 99ccac82edde..56469fdcee3f 100644
--- a/rust/macros/kunit.rs
+++ b/rust/macros/kunit.rs
@@ -89,8 +89,8 @@ pub(crate) fn kunit_tests(attr: TokenStream, ts: TokenStream) -> TokenStream {
// unsafe extern "C" fn kunit_rust_wrapper_bar(_test: *mut kernel::bindings::kunit) { bar(); }
//
// static mut TEST_CASES: [kernel::bindings::kunit_case; 3] = [
- // kernel::kunit::kunit_case(kernel::c_str!("foo"), kunit_rust_wrapper_foo),
- // kernel::kunit::kunit_case(kernel::c_str!("bar"), kunit_rust_wrapper_bar),
+ // kernel::kunit::kunit_case(c"foo", kunit_rust_wrapper_foo),
+ // kernel::kunit::kunit_case(c"bar", kunit_rust_wrapper_bar),
// kernel::kunit::kunit_case_null(),
// ];
//
@@ -106,7 +106,7 @@ pub(crate) fn kunit_tests(attr: TokenStream, ts: TokenStream) -> TokenStream {
writeln!(kunit_macros, "{kunit_wrapper}").unwrap();
writeln!(
test_cases,
- " kernel::kunit::kunit_case(kernel::c_str!(\"{test}\"), {kunit_wrapper_fn_name}),"
+ " kernel::kunit::kunit_case(c\"{test}\", {kunit_wrapper_fn_name}),",
)
.unwrap();
}
diff --git a/rust/macros/module.rs b/rust/macros/module.rs
index 44e5cb108cea..7b71d6d99d5c 100644
--- a/rust/macros/module.rs
+++ b/rust/macros/module.rs
@@ -232,7 +232,7 @@ pub(crate) fn module(ts: TokenStream) -> TokenStream {
type LocalModule = {type_};
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_faux.rs b/samples/rust/rust_driver_faux.rs
index ecc9fd378cbd..23add3160693 100644
--- a/samples/rust/rust_driver_faux.rs
+++ b/samples/rust/rust_driver_faux.rs
@@ -2,7 +2,7 @@
//! Rust faux device sample.
-use kernel::{c_str, faux, prelude::*, Module};
+use kernel::{faux, prelude::*, Module};
module! {
type: SampleModule,
@@ -20,7 +20,7 @@ impl Module for SampleModule {
fn init(_module: &'static ThisModule) -> Result<Self> {
pr_info!("Initialising Rust Faux Device Sample\n");
- let reg = faux::Registration::new(c_str!("rust-faux-sample-device"), None)?;
+ let reg = faux::Registration::new(c"rust-faux-sample-device", None)?;
dev_info!(reg.as_ref(), "Hello from faux device!\n");
diff --git a/samples/rust/rust_driver_pci.rs b/samples/rust/rust_driver_pci.rs
index 2bb260aebc9e..8da48c1c3c2d 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, device::Core, devres::Devres, pci, prelude::*, types::ARef};
+use kernel::{bindings, device::Core, devres::Devres, pci, prelude::*, types::ARef};
struct Regs;
@@ -73,7 +73,7 @@ fn probe(pdev: &pci::Device<Core>, 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 8b42b3cfb363..e6487a970a59 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, device::Core, of, platform, prelude::*, types::ARef};
+use kernel::{device::Core, of, platform, prelude::*, types::ARef};
struct SampleDriver {
pdev: ARef<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 c881fd6dbd08..12b64296e912 100644
--- a/samples/rust/rust_misc_device.rs
+++ b/samples/rust/rust_misc_device.rs
@@ -98,7 +98,6 @@
use core::pin::Pin;
use kernel::{
- c_str,
device::Device,
fs::File,
ioctl::{_IO, _IOC_SIZE, _IOR, _IOW},
@@ -133,7 +132,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.49.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [PATCH v10 4/5] rust: replace `kernel::c_str!` with C-Strings
2025-05-24 20:33 ` [PATCH v10 4/5] rust: replace `kernel::c_str!` with C-Strings Tamir Duberstein
@ 2025-05-26 15:04 ` Benno Lossin
2025-05-26 22:29 ` Tamir Duberstein
0 siblings, 1 reply; 34+ messages in thread
From: Benno Lossin @ 2025-05-26 15:04 UTC (permalink / raw)
To: Tamir Duberstein, Michal Rostecki, Miguel Ojeda, Alex Gaynor,
Boqun Feng, Gary Guo, Björn Roy Baron, Andreas Hindborg,
Alice Ryhl, Trevor Gross, Brendan Higgins, David Gow, Rae Moar,
Danilo Krummrich, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter,
Greg Kroah-Hartman, Rafael J. Wysocki, Luis Chamberlain,
Russ Weight, FUJITA Tomonori, Rob Herring, Saravana Kannan,
Peter Zijlstra, Ingo Molnar, Will Deacon, Waiman Long,
Nathan Chancellor, Nick Desaulniers, Bill Wendling, Justin Stitt,
Andrew Lunn, Heiner Kallweit, Russell King, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Bjorn Helgaas,
Arnd Bergmann, Jens Axboe, Krzysztof Wilczyński
Cc: rust-for-linux, linux-kernel, linux-kselftest, kunit-dev,
dri-devel, netdev, devicetree, llvm, linux-pci, nouveau,
linux-block
On Sat May 24, 2025 at 10:33 PM CEST, Tamir Duberstein wrote:
> -/// 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 {
I don't like this name, how about `concat_to_c_str` or
`concat_with_nul`?
This macro also is useful from macros that have a normal string literal,
but can't turn it into a `c""` one.
> + // 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()) {
Why is this still our CStr?
> + Ok(v) => v,
> + Err(err) => {
> + let _: core::ffi::FromBytesWithNulError = err;
Is this really necessary?
---
Cheers,
Benno
> + panic!("string contains interior NUL")
> + }
> + };
> C
> }};
> }
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH v10 4/5] rust: replace `kernel::c_str!` with C-Strings
2025-05-26 15:04 ` Benno Lossin
@ 2025-05-26 22:29 ` Tamir Duberstein
2025-05-26 23:06 ` Benno Lossin
2025-05-28 10:36 ` Alice Ryhl
0 siblings, 2 replies; 34+ messages in thread
From: Tamir Duberstein @ 2025-05-26 22:29 UTC (permalink / raw)
To: Benno Lossin
Cc: Michal Rostecki, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Brendan Higgins, David Gow, Rae Moar, Danilo Krummrich,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Greg Kroah-Hartman, Rafael J. Wysocki,
Luis Chamberlain, Russ Weight, FUJITA Tomonori, Rob Herring,
Saravana Kannan, Peter Zijlstra, Ingo Molnar, Will Deacon,
Waiman Long, Nathan Chancellor, Nick Desaulniers, Bill Wendling,
Justin Stitt, Andrew Lunn, Heiner Kallweit, Russell King,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Bjorn Helgaas, Arnd Bergmann, Jens Axboe,
Krzysztof Wilczyński, rust-for-linux, linux-kernel,
linux-kselftest, kunit-dev, dri-devel, netdev, devicetree, llvm,
linux-pci, nouveau, linux-block
On Mon, May 26, 2025 at 11:04 AM Benno Lossin <lossin@kernel.org> wrote:
>
> On Sat May 24, 2025 at 10:33 PM CEST, Tamir Duberstein wrote:
> > +macro_rules! c_str_avoid_literals {
>
> I don't like this name, how about `concat_to_c_str` or
> `concat_with_nul`?
>
> This macro also is useful from macros that have a normal string literal,
> but can't turn it into a `c""` one.
Uh, can you give an example? I'm not attached to the name.
>
> > + // 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()) {
>
> Why is this still our CStr?
Good question. I'll just revert all the changes here, I don't need to
touch this.
>
> > + Ok(v) => v,
> > + Err(err) => {
> > + let _: core::ffi::FromBytesWithNulError = err;
>
> Is this really necessary?
No. Reverted in v11.
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH v10 4/5] rust: replace `kernel::c_str!` with C-Strings
2025-05-26 22:29 ` Tamir Duberstein
@ 2025-05-26 23:06 ` Benno Lossin
2025-05-27 15:31 ` Tamir Duberstein
2025-05-28 10:36 ` Alice Ryhl
1 sibling, 1 reply; 34+ messages in thread
From: Benno Lossin @ 2025-05-26 23:06 UTC (permalink / raw)
To: Tamir Duberstein
Cc: Michal Rostecki, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Brendan Higgins, David Gow, Rae Moar, Danilo Krummrich,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Greg Kroah-Hartman, Rafael J. Wysocki,
Luis Chamberlain, Russ Weight, FUJITA Tomonori, Rob Herring,
Saravana Kannan, Peter Zijlstra, Ingo Molnar, Will Deacon,
Waiman Long, Nathan Chancellor, Nick Desaulniers, Bill Wendling,
Justin Stitt, Andrew Lunn, Heiner Kallweit, Russell King,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Bjorn Helgaas, Arnd Bergmann, Jens Axboe,
Krzysztof Wilczyński, rust-for-linux, linux-kernel,
linux-kselftest, kunit-dev, dri-devel, netdev, devicetree, llvm,
linux-pci, nouveau, linux-block
On Tue May 27, 2025 at 12:29 AM CEST, Tamir Duberstein wrote:
> On Mon, May 26, 2025 at 11:04 AM Benno Lossin <lossin@kernel.org> wrote:
>> On Sat May 24, 2025 at 10:33 PM CEST, Tamir Duberstein wrote:
>> > +macro_rules! c_str_avoid_literals {
>>
>> I don't like this name, how about `concat_to_c_str` or
>> `concat_with_nul`?
>>
>> This macro also is useful from macros that have a normal string literal,
>> but can't turn it into a `c""` one.
>
> Uh, can you give an example? I'm not attached to the name.
There is one in this patch (:
diff --git a/rust/kernel/kunit.rs b/rust/kernel/kunit.rs
index e5621d596ed3..09148e982f48 100644
--- a/rust/kernel/kunit.rs
+++ b/rust/kernel/kunit.rs
@@ -58,9 +58,10 @@ macro_rules! kunit_assert {
break 'out;
}
- static FILE: &'static $crate::str::CStr = $crate::c_str!($file);
+ static FILE: &'static $crate::str::CStr = $crate::c_str_avoid_literals!($file);
---
Cheers,
Benno
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH v10 4/5] rust: replace `kernel::c_str!` with C-Strings
2025-05-26 23:06 ` Benno Lossin
@ 2025-05-27 15:31 ` Tamir Duberstein
0 siblings, 0 replies; 34+ messages in thread
From: Tamir Duberstein @ 2025-05-27 15:31 UTC (permalink / raw)
To: Benno Lossin
Cc: Michal Rostecki, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Brendan Higgins, David Gow, Rae Moar, Danilo Krummrich,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Greg Kroah-Hartman, Rafael J. Wysocki,
Luis Chamberlain, Russ Weight, FUJITA Tomonori, Rob Herring,
Saravana Kannan, Peter Zijlstra, Ingo Molnar, Will Deacon,
Waiman Long, Nathan Chancellor, Nick Desaulniers, Bill Wendling,
Justin Stitt, Andrew Lunn, Heiner Kallweit, Russell King,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Bjorn Helgaas, Arnd Bergmann, Jens Axboe,
Krzysztof Wilczyński, rust-for-linux, linux-kernel,
linux-kselftest, kunit-dev, dri-devel, netdev, devicetree, llvm,
linux-pci, nouveau, linux-block
On Mon, May 26, 2025 at 7:07 PM Benno Lossin <lossin@kernel.org> wrote:
>
> On Tue May 27, 2025 at 12:29 AM CEST, Tamir Duberstein wrote:
> > On Mon, May 26, 2025 at 11:04 AM Benno Lossin <lossin@kernel.org> wrote:
> >> On Sat May 24, 2025 at 10:33 PM CEST, Tamir Duberstein wrote:
> >> > +macro_rules! c_str_avoid_literals {
> >>
> >> I don't like this name, how about `concat_to_c_str` or
> >> `concat_with_nul`?
> >>
> >> This macro also is useful from macros that have a normal string literal,
> >> but can't turn it into a `c""` one.
> >
> > Uh, can you give an example? I'm not attached to the name.
>
> There is one in this patch (:
>
> diff --git a/rust/kernel/kunit.rs b/rust/kernel/kunit.rs
> index e5621d596ed3..09148e982f48 100644
> --- a/rust/kernel/kunit.rs
> +++ b/rust/kernel/kunit.rs
> @@ -58,9 +58,10 @@ macro_rules! kunit_assert {
> break 'out;
> }
>
> - static FILE: &'static $crate::str::CStr = $crate::c_str!($file);
> + static FILE: &'static $crate::str::CStr = $crate::c_str_avoid_literals!($file);
Great point, and an easy one to replace with a c-string literal. Done in v11.
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v10 4/5] rust: replace `kernel::c_str!` with C-Strings
2025-05-26 22:29 ` Tamir Duberstein
2025-05-26 23:06 ` Benno Lossin
@ 2025-05-28 10:36 ` Alice Ryhl
2025-05-28 15:34 ` Benno Lossin
1 sibling, 1 reply; 34+ messages in thread
From: Alice Ryhl @ 2025-05-28 10:36 UTC (permalink / raw)
To: Tamir Duberstein
Cc: Benno Lossin, Michal Rostecki, Miguel Ojeda, Alex Gaynor,
Boqun Feng, Gary Guo, Björn Roy Baron, Andreas Hindborg,
Trevor Gross, Brendan Higgins, David Gow, Rae Moar,
Danilo Krummrich, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter,
Greg Kroah-Hartman, Rafael J. Wysocki, Luis Chamberlain,
Russ Weight, FUJITA Tomonori, Rob Herring, Saravana Kannan,
Peter Zijlstra, Ingo Molnar, Will Deacon, Waiman Long,
Nathan Chancellor, Nick Desaulniers, Bill Wendling, Justin Stitt,
Andrew Lunn, Heiner Kallweit, Russell King, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Bjorn Helgaas,
Arnd Bergmann, Jens Axboe, Krzysztof Wilczyński,
rust-for-linux, linux-kernel, linux-kselftest, kunit-dev,
dri-devel, netdev, devicetree, llvm, linux-pci, nouveau,
linux-block
On Mon, May 26, 2025 at 06:29:46PM -0400, Tamir Duberstein wrote:
> On Mon, May 26, 2025 at 11:04 AM Benno Lossin <lossin@kernel.org> wrote:
> >
> > On Sat May 24, 2025 at 10:33 PM CEST, Tamir Duberstein wrote:
> > > +macro_rules! c_str_avoid_literals {
> >
> > I don't like this name, how about `concat_to_c_str` or
> > `concat_with_nul`?
> >
> > This macro also is useful from macros that have a normal string literal,
> > but can't turn it into a `c""` one.
>
> Uh, can you give an example? I'm not attached to the name.
I also think it should be renamed. Right now it sounds like it creates a
c string while avoiding literals in the input ... whatever that means. I
like Benno's suggestions, but str_to_cstr! could also work?
Alice
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH v10 4/5] rust: replace `kernel::c_str!` with C-Strings
2025-05-28 10:36 ` Alice Ryhl
@ 2025-05-28 15:34 ` Benno Lossin
2025-05-29 22:21 ` Tamir Duberstein
0 siblings, 1 reply; 34+ messages in thread
From: Benno Lossin @ 2025-05-28 15:34 UTC (permalink / raw)
To: Alice Ryhl, Tamir Duberstein
Cc: Michal Rostecki, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Trevor Gross,
Brendan Higgins, David Gow, Rae Moar, Danilo Krummrich,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Greg Kroah-Hartman, Rafael J. Wysocki,
Luis Chamberlain, Russ Weight, FUJITA Tomonori, Rob Herring,
Saravana Kannan, Peter Zijlstra, Ingo Molnar, Will Deacon,
Waiman Long, Nathan Chancellor, Nick Desaulniers, Bill Wendling,
Justin Stitt, Andrew Lunn, Heiner Kallweit, Russell King,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Bjorn Helgaas, Arnd Bergmann, Jens Axboe,
Krzysztof Wilczyński, rust-for-linux, linux-kernel,
linux-kselftest, kunit-dev, dri-devel, netdev, devicetree, llvm,
linux-pci, nouveau, linux-block
On Wed May 28, 2025 at 12:36 PM CEST, Alice Ryhl wrote:
> On Mon, May 26, 2025 at 06:29:46PM -0400, Tamir Duberstein wrote:
>> On Mon, May 26, 2025 at 11:04 AM Benno Lossin <lossin@kernel.org> wrote:
>> >
>> > On Sat May 24, 2025 at 10:33 PM CEST, Tamir Duberstein wrote:
>> > > +macro_rules! c_str_avoid_literals {
>> >
>> > I don't like this name, how about `concat_to_c_str` or
>> > `concat_with_nul`?
>> >
>> > This macro also is useful from macros that have a normal string literal,
>> > but can't turn it into a `c""` one.
>>
>> Uh, can you give an example? I'm not attached to the name.
>
> I also think it should be renamed. Right now it sounds like it creates a
> c string while avoiding literals in the input ... whatever that means.
Yeah that's a good way to put why the name is weird.
> I like Benno's suggestions, but str_to_cstr! could also work?
Hmm, I think then people won't know that it can also concat? I don't
think it matters too much, the macro probably won't be used that often
and if someone needs to use it, they probably wouldn't fine it by name
alone.
---
Cheers,
Benno
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH v10 4/5] rust: replace `kernel::c_str!` with C-Strings
2025-05-28 15:34 ` Benno Lossin
@ 2025-05-29 22:21 ` Tamir Duberstein
2025-05-29 22:28 ` Benno Lossin
0 siblings, 1 reply; 34+ messages in thread
From: Tamir Duberstein @ 2025-05-29 22:21 UTC (permalink / raw)
To: Benno Lossin
Cc: Alice Ryhl, Michal Rostecki, Miguel Ojeda, Alex Gaynor,
Boqun Feng, Gary Guo, Björn Roy Baron, Andreas Hindborg,
Trevor Gross, Brendan Higgins, David Gow, Rae Moar,
Danilo Krummrich, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter,
Greg Kroah-Hartman, Rafael J. Wysocki, Luis Chamberlain,
Russ Weight, FUJITA Tomonori, Rob Herring, Saravana Kannan,
Peter Zijlstra, Ingo Molnar, Will Deacon, Waiman Long,
Nathan Chancellor, Nick Desaulniers, Bill Wendling, Justin Stitt,
Andrew Lunn, Heiner Kallweit, Russell King, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Bjorn Helgaas,
Arnd Bergmann, Jens Axboe, Krzysztof Wilczyński,
rust-for-linux, linux-kernel, linux-kselftest, kunit-dev,
dri-devel, netdev, devicetree, llvm, linux-pci, nouveau,
linux-block
On Wed, May 28, 2025 at 11:35 AM Benno Lossin <lossin@kernel.org> wrote:
>
> On Wed May 28, 2025 at 12:36 PM CEST, Alice Ryhl wrote:
> > On Mon, May 26, 2025 at 06:29:46PM -0400, Tamir Duberstein wrote:
> >> On Mon, May 26, 2025 at 11:04 AM Benno Lossin <lossin@kernel.org> wrote:
> >> >
> >> > On Sat May 24, 2025 at 10:33 PM CEST, Tamir Duberstein wrote:
> >> > > +macro_rules! c_str_avoid_literals {
> >> >
> >> > I don't like this name, how about `concat_to_c_str` or
> >> > `concat_with_nul`?
> >> >
> >> > This macro also is useful from macros that have a normal string literal,
> >> > but can't turn it into a `c""` one.
> >>
> >> Uh, can you give an example? I'm not attached to the name.
> >
> > I also think it should be renamed. Right now it sounds like it creates a
> > c string while avoiding literals in the input ... whatever that means.
>
> Yeah that's a good way to put why the name is weird.
>
> > I like Benno's suggestions, but str_to_cstr! could also work?
>
> Hmm, I think then people won't know that it can also concat? I don't
> think it matters too much, the macro probably won't be used that often
> and if someone needs to use it, they probably wouldn't fine it by name
> alone.
What do you mean by "it can also concat"? This macro by itself doesn't
concat, it takes only a single expr. The example in the docs
illustrates:
const MY_CSTR: &CStr = c_str_avoid_literals!(concat!(...));
I think str_to_cstr is ok - I'll do that in v11.
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH v10 4/5] rust: replace `kernel::c_str!` with C-Strings
2025-05-29 22:21 ` Tamir Duberstein
@ 2025-05-29 22:28 ` Benno Lossin
0 siblings, 0 replies; 34+ messages in thread
From: Benno Lossin @ 2025-05-29 22:28 UTC (permalink / raw)
To: Tamir Duberstein
Cc: Alice Ryhl, Michal Rostecki, Miguel Ojeda, Alex Gaynor,
Boqun Feng, Gary Guo, Björn Roy Baron, Andreas Hindborg,
Trevor Gross, Brendan Higgins, David Gow, Rae Moar,
Danilo Krummrich, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter,
Greg Kroah-Hartman, Rafael J. Wysocki, Luis Chamberlain,
Russ Weight, FUJITA Tomonori, Rob Herring, Saravana Kannan,
Peter Zijlstra, Ingo Molnar, Will Deacon, Waiman Long,
Nathan Chancellor, Nick Desaulniers, Bill Wendling, Justin Stitt,
Andrew Lunn, Heiner Kallweit, Russell King, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Bjorn Helgaas,
Arnd Bergmann, Jens Axboe, Krzysztof Wilczyński,
rust-for-linux, linux-kernel, linux-kselftest, kunit-dev,
dri-devel, netdev, devicetree, llvm, linux-pci, nouveau,
linux-block
On Fri May 30, 2025 at 12:21 AM CEST, Tamir Duberstein wrote:
> On Wed, May 28, 2025 at 11:35 AM Benno Lossin <lossin@kernel.org> wrote:
>> On Wed May 28, 2025 at 12:36 PM CEST, Alice Ryhl wrote:
>> > On Mon, May 26, 2025 at 06:29:46PM -0400, Tamir Duberstein wrote:
>> >> On Mon, May 26, 2025 at 11:04 AM Benno Lossin <lossin@kernel.org> wrote:
>> >> >
>> >> > On Sat May 24, 2025 at 10:33 PM CEST, Tamir Duberstein wrote:
>> >> > > +macro_rules! c_str_avoid_literals {
>> >> >
>> >> > I don't like this name, how about `concat_to_c_str` or
>> >> > `concat_with_nul`?
>> >> >
>> >> > This macro also is useful from macros that have a normal string literal,
>> >> > but can't turn it into a `c""` one.
>> >>
>> >> Uh, can you give an example? I'm not attached to the name.
>> >
>> > I also think it should be renamed. Right now it sounds like it creates a
>> > c string while avoiding literals in the input ... whatever that means.
>>
>> Yeah that's a good way to put why the name is weird.
>>
>> > I like Benno's suggestions, but str_to_cstr! could also work?
>>
>> Hmm, I think then people won't know that it can also concat? I don't
>> think it matters too much, the macro probably won't be used that often
>> and if someone needs to use it, they probably wouldn't fine it by name
>> alone.
>
> What do you mean by "it can also concat"? This macro by itself doesn't
> concat, it takes only a single expr.
Oh right, seems like I thought it took `$($t:tt)*`...
> The example in the docs illustrates:
>
> const MY_CSTR: &CStr = c_str_avoid_literals!(concat!(...));
>
> I think str_to_cstr is ok - I'll do that in v11.
Sounds good!
---
Cheers,
Benno
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH v10 5/5] rust: remove core::ffi::CStr reexport
2025-05-24 20:33 [PATCH v10 0/5] rust: replace kernel::str::CStr w/ core::ffi::CStr Tamir Duberstein
` (3 preceding siblings ...)
2025-05-24 20:33 ` [PATCH v10 4/5] rust: replace `kernel::c_str!` with C-Strings Tamir Duberstein
@ 2025-05-24 20:33 ` Tamir Duberstein
2025-05-26 15:05 ` Benno Lossin
2025-05-28 10:38 ` [PATCH v10 0/5] rust: replace kernel::str::CStr w/ core::ffi::CStr Alice Ryhl
5 siblings, 1 reply; 34+ messages in thread
From: Tamir Duberstein @ 2025-05-24 20:33 UTC (permalink / raw)
To: Michal Rostecki, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Brendan Higgins, David Gow, Rae Moar, Danilo Krummrich,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Greg Kroah-Hartman, Rafael J. Wysocki,
Luis Chamberlain, Russ Weight, FUJITA Tomonori, Rob Herring,
Saravana Kannan, Peter Zijlstra, Ingo Molnar, Will Deacon,
Waiman Long, Nathan Chancellor, Nick Desaulniers, Bill Wendling,
Justin Stitt, Andrew Lunn, Heiner Kallweit, Russell King,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Bjorn Helgaas, Arnd Bergmann, Jens Axboe, Benno Lossin,
Krzysztof Wilczyński, Benno Lossin
Cc: rust-for-linux, linux-kernel, linux-kselftest, kunit-dev,
dri-devel, netdev, devicetree, llvm, linux-pci, nouveau,
linux-block, Tamir Duberstein
Clean up references to `kernel::str::CStr`.
Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
drivers/gpu/drm/drm_panic_qr.rs | 3 ++-
drivers/gpu/nova-core/firmware.rs | 2 +-
drivers/net/phy/ax88796b_rust.rs | 1 +
drivers/net/phy/qt2025.rs | 1 +
rust/kernel/device.rs | 3 +--
rust/kernel/driver.rs | 4 ++--
rust/kernel/error.rs | 6 ++----
rust/kernel/faux.rs | 5 ++++-
rust/kernel/firmware.rs | 15 ++++-----------
rust/kernel/kunit.rs | 6 +++---
rust/kernel/lib.rs | 2 +-
rust/kernel/miscdevice.rs | 3 +--
rust/kernel/net/phy.rs | 4 +++-
rust/kernel/of.rs | 3 ++-
rust/kernel/pci.rs | 2 +-
rust/kernel/platform.rs | 2 +-
rust/kernel/prelude.rs | 5 +----
rust/kernel/str.rs | 22 ++++++++++------------
rust/kernel/sync/condvar.rs | 4 ++--
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 +-
24 files changed, 51 insertions(+), 55 deletions(-)
diff --git a/drivers/gpu/drm/drm_panic_qr.rs b/drivers/gpu/drm/drm_panic_qr.rs
index d8192a9bef63..ba63238d352f 100644
--- a/drivers/gpu/drm/drm_panic_qr.rs
+++ b/drivers/gpu/drm/drm_panic_qr.rs
@@ -27,7 +27,8 @@
//! * <https://github.com/erwanvivien/fast_qr>
//! * <https://github.com/bjguillot/qr>
-use kernel::{prelude::*, str::CStr};
+use core::ffi::CStr;
+use kernel::prelude::*;
#[derive(Debug, Clone, Copy, PartialEq, Eq, Ord, PartialOrd)]
struct Version(usize);
diff --git a/drivers/gpu/nova-core/firmware.rs b/drivers/gpu/nova-core/firmware.rs
index 6e6361c59ca1..0af1f0df2fa5 100644
--- a/drivers/gpu/nova-core/firmware.rs
+++ b/drivers/gpu/nova-core/firmware.rs
@@ -30,7 +30,7 @@ const fn make_entry_chipset(self, chipset: &str) -> Self {
}
pub(crate) const fn create(
- module_name: &'static kernel::str::CStr,
+ module_name: &'static core::ffi::CStr,
) -> firmware::ModInfoBuilder<N> {
let mut this = Self(firmware::ModInfoBuilder::new(module_name));
let mut i = 0;
diff --git a/drivers/net/phy/ax88796b_rust.rs b/drivers/net/phy/ax88796b_rust.rs
index 2d24628a4e58..68b8e30ae296 100644
--- a/drivers/net/phy/ax88796b_rust.rs
+++ b/drivers/net/phy/ax88796b_rust.rs
@@ -4,6 +4,7 @@
//! Rust Asix PHYs driver
//!
//! C version of this driver: [`drivers/net/phy/ax88796b.c`](./ax88796b.c)
+use core::ffi::CStr;
use kernel::{
net::phy::{self, reg::C22, DeviceId, Driver},
prelude::*,
diff --git a/drivers/net/phy/qt2025.rs b/drivers/net/phy/qt2025.rs
index 9ccc75f70219..78ce2866f2b6 100644
--- a/drivers/net/phy/qt2025.rs
+++ b/drivers/net/phy/qt2025.rs
@@ -9,6 +9,7 @@
//!
//! The QT2025 PHY integrates an Intel 8051 micro-controller.
+use core::ffi::CStr;
use kernel::error::code;
use kernel::firmware::Firmware;
use kernel::net::phy::{
diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs
index 9074322c79e8..2cf5903f7dde 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 ec9166cedfa7..9926664d9ba2 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, of, str::CStr, try_pin_init, types::Opaque, ThisModule};
-use core::pin::Pin;
+use crate::{device, of, try_pin_init, types::Opaque, ThisModule};
+use core::{ffi::CStr, pin::Pin};
use pin_init::{pin_data, pinned_drop, PinInit};
/// 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 933c048c04f1..b2b46d26f7b7 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/faux.rs b/rust/kernel/faux.rs
index 8a50fcd4c9bb..d9e5cd265101 100644
--- a/rust/kernel/faux.rs
+++ b/rust/kernel/faux.rs
@@ -7,7 +7,10 @@
//! C header: [`include/linux/device/faux.h`]
use crate::{bindings, device, error::code::*, prelude::*};
-use core::ptr::{addr_of_mut, null, null_mut, NonNull};
+use core::{
+ ffi::CStr,
+ ptr::{addr_of_mut, null, null_mut, NonNull},
+};
/// The registration of a faux device.
///
diff --git a/rust/kernel/firmware.rs b/rust/kernel/firmware.rs
index 09fd3a27bcf0..4ba5e5589d7b 100644
--- a/rust/kernel/firmware.rs
+++ b/rust/kernel/firmware.rs
@@ -4,15 +4,8 @@
//!
//! C header: [`include/linux/firmware.h`](srctree/include/linux/firmware.h)
-use crate::{
- bindings,
- device::Device,
- error::Error,
- error::Result,
- ffi,
- str::{CStr, CStrExt as _},
-};
-use core::ptr::NonNull;
+use crate::{bindings, device::Device, error::Error, error::Result, ffi, str::CStrExt as _};
+use core::{ffi::CStr, ptr::NonNull};
/// # Invariants
///
@@ -168,7 +161,7 @@ unsafe impl Sync for Firmware {}
/// const DIR: &'static str = "vendor/chip/";
/// const FILES: [&'static str; 3] = [ "foo", "bar", "baz" ];
///
-/// const fn create(module_name: &'static kernel::str::CStr) -> firmware::ModInfoBuilder<N> {
+/// const fn create(module_name: &'static core::ffi::CStr) -> firmware::ModInfoBuilder<N> {
/// let mut builder = firmware::ModInfoBuilder::new(module_name);
///
/// let mut i = 0;
@@ -202,7 +195,7 @@ macro_rules! module_firmware {
// this macro. Hence, we can neither use `expr` nor `ty`.
($($builder:tt)*) => {
const _: () = {
- const __MODULE_FIRMWARE_PREFIX: &'static $crate::str::CStr = if cfg!(MODULE) {
+ const __MODULE_FIRMWARE_PREFIX: &'static ::core::ffi::CStr = if cfg!(MODULE) {
c""
} else {
<LocalModule as $crate::ModuleMetadata>::NAME
diff --git a/rust/kernel/kunit.rs b/rust/kernel/kunit.rs
index 09148e982f48..83b48e2f8379 100644
--- a/rust/kernel/kunit.rs
+++ b/rust/kernel/kunit.rs
@@ -58,9 +58,9 @@ macro_rules! kunit_assert {
break 'out;
}
- static FILE: &'static $crate::str::CStr = $crate::c_str_avoid_literals!($file);
+ 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.
@@ -171,7 +171,7 @@ macro_rules! kunit_assert_eq {
/// Use [`kunit_case_null`] to generate such a delimiter.
#[doc(hidden)]
pub const fn kunit_case(
- name: &'static kernel::str::CStr,
+ name: &'static core::ffi::CStr,
run_case: unsafe extern "C" fn(*mut kernel::bindings::kunit),
) -> kernel::bindings::kunit_case {
kernel::bindings::kunit_case {
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index ec48c818d512..d36b123c518b 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -152,7 +152,7 @@ fn init(module: &'static ThisModule) -> impl pin_init::PinInit<Self, error::Erro
/// 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 d684ec4ef4d0..47f718a9ceb5 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/net/phy.rs b/rust/kernel/net/phy.rs
index 8129419a3931..72635af8f20e 100644
--- a/rust/kernel/net/phy.rs
+++ b/rust/kernel/net/phy.rs
@@ -7,7 +7,7 @@
//! C headers: [`include/linux/phy.h`](srctree/include/linux/phy.h).
use crate::{error::*, prelude::*, types::Opaque};
-use core::{marker::PhantomData, ptr::addr_of_mut};
+use core::{ffi::CStr, marker::PhantomData, ptr::addr_of_mut};
pub mod reg;
@@ -780,6 +780,7 @@ const fn as_int(&self) -> u32 {
///
/// ```
/// # mod module_phy_driver_sample {
+/// use core::ffi::CStr;
/// use kernel::net::phy::{self, DeviceId};
/// use kernel::prelude::*;
///
@@ -807,6 +808,7 @@ const fn as_int(&self) -> u32 {
/// This expands to the following code:
///
/// ```ignore
+/// use core::ffi::CStr;
/// use kernel::net::phy::{self, DeviceId};
/// use kernel::prelude::*;
///
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 3aeb1250c27f..b639e6a1f590 100644
--- a/rust/kernel/pci.rs
+++ b/rust/kernel/pci.rs
@@ -13,11 +13,11 @@
error::{to_result, Result},
io::Io,
io::IoRaw,
- str::CStr,
types::{ARef, ForeignOwnable, Opaque},
ThisModule,
};
use core::{
+ ffi::CStr,
marker::PhantomData,
ops::Deref,
ptr::{addr_of_mut, NonNull},
diff --git a/rust/kernel/platform.rs b/rust/kernel/platform.rs
index 7163fc468b32..ba580a4e3416 100644
--- a/rust/kernel/platform.rs
+++ b/rust/kernel/platform.rs
@@ -9,12 +9,12 @@
error::{to_result, Result},
of,
prelude::*,
- str::CStr,
types::{ARef, ForeignOwnable, Opaque},
ThisModule,
};
use core::{
+ ffi::CStr,
marker::PhantomData,
ops::Deref,
ptr::{addr_of_mut, NonNull},
diff --git a/rust/kernel/prelude.rs b/rust/kernel/prelude.rs
index 97e8bcf73669..5d55e274b41e 100644
--- a/rust/kernel/prelude.rs
+++ b/rust/kernel/prelude.rs
@@ -35,10 +35,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;
diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
index 0385d927fcd5..cf0402d1daac 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, Index};
@@ -175,8 +176,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:
//
@@ -267,7 +266,6 @@ impl fmt::Display for crate::fmt::Adapter<&CStr> {
///
/// ```
/// # use kernel::prelude::fmt;
- /// # use kernel::str::CStr;
/// # use kernel::str::CString;
/// let penguin = c"🐧";
/// let s = CString::try_from_fmt(fmt!("{}", penguin))?;
@@ -377,8 +375,8 @@ fn as_ref(&self) -> &BStr {
/// # Examples
///
/// ```
+/// # use core::ffi::CStr;
/// # use kernel::c_str_avoid_literals;
-/// # use kernel::str::CStr;
/// const MY_CSTR: &CStr = c_str_avoid_literals!(concat!(file!(), ":", line!(), ": My CStr!"));
/// ```
#[macro_export]
@@ -388,14 +386,14 @@ 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(err) => {
- let _: core::ffi::FromBytesWithNulError = err;
- 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(err) => {
+ let _: core::ffi::FromBytesWithNulError = err;
+ panic!("string contains interior NUL")
+ }
+ };
C
}};
}
diff --git a/rust/kernel/sync/condvar.rs b/rust/kernel/sync/condvar.rs
index 0b6bc7f2878d..09bc35feb451 100644
--- a/rust/kernel/sync/condvar.rs
+++ b/rust/kernel/sync/condvar.rs
@@ -8,14 +8,14 @@
use super::{lock::Backend, lock::Guard, LockClassKey};
use crate::{
ffi::{c_int, c_long},
- str::{CStr, CStrExt as _},
+ str::CStrExt as _,
task::{
MAX_SCHEDULE_TIMEOUT, TASK_FREEZABLE, TASK_INTERRUPTIBLE, TASK_NORMAL, TASK_UNINTERRUPTIBLE,
},
time::Jiffies,
types::Opaque,
};
-use core::{marker::PhantomPinned, pin::Pin, ptr};
+use core::{ffi::CStr, marker::PhantomPinned, pin::Pin, ptr};
use pin_init::{pin_data, pin_init, PinInit};
/// Creates a [`CondVar`] initialiser with the given name and a newly-created lock class.
diff --git a/rust/kernel/sync/lock.rs b/rust/kernel/sync/lock.rs
index a777a22976e0..21deff0bb13b 100644
--- a/rust/kernel/sync/lock.rs
+++ b/rust/kernel/sync/lock.rs
@@ -7,10 +7,10 @@
use super::LockClassKey;
use crate::{
- str::{CStr, CStrExt as _},
+ str::CStrExt as _,
types::{NotThreadSafe, Opaque, ScopeGuard},
};
-use core::{cell::UnsafeCell, marker::PhantomPinned, pin::Pin};
+use core::{cell::UnsafeCell, ffi::CStr, marker::PhantomPinned, pin::Pin};
use pin_init::{pin_data, pin_init, PinInit};
pub mod mutex;
diff --git a/rust/kernel/sync/lock/global.rs b/rust/kernel/sync/lock/global.rs
index 0ca23b12427c..bfeaa21ab101 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},
pin::Pin,
};
@@ -267,7 +268,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 d7e6e59e124b..bf2fb24d04ea 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 f98bd02b838f..c64769ef5b90 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 7b71d6d99d5c..5246fed82e63 100644
--- a/rust/macros/module.rs
+++ b/rust/macros/module.rs
@@ -232,7 +232,7 @@ pub(crate) fn module(ts: TokenStream) -> TokenStream {
type LocalModule = {type_};
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.49.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [PATCH v10 5/5] rust: remove core::ffi::CStr reexport
2025-05-24 20:33 ` [PATCH v10 5/5] rust: remove core::ffi::CStr reexport Tamir Duberstein
@ 2025-05-26 15:05 ` Benno Lossin
2025-05-26 22:30 ` Tamir Duberstein
0 siblings, 1 reply; 34+ messages in thread
From: Benno Lossin @ 2025-05-26 15:05 UTC (permalink / raw)
To: Tamir Duberstein, Michal Rostecki, Miguel Ojeda, Alex Gaynor,
Boqun Feng, Gary Guo, Björn Roy Baron, Andreas Hindborg,
Alice Ryhl, Trevor Gross, Brendan Higgins, David Gow, Rae Moar,
Danilo Krummrich, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter,
Greg Kroah-Hartman, Rafael J. Wysocki, Luis Chamberlain,
Russ Weight, FUJITA Tomonori, Rob Herring, Saravana Kannan,
Peter Zijlstra, Ingo Molnar, Will Deacon, Waiman Long,
Nathan Chancellor, Nick Desaulniers, Bill Wendling, Justin Stitt,
Andrew Lunn, Heiner Kallweit, Russell King, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Bjorn Helgaas,
Arnd Bergmann, Jens Axboe, Krzysztof Wilczyński
Cc: rust-for-linux, linux-kernel, linux-kselftest, kunit-dev,
dri-devel, netdev, devicetree, llvm, linux-pci, nouveau,
linux-block
On Sat May 24, 2025 at 10:33 PM CEST, Tamir Duberstein wrote:
> Clean up references to `kernel::str::CStr`.
>
> Signed-off-by: Tamir Duberstein <tamird@gmail.com>
> ---
> drivers/gpu/drm/drm_panic_qr.rs | 3 ++-
> drivers/gpu/nova-core/firmware.rs | 2 +-
> drivers/net/phy/ax88796b_rust.rs | 1 +
> drivers/net/phy/qt2025.rs | 1 +
> rust/kernel/device.rs | 3 +--
> rust/kernel/driver.rs | 4 ++--
> rust/kernel/error.rs | 6 ++----
> rust/kernel/faux.rs | 5 ++++-
> rust/kernel/firmware.rs | 15 ++++-----------
> rust/kernel/kunit.rs | 6 +++---
> rust/kernel/lib.rs | 2 +-
> rust/kernel/miscdevice.rs | 3 +--
> rust/kernel/net/phy.rs | 4 +++-
> rust/kernel/of.rs | 3 ++-
> rust/kernel/pci.rs | 2 +-
> rust/kernel/platform.rs | 2 +-
> rust/kernel/prelude.rs | 5 +----
> rust/kernel/str.rs | 22 ++++++++++------------
> rust/kernel/sync/condvar.rs | 4 ++--
> 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 +-
> 24 files changed, 51 insertions(+), 55 deletions(-)
Haven't compile tested this series yet, but this commit seems to suggest
to me that some of the previous commits introduced code that doesn't
compile or emits warnings? If so that needs to be fixed.
---
Cheers,
Benno
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v10 5/5] rust: remove core::ffi::CStr reexport
2025-05-26 15:05 ` Benno Lossin
@ 2025-05-26 22:30 ` Tamir Duberstein
2025-05-26 23:17 ` Benno Lossin
0 siblings, 1 reply; 34+ messages in thread
From: Tamir Duberstein @ 2025-05-26 22:30 UTC (permalink / raw)
To: Benno Lossin
Cc: Michal Rostecki, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Brendan Higgins, David Gow, Rae Moar, Danilo Krummrich,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Greg Kroah-Hartman, Rafael J. Wysocki,
Luis Chamberlain, Russ Weight, FUJITA Tomonori, Rob Herring,
Saravana Kannan, Peter Zijlstra, Ingo Molnar, Will Deacon,
Waiman Long, Nathan Chancellor, Nick Desaulniers, Bill Wendling,
Justin Stitt, Andrew Lunn, Heiner Kallweit, Russell King,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Bjorn Helgaas, Arnd Bergmann, Jens Axboe,
Krzysztof Wilczyński, rust-for-linux, linux-kernel,
linux-kselftest, kunit-dev, dri-devel, netdev, devicetree, llvm,
linux-pci, nouveau, linux-block
On Mon, May 26, 2025 at 11:05 AM Benno Lossin <lossin@kernel.org> wrote:
>
> On Sat May 24, 2025 at 10:33 PM CEST, Tamir Duberstein wrote:
> > Clean up references to `kernel::str::CStr`.
> >
> > Signed-off-by: Tamir Duberstein <tamird@gmail.com>
> > ---
> > drivers/gpu/drm/drm_panic_qr.rs | 3 ++-
> > drivers/gpu/nova-core/firmware.rs | 2 +-
> > drivers/net/phy/ax88796b_rust.rs | 1 +
> > drivers/net/phy/qt2025.rs | 1 +
> > rust/kernel/device.rs | 3 +--
> > rust/kernel/driver.rs | 4 ++--
> > rust/kernel/error.rs | 6 ++----
> > rust/kernel/faux.rs | 5 ++++-
> > rust/kernel/firmware.rs | 15 ++++-----------
> > rust/kernel/kunit.rs | 6 +++---
> > rust/kernel/lib.rs | 2 +-
> > rust/kernel/miscdevice.rs | 3 +--
> > rust/kernel/net/phy.rs | 4 +++-
> > rust/kernel/of.rs | 3 ++-
> > rust/kernel/pci.rs | 2 +-
> > rust/kernel/platform.rs | 2 +-
> > rust/kernel/prelude.rs | 5 +----
> > rust/kernel/str.rs | 22 ++++++++++------------
> > rust/kernel/sync/condvar.rs | 4 ++--
> > 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 +-
> > 24 files changed, 51 insertions(+), 55 deletions(-)
>
> Haven't compile tested this series yet, but this commit seems to suggest
> to me that some of the previous commits introduced code that doesn't
> compile or emits warnings? If so that needs to be fixed.
That's not the case. There are no warnings and no compilation failures
in prior commits.
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v10 5/5] rust: remove core::ffi::CStr reexport
2025-05-26 22:30 ` Tamir Duberstein
@ 2025-05-26 23:17 ` Benno Lossin
0 siblings, 0 replies; 34+ messages in thread
From: Benno Lossin @ 2025-05-26 23:17 UTC (permalink / raw)
To: Tamir Duberstein
Cc: Michal Rostecki, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Brendan Higgins, David Gow, Rae Moar, Danilo Krummrich,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Greg Kroah-Hartman, Rafael J. Wysocki,
Luis Chamberlain, Russ Weight, FUJITA Tomonori, Rob Herring,
Saravana Kannan, Peter Zijlstra, Ingo Molnar, Will Deacon,
Waiman Long, Nathan Chancellor, Nick Desaulniers, Bill Wendling,
Justin Stitt, Andrew Lunn, Heiner Kallweit, Russell King,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Bjorn Helgaas, Arnd Bergmann, Jens Axboe,
Krzysztof Wilczyński, rust-for-linux, linux-kernel,
linux-kselftest, kunit-dev, dri-devel, netdev, devicetree, llvm,
linux-pci, nouveau, linux-block
On Tue May 27, 2025 at 12:30 AM CEST, Tamir Duberstein wrote:
> On Mon, May 26, 2025 at 11:05 AM Benno Lossin <lossin@kernel.org> wrote:
>>
>> On Sat May 24, 2025 at 10:33 PM CEST, Tamir Duberstein wrote:
>> > Clean up references to `kernel::str::CStr`.
>> >
>> > Signed-off-by: Tamir Duberstein <tamird@gmail.com>
>> > ---
>> > drivers/gpu/drm/drm_panic_qr.rs | 3 ++-
>> > drivers/gpu/nova-core/firmware.rs | 2 +-
>> > drivers/net/phy/ax88796b_rust.rs | 1 +
>> > drivers/net/phy/qt2025.rs | 1 +
>> > rust/kernel/device.rs | 3 +--
>> > rust/kernel/driver.rs | 4 ++--
>> > rust/kernel/error.rs | 6 ++----
>> > rust/kernel/faux.rs | 5 ++++-
>> > rust/kernel/firmware.rs | 15 ++++-----------
>> > rust/kernel/kunit.rs | 6 +++---
>> > rust/kernel/lib.rs | 2 +-
>> > rust/kernel/miscdevice.rs | 3 +--
>> > rust/kernel/net/phy.rs | 4 +++-
>> > rust/kernel/of.rs | 3 ++-
>> > rust/kernel/pci.rs | 2 +-
>> > rust/kernel/platform.rs | 2 +-
>> > rust/kernel/prelude.rs | 5 +----
>> > rust/kernel/str.rs | 22 ++++++++++------------
>> > rust/kernel/sync/condvar.rs | 4 ++--
>> > 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 +-
>> > 24 files changed, 51 insertions(+), 55 deletions(-)
>>
>> Haven't compile tested this series yet, but this commit seems to suggest
>> to me that some of the previous commits introduced code that doesn't
>> compile or emits warnings? If so that needs to be fixed.
>
> That's not the case. There are no warnings and no compilation failures
> in prior commits.
Ah it's because of the `pub use`... I tested it both with 1.86 and 1.78
and aside from the `use<>` error reported by the bot everything worked.
---
Cheers,
Benno
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v10 0/5] rust: replace kernel::str::CStr w/ core::ffi::CStr
2025-05-24 20:33 [PATCH v10 0/5] rust: replace kernel::str::CStr w/ core::ffi::CStr Tamir Duberstein
` (4 preceding siblings ...)
2025-05-24 20:33 ` [PATCH v10 5/5] rust: remove core::ffi::CStr reexport Tamir Duberstein
@ 2025-05-28 10:38 ` Alice Ryhl
2025-05-28 14:10 ` Tamir Duberstein
5 siblings, 1 reply; 34+ messages in thread
From: Alice Ryhl @ 2025-05-28 10:38 UTC (permalink / raw)
To: Tamir Duberstein
Cc: Michal Rostecki, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Trevor Gross,
Brendan Higgins, David Gow, Rae Moar, Danilo Krummrich,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Greg Kroah-Hartman, Rafael J. Wysocki,
Luis Chamberlain, Russ Weight, FUJITA Tomonori, Rob Herring,
Saravana Kannan, Peter Zijlstra, Ingo Molnar, Will Deacon,
Waiman Long, Nathan Chancellor, Nick Desaulniers, Bill Wendling,
Justin Stitt, Andrew Lunn, Heiner Kallweit, Russell King,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Bjorn Helgaas, Arnd Bergmann, Jens Axboe, Benno Lossin,
Krzysztof Wilczyński, rust-for-linux, linux-kernel,
linux-kselftest, kunit-dev, dri-devel, netdev, devicetree, llvm,
linux-pci, nouveau, linux-block
On Sat, May 24, 2025 at 04:33:00PM -0400, Tamir Duberstein 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>
Overall LGTM, thanks! Left a few comments on individual patches, but I
can probably give a RB when those a fixed. :)
Alice
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH v10 0/5] rust: replace kernel::str::CStr w/ core::ffi::CStr
2025-05-28 10:38 ` [PATCH v10 0/5] rust: replace kernel::str::CStr w/ core::ffi::CStr Alice Ryhl
@ 2025-05-28 14:10 ` Tamir Duberstein
0 siblings, 0 replies; 34+ messages in thread
From: Tamir Duberstein @ 2025-05-28 14:10 UTC (permalink / raw)
To: Alice Ryhl
Cc: Michal Rostecki, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Trevor Gross,
Brendan Higgins, David Gow, Rae Moar, Danilo Krummrich,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Greg Kroah-Hartman, Rafael J. Wysocki,
Luis Chamberlain, Russ Weight, FUJITA Tomonori, Rob Herring,
Saravana Kannan, Peter Zijlstra, Ingo Molnar, Will Deacon,
Waiman Long, Nathan Chancellor, Nick Desaulniers, Bill Wendling,
Justin Stitt, Andrew Lunn, Heiner Kallweit, Russell King,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Bjorn Helgaas, Arnd Bergmann, Jens Axboe, Benno Lossin,
Krzysztof Wilczyński, rust-for-linux, linux-kernel,
linux-kselftest, kunit-dev, dri-devel, netdev, devicetree, llvm,
linux-pci, nouveau, linux-block
On Wed, May 28, 2025 at 6:38 AM Alice Ryhl <aliceryhl@google.com> wrote:
>
> On Sat, May 24, 2025 at 04:33:00PM -0400, Tamir Duberstein 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>
>
> Overall LGTM, thanks! Left a few comments on individual patches, but I
> can probably give a RB when those a fixed. :)
Thanks for looking! You say a few comments, but I only saw one. Did
some get lost?
^ permalink raw reply [flat|nested] 34+ messages in thread