[PATCH v13 2/5] rust: support formatting of foreign types
Benno Lossin
lossin at kernel.org
Thu Jul 3 09:32:05 UTC 2025
On Tue Jul 1, 2025 at 6:49 PM CEST, Tamir Duberstein wrote:
> Introduce a `fmt!` macro which wraps all arguments in
> `kernel::fmt::Adapter` and a `kernel::fmt::Display` trait. This enables
> formatting of foreign types (like `core::ffi::CStr`) that do not
> implement `core::fmt::Display` due to concerns around lossy conversions which
> do not apply in the kernel.
>
> Replace all direct calls to `format_args!` with `fmt!`.
>
> Replace all implementations of `core::fmt::Display` with implementations
> of `kernel::fmt::Display`.
>
> Suggested-by: Alice Ryhl <aliceryhl at google.com>
> Link: https://rust-for-linux.zulipchat.com/#narrow/channel/288089-General/topic/Custom.20formatting/with/516476467
> Acked-by: Greg Kroah-Hartman <gregkh at linuxfoundation.org>
> Reviewed-by: Alice Ryhl <aliceryhl at google.com>
> Signed-off-by: Tamir Duberstein <tamird at gmail.com>
> ---
> drivers/block/rnull.rs | 2 +-
> drivers/gpu/nova-core/gpu.rs | 4 +-
> rust/kernel/block/mq.rs | 2 +-
> rust/kernel/device.rs | 2 +-
> rust/kernel/fmt.rs | 89 +++++++++++++++++++++++++++++++++++++++
> 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 | 22 ++++------
> rust/macros/fmt.rs | 99 ++++++++++++++++++++++++++++++++++++++++++++
> rust/macros/lib.rs | 19 +++++++++
> rust/macros/quote.rs | 7 ++++
> scripts/rustdoc_test_gen.rs | 2 +-
> 15 files changed, 236 insertions(+), 28 deletions(-)
This would be a lot easier to review if he proc-macro and the call
replacement were different patches.
Also the `kernel/fmt.rs` file should be a different commit.
> diff --git a/rust/kernel/fmt.rs b/rust/kernel/fmt.rs
> new file mode 100644
> index 000000000000..348d16987de6
> --- /dev/null
> +++ b/rust/kernel/fmt.rs
> @@ -0,0 +1,89 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Formatting utilities.
> +
> +use core::fmt;
I think we should pub export all types that we are still using from
`core::fmt`. For example `Result`, `Formatter`, `Debug` etc.
That way I can still use the same pattern of importing `fmt` and then
writing
impl fmt::Display for MyType {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {}
}
> +
> +/// 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);
> +
> +/// A copy of [`fmt::Display`] that allows us to implement it for foreign types.
> +///
> +/// Types should implement this trait rather than [`fmt::Display`]. Together with the [`Adapter`]
> +/// type and [`fmt!`] macro, it allows for formatting foreign types (e.g. types from core) which do
> +/// not implement [`fmt::Display`] directly.
> +///
> +/// [`fmt!`]: crate::prelude::fmt!
> +pub trait Display {
> + /// Same as [`fmt::Display::fmt`].
> + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result;
> +}
> +
> +impl<T: ?Sized + Display> Display for &T {
> + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
> + Display::fmt(*self, f)
> + }
> +}
> +
> +impl<T: ?Sized + Display> fmt::Display for Adapter<&T> {
> + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
> + let Self(t) = self;
> + Display::fmt(t, f)
Why not `Display::fmt(&self.0, f)`?
> + }
> +}
> +
> +macro_rules! impl_display_forward {
> + ($(
> + $( { $($generics:tt)* } )? $ty:ty $( { where $($where:tt)* } )?
> + ),* $(,)?) => {
> + $(
> + impl$($($generics)*)? Display for $ty $(where $($where)*)? {
> + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
> + fmt::Display::fmt(self, f)
> + }
> + }
> + )*
> + };
> +}
> +
> +impl_display_forward!(
> + bool,
> + char,
> + core::panic::PanicInfo<'_>,
> + 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/macros/fmt.rs b/rust/macros/fmt.rs
> new file mode 100644
> index 000000000000..edc37c220a89
> --- /dev/null
> +++ b/rust/macros/fmt.rs
> @@ -0,0 +1,99 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +use proc_macro::{Ident, 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))
You're only using first_lit to get the span later, so why not just get
the span directly here?
> + })
> + }
> + _ => None,
> + }) else {
> + return first_opt.into_iter().chain(input).collect();
> + };
> + while let Some((_, rest)) = first_str.split_once('{') {
Let's put a comment above this loop mentioning [1] and saying that it
parses the identifiers from the format arguments.
[1]: https://doc.rust-lang.org/std/fmt/index.html#syntax
> + first_str = rest;
> + if let Some(rest) = first_str.strip_prefix('{') {
> + first_str = rest;
> + continue;
> + }
> + if let Some((name, rest)) = first_str.split_once('}') {
> + first_str = rest;
> + 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);
> + }
> + }
> + }
> + first_lit
> + };
> +
> + let first_span = first_lit.span();
> + let adapter = quote_spanned! {
> + first_span => ::kernel::fmt::Adapter
> + };
I think we should follow the formatting convention from the quote crate:
let adapter = quote_spanned!(first_span=> ::kernel::fmt::Adapter);
> +
> + let mut args = TokenStream::from_iter(first_opt);
> + {
> + let mut flush = |args: &mut TokenStream, current: &mut TokenStream| {
You don't need to pass `args` as a closure argument, since you always
call it with `&mut args`.
> + let current = std::mem::take(current);
> + if !current.is_empty() {
> + let (lhs, rhs) = (|| {
> + let mut current = current.into_iter();
> + let mut acc = TokenStream::new();
> + while let Some(tt) = current.next() {
> + // Split on `=` only once to handle cases like `a = b = c`.
> + if matches!(&tt, TokenTree::Punct(p) if p.as_char() == '=') {
> + names.remove(acc.to_string().as_str());
> + // Include the `=` itself to keep the handling below uniform.
> + acc.extend([tt]);
> + return (Some(acc), current.collect::<TokenStream>());
> + }
> + acc.extend([tt]);
> + }
> + (None, acc)
> + })();
> + args.extend(quote_spanned! {
> + first_span => #lhs #adapter(&#rhs)
> + });
> + }
> + };
> +
> + let mut current = TokenStream::new();
Define this before the closure, then you don't need to pass it as an
argument.
---
Cheers,
Benno
> + for tt in input {
> + match &tt {
> + TokenTree::Punct(p) if p.as_char() == ',' => {
> + flush(&mut args, &mut current);
> + &mut args
> + }
> + _ => &mut current,
> + }
> + .extend([tt]);
> + }
> + flush(&mut args, &mut current);
> + }
> +
> + for name in names {
> + let name = Ident::new(name, first_span);
> + args.extend(quote_spanned! {
> + first_span => , #name = #adapter(&#name)
> + });
> + }
> +
> + quote_spanned! {
> + first_span => ::core::format_args!(#args)
> + }
> +}
More information about the dri-devel
mailing list