rust-for-linux.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] rust: str: implement `Display` and `Debug` for `BStr`
@ 2024-01-24 16:58 Yutaro Ohno
  2024-01-24 20:18 ` Gary Guo
  0 siblings, 1 reply; 3+ messages in thread
From: Yutaro Ohno @ 2024-01-24 16:58 UTC (permalink / raw)
  To: Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Boqun Feng,
	Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
	Alice Ryhl
  Cc: rust-for-linux, Yutaro Ohno, Virgile Andreani

Currently, `BStr` is just a type alias of `[u8]`, limiting its
representation to a byte list rather than a character list, which is not
ideal for printing and debugging.

Implement `Display` and `Debug` traits for `BStr` to facilitate easier
printing and debugging.

Also, for this purpose, change `BStr` from a type alias of `[u8]` to a
struct wrapper of `[u8]`.

Co-developed-by: Virgile Andreani <armavica@ulminfo.fr>
Signed-off-by: Virgile Andreani <armavica@ulminfo.fr>
Signed-off-by: Yutaro Ohno <yutaro.ono.418@gmail.com>
---
 rust/kernel/str.rs | 211 +++++++++++++++++++++++++++++++++++++++------
 1 file changed, 186 insertions(+), 25 deletions(-)

diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
index 7d848b83add4..0f0261e063d2 100644
--- a/rust/kernel/str.rs
+++ b/rust/kernel/str.rs
@@ -14,8 +14,104 @@
 
 /// Byte string without UTF-8 validity guarantee.
 ///
-/// `BStr` is simply an alias to `[u8]`, but has a more evident semantical meaning.
-pub type BStr = [u8];
+/// `BStr` is simply a wrapper over `[u8]`, but has a more evident semantical
+/// meaning.
+#[repr(transparent)]
+pub struct BStr([u8]);
+
+impl BStr {
+    /// Returns the length of this string.
+    #[inline]
+    pub const fn len(&self) -> usize {
+        self.0.len()
+    }
+
+    /// Returns `true` if the string is empty.
+    #[inline]
+    pub const fn is_empty(&self) -> bool {
+        self.len() == 0
+    }
+
+    /// Creates a [`BStr`] from a `[u8]`.
+    #[inline]
+    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.
+    ///
+    /// ```
+    /// # use kernel::{fmt, b_str, str::{BStr, CString}};
+    /// let ascii = b_str!("Hello, BStr!");
+    /// let s = CString::try_from_fmt(fmt!("{}", ascii)).unwrap();
+    /// assert_eq!(s.as_bytes(), "Hello, BStr!".as_bytes());
+    ///
+    /// let non_ascii = b_str!("🦀");
+    /// let s = CString::try_from_fmt(fmt!("{}", non_ascii)).unwrap();
+    /// assert_eq!(s.as_bytes(), "\\xf0\\x9f\\xa6\\x80".as_bytes());
+    /// ```
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        for &b in &self.0 {
+            match b {
+                // Common escape codes.
+                b'\t' => f.write_str("\\t")?,
+                b'\n' => f.write_str("\\n")?,
+                b'\r' => f.write_str("\\r")?,
+                // Printable characters.
+                0x20..=0x7e => f.write_char(b as char)?,
+                _ => write!(f, "\\x{:02x}", b)?,
+            }
+        }
+        Ok(())
+    }
+}
+
+impl fmt::Debug for BStr {
+    /// Formats printable ASCII characters with a double quote on either end,
+    /// escaping the rest.
+    ///
+    /// ```
+    /// # use kernel::{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)).unwrap();
+    /// assert_eq!(s.as_bytes(), "\"Hello, \\\"BStr\\\"!\"".as_bytes());
+    ///
+    /// let non_ascii = b_str!("😺");
+    /// let s = CString::try_from_fmt(fmt!("{:?}", non_ascii)).unwrap();
+    /// assert_eq!(s.as_bytes(), "\"\\xf0\\x9f\\x98\\xba\"".as_bytes());
+    /// ```
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        f.write_str("\"")?;
+        for &b in &self.0 {
+            match b {
+                // Common escape codes.
+                b'\t' => f.write_str("\\t")?,
+                b'\n' => f.write_str("\\n")?,
+                b'\r' => f.write_str("\\r")?,
+                // String escape characters.
+                b'\\' => f.write_str("\\\\")?,
+                b'\"' => f.write_str("\\\"")?,
+                // Printable characters.
+                0x20..=0x7e => f.write_char(b as char)?,
+                _ => write!(f, "\\x{:02x}", b)?,
+            }
+        }
+        f.write_str("\"")
+    }
+}
+
+impl Deref for BStr {
+    type Target = [u8];
+
+    #[inline]
+    fn deref(&self) -> &Self::Target {
+        &self.0
+    }
+}
 
 /// Creates a new [`BStr`] from a string literal.
 ///
