Re: [PATCH V2] rust: str: implement `Display` and `Debug` for `BStr`

[Alice Ryhl <aliceryhl@google.com>]

Yutaro Ohno <yutaro.ono.418@gmail.com> writes:
> 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>
>
> [...]
>
> +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 {
> +                // Printable characters.
> +                0x20..=0x7e => f.write_char(b as char)?,
> +                _ => write!(f, "\\x{:02x}", b)?,
> +            }
> +        }
> +        Ok(())
> +    }
> +}

What about newlines and tabs? Those are printable.

If you are intentionally excluding them, please add a comment saying that.

> +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 {
> +                // Printable characters.
> +                b'\"' => f.write_str("\\\"")?,
> +                0x20..=0x7e => f.write_char(b as char)?,
> +                _ => write!(f, "\\x{:02x}", b)?,
> +            }
> +        }
> +        f.write_str("\"")
> +    }
> +}

This escapes quotes, but what about backslashes?

Also, perhaps `write_char('"')` makes more sense than `write_str("\"")`?

>  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
>      }};
>  }

This should use the full path both times:

const C: &'static $crate::str::BStr = $crate::str::BStr::from_bytes(S.as_bytes());

Alice