From: Tamir Duberstein <tamird@gmail.com>
To: "Michal Rostecki" <vadorovsky@protonmail.com>,
"Miguel Ojeda" <ojeda@kernel.org>,
"Alex Gaynor" <alex.gaynor@gmail.com>,
"Boqun Feng" <boqun.feng@gmail.com>,
"Gary Guo" <gary@garyguo.net>,
"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
"Benno Lossin" <benno.lossin@proton.me>,
"Andreas Hindborg" <a.hindborg@kernel.org>,
"Alice Ryhl" <aliceryhl@google.com>,
"Trevor Gross" <tmgross@umich.edu>
Cc: rust-for-linux@vger.kernel.org, linux-kernel@vger.kernel.org,
Tamir Duberstein <tamird@gmail.com>
Subject: [PATCH v6 1/4] rust: move BStr,CStr Display impls behind method
Date: Sun, 02 Feb 2025 07:20:46 -0500 [thread overview]
Message-ID: <20250202-cstr-core-v6-1-8469cd6d29fd@gmail.com> (raw)
In-Reply-To: <20250202-cstr-core-v6-0-8469cd6d29fd@gmail.com>
There are two reasons for doing this:
- Moving the Display impl behind a display method matches the style used
in the standard library for printing non-Unicode data.
- The standard library's core::ffi::CStr doesn't implement Display;
moving Display to a helper struct is a necessary step toward using it.
Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
rust/kernel/kunit.rs | 9 ++--
rust/kernel/str.rs | 120 +++++++++++++++++++++++++++++++++++----------------
2 files changed, 89 insertions(+), 40 deletions(-)
diff --git a/rust/kernel/kunit.rs b/rust/kernel/kunit.rs
index 824da0e9738a..630b947c708c 100644
--- a/rust/kernel/kunit.rs
+++ b/rust/kernel/kunit.rs
@@ -56,6 +56,7 @@ macro_rules! kunit_assert {
break 'out;
}
+ static NAME: &'static $crate::str::CStr = $crate::c_str!($name);
static FILE: &'static $crate::str::CStr = $crate::c_str!($file);
static LINE: i32 = core::line!() as i32 - $diff;
static CONDITION: &'static $crate::str::CStr = $crate::c_str!(stringify!($condition));
@@ -71,11 +72,13 @@ macro_rules! kunit_assert {
//
// This mimics KUnit's failed assertion format.
$crate::kunit::err(format_args!(
- " # {}: ASSERTION FAILED at {FILE}:{LINE}\n",
- $name
+ " # {NAME}: ASSERTION FAILED at {FILE}:{LINE}\n",
+ NAME = NAME.display(),
+ FILE = FILE.display(),
));
$crate::kunit::err(format_args!(
- " Expected {CONDITION} to be true, but is false\n"
+ " Expected {CONDITION} to be true, but is false\n",
+ CONDITION = CONDITION.display(),
));
$crate::kunit::err(format_args!(
" Failure not reported to KUnit since this is a non-KUnit task\n"
diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
index 28e2201604d6..53450c956a6a 100644
--- a/rust/kernel/str.rs
+++ b/rust/kernel/str.rs
@@ -31,29 +31,76 @@ pub const fn from_bytes(bytes: &[u8]) -> &Self {
// SAFETY: `BStr` is transparent to `[u8]`.
unsafe { &*(bytes as *const [u8] as *const BStr) }
}
-}
-impl fmt::Display for BStr {
- /// Formats printable ASCII characters, escaping the rest.
+ /// Returns an object that implements [`Display`] for safely printing a [`BStr`] that may
+ /// contain non-Unicode data. If you would like an implementation which escapes the [`BStr`]
+ /// please use [`Debug`] instead.
+ ///
+ /// [`Display`]: fmt::Display
+ /// [`Debug`]: fmt::Debug
+ ///
+ /// # Examples
///
/// ```
- /// # use kernel::{fmt, b_str, str::{BStr, CString}};
+ /// # use kernel::{fmt, b_str, str::CString};
/// let ascii = b_str!("Hello, BStr!");
- /// let s = CString::try_from_fmt(fmt!("{}", ascii))?;
+ /// let s = CString::try_from_fmt(fmt!("{}", ascii.display()))?;
/// assert_eq!(s.as_bytes(), "Hello, BStr!".as_bytes());
///
/// let non_ascii = b_str!("🦀");
- /// let s = CString::try_from_fmt(fmt!("{}", non_ascii))?;
+ /// let s = CString::try_from_fmt(fmt!("{}", non_ascii.display()))?;
/// assert_eq!(s.as_bytes(), "\\xf0\\x9f\\xa6\\x80".as_bytes());
/// # Ok::<(), kernel::error::Error>(())
/// ```
+ #[inline]
+ pub fn display(&self) -> Display<'_> {
+ Display {
+ inner: self,
+ escape_common: true,
+ }
+ }
+}
+
+/// Helper struct for safely printing a [`BStr`] with [`fmt!`] and `{}`.
+///
+/// A [`BStr`] might contain non-Unicode data. This `struct` implements the [`Display`] trait in a
+/// way that mitigates that. It is created by the [`display`](BStr::display) method on [`BStr`].
+///
+/// If you would like an implementation which escapes the string please use [`Debug`] instead.
+///
+/// # Examples
+///
+/// ```
+/// # use kernel::{fmt, b_str, str::CString};
+/// let ascii = b_str!("Hello, BStr!");
+/// let s = CString::try_from_fmt(fmt!("{}", ascii.display()))?;
+/// assert_eq!(s.as_bytes(), "Hello, BStr!".as_bytes());
+///
+/// let non_ascii = b_str!("🦀");
+/// let s = CString::try_from_fmt(fmt!("{}", non_ascii.display()))?;
+/// assert_eq!(s.as_bytes(), "\\xf0\\x9f\\xa6\\x80".as_bytes());
+/// # Ok::<(), kernel::error::Error>(())
+/// ```
+///
+/// [`Debug`]: fmt::Debug
+/// [`Display`]: fmt::Display
+pub struct Display<'a> {
+ inner: &'a BStr,
+ escape_common: bool,
+}
+
+impl fmt::Display for Display<'_> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
- for &b in &self.0 {
+ let Self {
+ inner: BStr(b),
+ escape_common,
+ } = self;
+ for &b in b {
match b {
// Common escape codes.
- b'\t' => f.write_str("\\t")?,
- b'\n' => f.write_str("\\n")?,
- b'\r' => f.write_str("\\r")?,
+ b'\t' if *escape_common => f.write_str("\\t")?,
+ b'\n' if *escape_common => f.write_str("\\n")?,
+ b'\r' if *escape_common => f.write_str("\\r")?,
// Printable characters.
0x20..=0x7e => f.write_char(b as char)?,
_ => write!(f, "\\x{:02x}", b)?,
@@ -68,7 +115,7 @@ impl fmt::Debug for BStr {
/// escaping the rest.
///
/// ```
- /// # use kernel::{fmt, b_str, str::{BStr, CString}};
+ /// # use kernel::{fmt, b_str, str::CString};
/// // Embedded double quotes are escaped.
/// let ascii = b_str!("Hello, \"BStr\"!");
/// let s = CString::try_from_fmt(fmt!("{:?}", ascii))?;
@@ -376,35 +423,35 @@ pub fn to_ascii_uppercase(&self) -> Result<CString, AllocError> {
Ok(s)
}
-}
-impl fmt::Display for CStr {
- /// Formats printable ASCII characters, escaping the rest.
+ /// Returns an object that implements [`Display`] for safely printing a [`CStr`] that may
+ /// contain non-Unicode data. If you would like an implementation which escapes the [`CStr`]
+ /// please use [`Debug`] instead.
+ ///
+ /// [`Display`]: fmt::Display
+ /// [`Debug`]: fmt::Debug
+ ///
+ /// # Examples
///
/// ```
/// # use kernel::c_str;
/// # use kernel::fmt;
- /// # use kernel::str::CStr;
/// # use kernel::str::CString;
/// let penguin = c_str!("🐧");
- /// let s = CString::try_from_fmt(fmt!("{}", penguin))?;
+ /// let s = CString::try_from_fmt(fmt!("{}", penguin.display()))?;
/// assert_eq!(s.as_bytes_with_nul(), "\\xf0\\x9f\\x90\\xa7\0".as_bytes());
///
/// let ascii = c_str!("so \"cool\"");
- /// let s = CString::try_from_fmt(fmt!("{}", ascii))?;
+ /// let s = CString::try_from_fmt(fmt!("{}", ascii.display()))?;
/// assert_eq!(s.as_bytes_with_nul(), "so \"cool\"\0".as_bytes());
/// # Ok::<(), kernel::error::Error>(())
/// ```
- fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
- for &c in self.as_bytes() {
- if (0x20..0x7f).contains(&c) {
- // Printable character.
- f.write_char(c as char)?;
- } else {
- write!(f, "\\x{:02x}", c)?;
- }
+ #[inline]
+ pub fn display(&self) -> Display<'_> {
+ Display {
+ inner: self,
+ escape_common: false,
}
- Ok(())
}
}
@@ -414,7 +461,6 @@ impl fmt::Debug for CStr {
/// ```
/// # use kernel::c_str;
/// # use kernel::fmt;
- /// # use kernel::str::CStr;
/// # use kernel::str::CString;
/// let penguin = c_str!("🐧");
/// let s = CString::try_from_fmt(fmt!("{:?}", penguin))?;
@@ -595,13 +641,13 @@ fn test_cstr_as_str_unchecked() {
#[test]
fn test_cstr_display() {
let hello_world = CStr::from_bytes_with_nul(b"hello, world!\0").unwrap();
- assert_eq!(format!("{}", hello_world), "hello, world!");
+ assert_eq!(format!("{}", hello_world.display()), "hello, world!");
let non_printables = CStr::from_bytes_with_nul(b"\x01\x09\x0a\0").unwrap();
- assert_eq!(format!("{}", non_printables), "\\x01\\x09\\x0a");
+ assert_eq!(format!("{}", non_printables.display()), "\\x01\\x09\\x0a");
let non_ascii = CStr::from_bytes_with_nul(b"d\xe9j\xe0 vu\0").unwrap();
- assert_eq!(format!("{}", non_ascii), "d\\xe9j\\xe0 vu");
+ assert_eq!(format!("{}", non_ascii.display()), "d\\xe9j\\xe0 vu");
let good_bytes = CStr::from_bytes_with_nul(b"\xf0\x9f\xa6\x80\0").unwrap();
- assert_eq!(format!("{}", good_bytes), "\\xf0\\x9f\\xa6\\x80");
+ assert_eq!(format!("{}", good_bytes.display()), "\\xf0\\x9f\\xa6\\x80");
}
#[test]
@@ -612,7 +658,7 @@ fn test_cstr_display_all_bytes() {
bytes[i as usize] = i.wrapping_add(1);
}
let cstr = CStr::from_bytes_with_nul(&bytes).unwrap();
- assert_eq!(format!("{}", cstr), ALL_ASCII_CHARS);
+ assert_eq!(format!("{}", cstr.display()), ALL_ASCII_CHARS);
}
#[test]
@@ -630,15 +676,15 @@ fn test_cstr_debug() {
#[test]
fn test_bstr_display() {
let hello_world = BStr::from_bytes(b"hello, world!");
- assert_eq!(format!("{}", hello_world), "hello, world!");
+ assert_eq!(format!("{}", hello_world.display()), "hello, world!");
let escapes = BStr::from_bytes(b"_\t_\n_\r_\\_\'_\"_");
- assert_eq!(format!("{}", escapes), "_\\t_\\n_\\r_\\_'_\"_");
+ assert_eq!(format!("{}", escapes.display()), "_\\t_\\n_\\r_\\_'_\"_");
let others = BStr::from_bytes(b"\x01");
- assert_eq!(format!("{}", others), "\\x01");
+ assert_eq!(format!("{}", others.display()), "\\x01");
let non_ascii = BStr::from_bytes(b"d\xe9j\xe0 vu");
- assert_eq!(format!("{}", non_ascii), "d\\xe9j\\xe0 vu");
+ assert_eq!(format!("{}", non_ascii.display()), "d\\xe9j\\xe0 vu");
let good_bytes = BStr::from_bytes(b"\xf0\x9f\xa6\x80");
- assert_eq!(format!("{}", good_bytes), "\\xf0\\x9f\\xa6\\x80");
+ assert_eq!(format!("{}", good_bytes.display()), "\\xf0\\x9f\\xa6\\x80");
}
#[test]
--
2.48.1
next prev parent reply other threads:[~2025-02-02 12:20 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-02-02 12:20 [PATCH v6 0/4] rust: replace kernel::str::CStr w/ core::ffi::CStr Tamir Duberstein
2025-02-02 12:20 ` Tamir Duberstein [this message]
2025-02-02 12:20 ` [PATCH v6 2/4] rust: replace `CStr` with `core::ffi::CStr` Tamir Duberstein
2025-02-02 12:54 ` Tamir Duberstein
2025-02-02 14:40 ` Tamir Duberstein
2025-02-02 12:20 ` [PATCH v6 3/4] rust: replace `kernel::c_str!` with C-Strings Tamir Duberstein
2025-02-02 13:10 ` Dirk Behme
2025-02-02 14:18 ` Tamir Duberstein
2025-02-02 12:20 ` [PATCH v6 4/4] rust: remove core::ffi::CStr reexport Tamir Duberstein
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20250202-cstr-core-v6-1-8469cd6d29fd@gmail.com \
--to=tamird@gmail.com \
--cc=a.hindborg@kernel.org \
--cc=alex.gaynor@gmail.com \
--cc=aliceryhl@google.com \
--cc=benno.lossin@proton.me \
--cc=bjorn3_gh@protonmail.com \
--cc=boqun.feng@gmail.com \
--cc=gary@garyguo.net \
--cc=linux-kernel@vger.kernel.org \
--cc=ojeda@kernel.org \
--cc=rust-for-linux@vger.kernel.org \
--cc=tmgross@umich.edu \
--cc=vadorovsky@protonmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.