@@ -33,7 +129,7 @@
 macro_rules! b_str {
     ($str:literal) => {{
         const S: &'static str = $str;
-        const C: &'static $crate::str::BStr = S.as_bytes();
+        const C: &'static $crate::str::BStr = BStr::from_bytes(S.as_bytes());
         C
     }};
 }
@@ -225,15 +321,7 @@ impl fmt::Display for CStr {
     /// assert_eq!(s.as_bytes_with_nul(), "so \"cool\"\0".as_bytes());
     /// ```
     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)?;
-            }
-        }
-        Ok(())
+        fmt::Display::fmt(self.as_ref(), f)
     }
 }
 
@@ -255,23 +343,14 @@ impl fmt::Debug for CStr {
     /// assert_eq!(s.as_bytes_with_nul(), "\"so \\\"cool\\\"\"\0".as_bytes());
     /// ```
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        f.write_str("\"")?;
-        for &c in self.as_bytes() {
-            match c {
-                // Printable characters.
-                b'\"' => f.write_str("\\\"")?,
-                0x20..=0x7e => f.write_char(c as char)?,
-                _ => write!(f, "\\x{:02x}", c)?,
-            }
-        }
-        f.write_str("\"")
+        fmt::Debug::fmt(self.as_ref(), f)
     }
 }
 
 impl AsRef<BStr> for CStr {
     #[inline]
     fn as_ref(&self) -> &BStr {
-        self.as_bytes()
+        BStr::from_bytes(self.as_bytes())
     }
 }
 
@@ -280,7 +359,7 @@ impl Deref for CStr {
 
     #[inline]
     fn deref(&self) -> &Self::Target {
-        self.as_bytes()
+        BStr::from_bytes(self.as_bytes())
     }
 }
 
@@ -327,7 +406,7 @@ impl<Idx> Index<Idx> for CStr
 
     #[inline]
     fn index(&self, index: Idx) -> &Self::Output {
-        &self.as_bytes()[index]
+        &self.as_ref()[index]
     }
 }
 
@@ -357,6 +436,21 @@ macro_rules! c_str {
 #[cfg(test)]
 mod tests {
     use super::*;
+    use alloc::format;
+
+    const ALL_ASCII_CHARS: &'static str =
+        "\\x01\\x02\\x03\\x04\\x05\\x06\\x07\\x08\\t\\n\\x0b\\x0c\\r\\x0e\\x0f\
+        \\x10\\x11\\x12\\x13\\x14\\x15\\x16\\x17\\x18\\x19\\x1a\\x1b\\x1c\\x1d\\x1e\\x1f \
+        !\"#$%&'()*+,-./0123456789:;<=>?@\
+        ABCDEFGHIJKLMNOPQRSTUVWXYZ[\\]^_`abcdefghijklmnopqrstuvwxyz{|}~\\x7f\
+        \\x80\\x81\\x82\\x83\\x84\\x85\\x86\\x87\\x88\\x89\\x8a\\x8b\\x8c\\x8d\\x8e\\x8f\
+        \\x90\\x91\\x92\\x93\\x94\\x95\\x96\\x97\\x98\\x99\\x9a\\x9b\\x9c\\x9d\\x9e\\x9f\
+        \\xa0\\xa1\\xa2\\xa3\\xa4\\xa5\\xa6\\xa7\\xa8\\xa9\\xaa\\xab\\xac\\xad\\xae\\xaf\
+        \\xb0\\xb1\\xb2\\xb3\\xb4\\xb5\\xb6\\xb7\\xb8\\xb9\\xba\\xbb\\xbc\\xbd\\xbe\\xbf\
+        \\xc0\\xc1\\xc2\\xc3\\xc4\\xc5\\xc6\\xc7\\xc8\\xc9\\xca\\xcb\\xcc\\xcd\\xce\\xcf\
+        \\xd0\\xd1\\xd2\\xd3\\xd4\\xd5\\xd6\\xd7\\xd8\\xd9\\xda\\xdb\\xdc\\xdd\\xde\\xdf\
+        \\xe0\\xe1\\xe2\\xe3\\xe4\\xe5\\xe6\\xe7\\xe8\\xe9\\xea\\xeb\\xec\\xed\\xee\\xef\
+        \\xf0\\xf1\\xf2\\xf3\\xf4\\xf5\\xf6\\xf7\\xf8\\xf9\\xfa\\xfb\\xfc\\xfd\\xfe\\xff";
 
     #[test]
     fn test_cstr_to_str() {
@@ -381,6 +475,73 @@ fn test_cstr_as_str_unchecked() {
         let unchecked_str = unsafe { checked_cstr.as_str_unchecked() };
         assert_eq!(unchecked_str, "🐧");
     }
+
+    #[test]
+    fn test_cstr_display() {
+        let hello_world = CStr::from_bytes_with_nul(b"hello, world!\0").unwrap();
+        assert_eq!(format!("{}", hello_world), "hello, world!");
+        let escapes = CStr::from_bytes_with_nul(b"_\t_\n_\r_\\_\'_\"_\0").unwrap();
+        assert_eq!(format!("{}", escapes), "_\\t_\\n_\\r_\\_'_\"_");
+        let others = CStr::from_bytes_with_nul(b"\x01\0").unwrap();
+        assert_eq!(format!("{}", others), "\\x01");
+        let non_ascii = CStr::from_bytes_with_nul(b"d\xe9j\xe0 vu\0").unwrap();
+        assert_eq!(format!("{}", non_ascii), "d\\xe9j\\xe0 vu");
+        let good_bytes = CStr::from_bytes_with_nul(b"\xf0\x9f\xa6\x80\0").unwrap();
+        assert_eq!(format!("{}", good_bytes), "\\xf0\\x9f\\xa6\\x80");
+    }
+
+    #[test]
+    fn test_cstr_debug() {
+        let hello_world = CStr::from_bytes_with_nul(b"hello, world!\0").unwrap();
+        assert_eq!(format!("{:?}", hello_world), "\"hello, world!\"");
+        let escapes = CStr::from_bytes_with_nul(b"_\t_\n_\r_\\_\'_\"_\0").unwrap();
+        assert_eq!(format!("{:?}", escapes), "\"_\\t_\\n_\\r_\\\\_'_\\\"_\"");
+        let others = CStr::from_bytes_with_nul(b"\x01\0").unwrap();
+        assert_eq!(format!("{:?}", others), "\"\\x01\"");
+        let non_ascii = CStr::from_bytes_with_nul(b"d\xe9j\xe0 vu\0").unwrap();
+        assert_eq!(format!("{:?}", non_ascii), "\"d\\xe9j\\xe0 vu\"");
+        let good_bytes = CStr::from_bytes_with_nul(b"\xf0\x9f\xa6\x80\0").unwrap();
+        assert_eq!(format!("{:?}", good_bytes), "\"\\xf0\\x9f\\xa6\\x80\"");
+    }
+
+    #[test]
+    fn test_bstr_display() {
+        let hello_world = BStr::from_bytes(b"hello, world!");
+        assert_eq!(format!("{}", hello_world), "hello, world!");
+        let escapes = BStr::from_bytes(b"_\t_\n_\r_\\_\'_\"_");
+        assert_eq!(format!("{}", escapes), "_\\t_\\n_\\r_\\_'_\"_");
+        let others = BStr::from_bytes(b"\x01");
+        assert_eq!(format!("{}", others), "\\x01");
+        let non_ascii = BStr::from_bytes(b"d\xe9j\xe0 vu");
+        assert_eq!(format!("{}", non_ascii), "d\\xe9j\\xe0 vu");
+        let good_bytes = BStr::from_bytes(b"\xf0\x9f\xa6\x80");
+        assert_eq!(format!("{}", good_bytes), "\\xf0\\x9f\\xa6\\x80");
+    }
+
+    #[test]
+    fn test_bstr_debug() {
+        let hello_world = BStr::from_bytes(b"hello, world!");
+        assert_eq!(format!("{:?}", hello_world), "\"hello, world!\"");
+        let escapes = BStr::from_bytes(b"_\t_\n_\r_\\_\'_\"_");
+        assert_eq!(format!("{:?}", escapes), "\"_\\t_\\n_\\r_\\\\_'_\\\"_\"");
+        let others = BStr::from_bytes(b"\x01");
+        assert_eq!(format!("{:?}", others), "\"\\x01\"");
+        let non_ascii = BStr::from_bytes(b"d\xe9j\xe0 vu");
+        assert_eq!(format!("{:?}", non_ascii), "\"d\\xe9j\\xe0 vu\"");
+        let good_bytes = BStr::from_bytes(b"\xf0\x9f\xa6\x80");
+        assert_eq!(format!("{:?}", good_bytes), "\"\\xf0\\x9f\\xa6\\x80\"");
+    }
+
+    #[test]
+    fn test_cstr_display_all_bytes() {
+        let mut bytes: [u8; 256] = [0; 256];
+        // fill `bytes` with [1..=255] + [0]
+        for i in u8::MIN..=u8::MAX {
+            bytes[i as usize] = i.wrapping_add(1);
+        }
+        let cstr = CStr::from_bytes_with_nul(&bytes).unwrap();
+        assert_eq!(format!("{}", cstr), ALL_ASCII_CHARS);
+    }
 }
 
 /// Allows formatting of [`fmt::Arguments`] into a raw buffer.
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] rust: str: implement `Display` and `Debug` for `BStr`
  2024-01-24 16:58 [PATCH] rust: str: implement `Display` and `Debug` for `BStr` Yutaro Ohno
@ 2024-01-24 20:18 ` Gary Guo
  2024-01-25 13:25   ` Yutaro Ohno
  0 siblings, 1 reply; 3+ messages in thread
From: Gary Guo @ 2024-01-24 20:18 UTC (permalink / raw)
  To: Yutaro Ohno
  Cc: Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Boqun Feng,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	rust-for-linux, Virgile Andreani

On Thu, 25 Jan 2024 01:58:05 +0900
Yutaro Ohno <yutaro.ono.418@gmail.com> wrote:

> Currently, `BStr` is just a type alias of `[u8]`, limiting its
> representation to a byte list rather than a character list, which is not
> ideal for printing and debugging.
> 
> Implement `Display` and `Debug` traits for `BStr` to facilitate easier
> printing and debugging.
> 
> Also, for this purpose, change `BStr` from a type alias of `[u8]` to a
> struct wrapper of `[u8]`.
> 
> Co-developed-by: Virgile Andreani <armavica@ulminfo.fr>
> Signed-off-by: Virgile Andreani <armavica@ulminfo.fr>
> Signed-off-by: Yutaro Ohno <yutaro.ono.418@gmail.com>
> ---
>  rust/kernel/str.rs | 211 +++++++++++++++++++++++++++++++++++++++------
>  1 file changed, 186 insertions(+), 25 deletions(-)
> 
> diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
> index 7d848b83add4..0f0261e063d2 100644
> --- a/rust/kernel/str.rs
> +++ b/rust/kernel/str.rs
> @@ -14,8 +14,104 @@
>  
>  /// Byte string without UTF-8 validity guarantee.
>  ///
> -/// `BStr` is simply an alias to `[u8]`, but has a more evident semantical meaning.
> -pub type BStr = [u8];
> +/// `BStr` is simply a wrapper over `[u8]`, but has a more evident semantical
> +/// meaning.

I think this line can go?

> +#[repr(transparent)]
> +pub struct BStr([u8]);
> +
> +impl BStr {
> +    /// Returns the length of this string.
> +    #[inline]
> +    pub const fn len(&self) -> usize {
> +        self.0.len()
> +    }
> +
> +    /// Returns `true` if the string is empty.
> +    #[inline]
> +    pub const fn is_empty(&self) -> bool {
> +        self.len() == 0
> +    }
> +
> +    /// Creates a [`BStr`] from a `[u8]`.
> +    #[inline]
> +    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.
> +    ///
> +    /// ```
> +    /// # use kernel::{fmt, b_str, str::{BStr, CString}};
> +    /// let ascii = b_str!("Hello, BStr!");
> +    /// let s = CString::try_from_fmt(fmt!("{}", ascii)).unwrap();
> +    /// assert_eq!(s.as_bytes(), "Hello, BStr!".as_bytes());
> +    ///
> +    /// let non_ascii = b_str!("🦀");
> +    /// let s = CString::try_from_fmt(fmt!("{}", non_ascii)).unwrap();
> +    /// assert_eq!(s.as_bytes(), "\\xf0\\x9f\\xa6\\x80".as_bytes());
> +    /// ```
> +    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
> +        for &b in &self.0 {
> +            match b {
> +                // Common escape codes.
> +                b'\t' => f.write_str("\\t")?,
> +                b'\n' => f.write_str("\\n")?,
> +                b'\r' => f.write_str("\\r")?,

The current CStr code will print these as is. Why escaping these for
Display? Also, if you want to change the behaviour, please put it in a
separate patch.

> +                // Printable characters.
> +                0x20..=0x7e => f.write_char(b as char)?,
> +                _ => write!(f, "\\x{:02x}", b)?,
> +            }
> +        }
> +        Ok(())
> +    }
> +}
> +
> +impl fmt::Debug for BStr {
> +    /// Formats printable ASCII characters with a double quote on either end,
> +    /// escaping the rest.
> +    ///
> +    /// ```
> +    /// # use kernel::{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)).unwrap();
> +    /// assert_eq!(s.as_bytes(), "\"Hello, \\\"BStr\\\"!\"".as_bytes());
> +    ///
> +    /// let non_ascii = b_str!("😺");
> +    /// let s = CString::try_from_fmt(fmt!("{:?}", non_ascii)).unwrap();
> +    /// assert_eq!(s.as_bytes(), "\"\\xf0\\x9f\\x98\\xba\"".as_bytes());
> +    /// ```
> +    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
> +        f.write_str("\"")?;
> +        for &b in &self.0 {
> +            match b {
> +                // Common escape codes.
> +                b'\t' => f.write_str("\\t")?,
> +                b'\n' => f.write_str("\\n")?,
> +                b'\r' => f.write_str("\\r")?,
> +                // String escape characters.
> +                b'\\' => f.write_str("\\\\")?,
> +                b'\"' => f.write_str("\\\"")?,
> +                // Printable characters.
> +                0x20..=0x7e => f.write_char(b as char)?,
> +                _ => write!(f, "\\x{:02x}", b)?,
> +            }
> +        }
> +        f.write_str("\"")
> +    }
> +}
> +
> +impl Deref for BStr {
> +    type Target = [u8];
> +
> +    #[inline]
> +    fn deref(&self) -> &Self::Target {
> +        &self.0
> +    }
> +}
>  
>  /// Creates a new [`BStr`] from a string literal.
>  ///
> @@ -33,7 +129,7 @@
>  macro_rules! b_str {
>      ($str:literal) => {{
>          const S: &'static str = $str;
> -        const C: &'static $crate::str::BStr = S.as_bytes();
> +        const C: &'static $crate::str::BStr = BStr::from_bytes(S.as_bytes());
>          C
>      }};
>  }
> @@ -225,15 +321,7 @@ impl fmt::Display for CStr {
>      /// assert_eq!(s.as_bytes_with_nul(), "so \"cool\"\0".as_bytes());
>      /// ```
>      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)?;
> -            }
> -        }
> -        Ok(())
> +        fmt::Display::fmt(self.as_ref(), f)
>      }
>  }
>  
> @@ -255,23 +343,14 @@ impl fmt::Debug for CStr {
>      /// assert_eq!(s.as_bytes_with_nul(), "\"so \\\"cool\\\"\"\0".as_bytes());
>      /// ```
>      fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
> -        f.write_str("\"")?;
> -        for &c in self.as_bytes() {
> -            match c {
> -                // Printable characters.
> -                b'\"' => f.write_str("\\\"")?,
> -                0x20..=0x7e => f.write_char(c as char)?,
> -                _ => write!(f, "\\x{:02x}", c)?,
> -            }
> -        }
> -        f.write_str("\"")
> +        fmt::Debug::fmt(self.as_ref(), f)
>      }
>  }
>  
>  impl AsRef<BStr> for CStr {
>      #[inline]
>      fn as_ref(&self) -> &BStr {
> -        self.as_bytes()
> +        BStr::from_bytes(self.as_bytes())
>      }
>  }
>  
> @@ -280,7 +359,7 @@ impl Deref for CStr {
>  
>      #[inline]
>      fn deref(&self) -> &Self::Target {
> -        self.as_bytes()
> +        BStr::from_bytes(self.as_bytes())

Either use `as_ref` here or use `deref()` in the `as_ref`.

>      }
>  }
>  
> @@ -327,7 +406,7 @@ impl<Idx> Index<Idx> for CStr
>  
>      #[inline]
>      fn index(&self, index: Idx) -> &Self::Output {
> -        &self.as_bytes()[index]
> +        &self.as_ref()[index]
>      }
>  }


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] rust: str: implement `Display` and `Debug` for `BStr`
  2024-01-24 20:18 ` Gary Guo
@ 2024-01-25 13:25   ` Yutaro Ohno
  0 siblings, 0 replies; 3+ messages in thread
From: Yutaro Ohno @ 2024-01-25 13:25 UTC (permalink / raw)
  To: Gary Guo
  Cc: Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Boqun Feng,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	rust-for-linux, Virgile Andreani

On Wed, Jan 24, 2024 at 08:18:32PM +0000, Gary Guo wrote:
> On Thu, 25 Jan 2024 01:58:05 +0900
> Yutaro Ohno <yutaro.ono.418@gmail.com> wrote:
> 
> > Currently, `BStr` is just a type alias of `[u8]`, limiting its
> > representation to a byte list rather than a character list, which is not
> > ideal for printing and debugging.
> > 
> > Implement `Display` and `Debug` traits for `BStr` to facilitate easier
> > printing and debugging.
> > 
> > Also, for this purpose, change `BStr` from a type alias of `[u8]` to a
> > struct wrapper of `[u8]`.
> > 
> > Co-developed-by: Virgile Andreani <armavica@ulminfo.fr>
> > Signed-off-by: Virgile Andreani <armavica@ulminfo.fr>
> > Signed-off-by: Yutaro Ohno <yutaro.ono.418@gmail.com>
> > ---
> >  rust/kernel/str.rs | 211 +++++++++++++++++++++++++++++++++++++++------
> >  1 file changed, 186 insertions(+), 25 deletions(-)
> > 
> > diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
> > index 7d848b83add4..0f0261e063d2 100644
> > --- a/rust/kernel/str.rs
> > +++ b/rust/kernel/str.rs
> > @@ -14,8 +14,104 @@
> >  
> >  /// Byte string without UTF-8 validity guarantee.
> >  ///
> > -/// `BStr` is simply an alias to `[u8]`, but has a more evident semantical meaning.
> > -pub type BStr = [u8];
> > +/// `BStr` is simply a wrapper over `[u8]`, but has a more evident semantical
> > +/// meaning.
> 
> I think this line can go?
> 

Indeed. Will remove in v2.

> > +#[repr(transparent)]
> > +pub struct BStr([u8]);
> > +
> > +impl BStr {
> > +    /// Returns the length of this string.
> > +    #[inline]
> > +    pub const fn len(&self) -> usize {
> > +        self.0.len()
> > +    }
> > +
> > +    /// Returns `true` if the string is empty.
> > +    #[inline]
> > +    pub const fn is_empty(&self) -> bool {
> > +        self.len() == 0
> > +    }
> > +
> > +    /// Creates a [`BStr`] from a `[u8]`.
> > +    #[inline]
> > +    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.
> > +    ///
> > +    /// ```
> > +    /// # use kernel::{fmt, b_str, str::{BStr, CString}};
> > +    /// let ascii = b_str!("Hello, BStr!");
> > +    /// let s = CString::try_from_fmt(fmt!("{}", ascii)).unwrap();
> > +    /// assert_eq!(s.as_bytes(), "Hello, BStr!".as_bytes());
> > +    ///
> > +    /// let non_ascii = b_str!("🦀");
> > +    /// let s = CString::try_from_fmt(fmt!("{}", non_ascii)).unwrap();
> > +    /// assert_eq!(s.as_bytes(), "\\xf0\\x9f\\xa6\\x80".as_bytes());
> > +    /// ```
> > +    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
> > +        for &b in &self.0 {
> > +            match b {
> > +                // Common escape codes.
> > +                b'\t' => f.write_str("\\t")?,
> > +                b'\n' => f.write_str("\\n")?,
> > +                b'\r' => f.write_str("\\r")?,
> 
> The current CStr code will print these as is. Why escaping these for
> Display? Also, if you want to change the behaviour, please put it in a
> separate patch.
> 

That makes sense. I'll change.

> > +                // Printable characters.
> > +                0x20..=0x7e => f.write_char(b as char)?,
> > +                _ => write!(f, "\\x{:02x}", b)?,
> > +            }
> > +        }
> > +        Ok(())
> > +    }
> > +}
> > +
> > +impl fmt::Debug for BStr {
> > +    /// Formats printable ASCII characters with a double quote on either end,
> > +    /// escaping the rest.
> > +    ///
> > +    /// ```
> > +    /// # use kernel::{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)).unwrap();
> > +    /// assert_eq!(s.as_bytes(), "\"Hello, \\\"BStr\\\"!\"".as_bytes());
> > +    ///
> > +    /// let non_ascii = b_str!("😺");
> > +    /// let s = CString::try_from_fmt(fmt!("{:?}", non_ascii)).unwrap();
> > +    /// assert_eq!(s.as_bytes(), "\"\\xf0\\x9f\\x98\\xba\"".as_bytes());
> > +    /// ```
> > +    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
> > +        f.write_str("\"")?;
> > +        for &b in &self.0 {
> > +            match b {
> > +                // Common escape codes.
> > +                b'\t' => f.write_str("\\t")?,
> > +                b'\n' => f.write_str("\\n")?,
> > +                b'\r' => f.write_str("\\r")?,
> > +                // String escape characters.
> > +                b'\\' => f.write_str("\\\\")?,
> > +                b'\"' => f.write_str("\\\"")?,
> > +                // Printable characters.
> > +                0x20..=0x7e => f.write_char(b as char)?,
> > +                _ => write!(f, "\\x{:02x}", b)?,
> > +            }
> > +        }
> > +        f.write_str("\"")
> > +    }
> > +}
> > +
> > +impl Deref for BStr {
> > +    type Target = [u8];
> > +
> > +    #[inline]
> > +    fn deref(&self) -> &Self::Target {
> > +        &self.0
> > +    }
> > +}
> >  
> >  /// Creates a new [`BStr`] from a string literal.
> >  ///
> > @@ -33,7 +129,7 @@
> >  macro_rules! b_str {
> >      ($str:literal) => {{
> >          const S: &'static str = $str;
> > -        const C: &'static $crate::str::BStr = S.as_bytes();
> > +        const C: &'static $crate::str::BStr = BStr::from_bytes(S.as_bytes());
> >          C
> >      }};
> >  }
> > @@ -225,15 +321,7 @@ impl fmt::Display for CStr {
> >      /// assert_eq!(s.as_bytes_with_nul(), "so \"cool\"\0".as_bytes());
> >      /// ```
> >      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)?;
> > -            }
> > -        }
> > -        Ok(())
> > +        fmt::Display::fmt(self.as_ref(), f)
> >      }
> >  }
> >  
> > @@ -255,23 +343,14 @@ impl fmt::Debug for CStr {
> >      /// assert_eq!(s.as_bytes_with_nul(), "\"so \\\"cool\\\"\"\0".as_bytes());
> >      /// ```
> >      fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
> > -        f.write_str("\"")?;
> > -        for &c in self.as_bytes() {
> > -            match c {
> > -                // Printable characters.
> > -                b'\"' => f.write_str("\\\"")?,
> > -                0x20..=0x7e => f.write_char(c as char)?,
> > -                _ => write!(f, "\\x{:02x}", c)?,
> > -            }
> > -        }
> > -        f.write_str("\"")
> > +        fmt::Debug::fmt(self.as_ref(), f)
> >      }
> >  }
> >  
> >  impl AsRef<BStr> for CStr {
> >      #[inline]
> >      fn as_ref(&self) -> &BStr {
> > -        self.as_bytes()
> > +        BStr::from_bytes(self.as_bytes())
> >      }
> >  }
> >  
> > @@ -280,7 +359,7 @@ impl Deref for CStr {
> >  
> >      #[inline]
> >      fn deref(&self) -> &Self::Target {
> > -        self.as_bytes()
> > +        BStr::from_bytes(self.as_bytes())
> 
> Either use `as_ref` here or use `deref()` in the `as_ref`.
> 

I'll change in v2.

> >      }
> >  }
> >  
> > @@ -327,7 +406,7 @@ impl<Idx> Index<Idx> for CStr
> >  
> >      #[inline]
> >      fn index(&self, index: Idx) -> &Self::Output {
> > -        &self.as_bytes()[index]
> > +        &self.as_ref()[index]
> >      }
> >  }
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2024-01-25 13:23 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-01-24 16:58 [PATCH] rust: str: implement `Display` and `Debug` for `BStr` Yutaro Ohno
2024-01-24 20:18 ` Gary Guo
2024-01-25 13:25   ` Yutaro Ohno

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).