Re: [PATCH 5/7] rust: str: take advantage of the `-> Result` support in KUnit `#[test]`'s

Tamir Duberstein
2025-05-04T17:29:58Z

On Fri, May 2, 2025 at 5:54 PM Miguel Ojeda <ojeda@kernel.org> wrote:
>
> Since now we have support for returning `-> Result`s, we can convert some
> of these tests to use the feature, and serve as a first user for it too.
>
> Thus convert them.
>
> This, in turn, simplifies them a fair bit.
>
> We keep the actual assertions we want to make as explicit ones with
> `assert*!`s.
>
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

Alice pointed this out in another thread but: one of the downsides of
returning Result is that in the event of failure the line number where
the error occurred is no longer contained in the test output. I'm 👎
on this change for that reason.

> ---
> rust/kernel/str.rs | 68 ++++++++++++++++++++--------------------------
> 1 file changed, 30 insertions(+), 38 deletions(-)
>
> diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
> index cf2caa2db168..8dcfb11013f2 100644
> --- a/rust/kernel/str.rs
> +++ b/rust/kernel/str.rs
> @@ -576,25 +576,9 @@ macro_rules! c_str {
> mod tests {
> use super::*;
>
> - struct String(CString);
> -
> - impl String {
> - fn from_fmt(args: fmt::Arguments<'_>) -> Self {
> - String(CString::try_from_fmt(args).unwrap())
> - }
> - }
> -
> - impl Deref for String {
> - type Target = str;
> -
> - fn deref(&self) -> &str {
> - self.0.to_str().unwrap()
> - }
> - }

These changes don't depend on returning `Result` from the tests
AFAICT. Can they be in a separate patch?

> -
> macro_rules! format {
> ($($f:tt)*) => ({
> - &*String::from_fmt(kernel::fmt!($($f)*))
> + CString::try_from_fmt(kernel::fmt!($($f)*))?.to_str()?
> })
> }
>
> @@ -613,66 +597,72 @@ macro_rules! format {
> \\xf0\\xf1\\xf2\\xf3\\xf4\\xf5\\xf6\\xf7\\xf8\\xf9\\xfa\\xfb\\xfc\\xfd\\xfe\\xff";
>
> #[test]
> - fn test_cstr_to_str() {
> + fn test_cstr_to_str() -> Result {
> let good_bytes = b"\xf0\x9f\xa6\x80\0";
> - let checked_cstr = CStr::from_bytes_with_nul(good_bytes).unwrap();
> - let checked_str = checked_cstr.to_str().unwrap();
> + let checked_cstr = CStr::from_bytes_with_nul(good_bytes)?;
> + let checked_str = checked_cstr.to_str()?;
> assert_eq!(checked_str, "🦀");
> + Ok(())
> }
>
> #[test]
> - fn test_cstr_to_str_invalid_utf8() {
> + fn test_cstr_to_str_invalid_utf8() -> Result {
> let bad_bytes = b"\xc3\x28\0";
> - let checked_cstr = CStr::from_bytes_with_nul(bad_bytes).unwrap();
> + let checked_cstr = CStr::from_bytes_with_nul(bad_bytes)?;
> assert!(checked_cstr.to_str().is_err());
> + Ok(())
> }
>
> #[test]
> - fn test_cstr_as_str_unchecked() {
> + fn test_cstr_as_str_unchecked() -> Result {
> let good_bytes = b"\xf0\x9f\x90\xA7\0";
> - let checked_cstr = CStr::from_bytes_with_nul(good_bytes).unwrap();
> + let checked_cstr = CStr::from_bytes_with_nul(good_bytes)?;
> // 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, "🐧");
> + Ok(())
> }
>
> #[test]
> - fn test_cstr_display() {
> - let hello_world = CStr::from_bytes_with_nul(b"hello, world!\0").unwrap();
> + fn test_cstr_display() -> Result {
> + let hello_world = CStr::from_bytes_with_nul(b"hello, world!\0")?;
> assert_eq!(format!("{}", hello_world), "hello, world!");
> - let non_printables = CStr::from_bytes_with_nul(b"\x01\x09\x0a\0").unwrap();
> + let non_printables = CStr::from_bytes_with_nul(b"\x01\x09\x0a\0")?;
> assert_eq!(format!("{}", non_printables), "\\x01\\x09\\x0a");
> - let non_ascii = CStr::from_bytes_with_nul(b"d\xe9j\xe0 vu\0").unwrap();
> + let non_ascii = CStr::from_bytes_with_nul(b"d\xe9j\xe0 vu\0")?;
> assert_eq!(format!("{}", non_ascii), "d\\xe9j\\xe0 vu");
> - let good_bytes = CStr::from_bytes_with_nul(b"\xf0\x9f\xa6\x80\0").unwrap();
> + let good_bytes = CStr::from_bytes_with_nul(b"\xf0\x9f\xa6\x80\0")?;
> assert_eq!(format!("{}", good_bytes), "\\xf0\\x9f\\xa6\\x80");
> + Ok(())
> }
>
> #[test]
> - fn test_cstr_display_all_bytes() {
> + fn test_cstr_display_all_bytes() -> Result {
> 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();
> + let cstr = CStr::from_bytes_with_nul(&bytes)?;
> assert_eq!(format!("{}", cstr), ALL_ASCII_CHARS);
> + Ok(())
> }
>
> #[test]
> - fn test_cstr_debug() {
> - let hello_world = CStr::from_bytes_with_nul(b"hello, world!\0").unwrap();
> + fn test_cstr_debug() -> Result {
> + let hello_world = CStr::from_bytes_with_nul(b"hello, world!\0")?;
> assert_eq!(format!("{:?}", hello_world), "\"hello, world!\"");
> - let non_printables = CStr::from_bytes_with_nul(b"\x01\x09\x0a\0").unwrap();
> + let non_printables = CStr::from_bytes_with_nul(b"\x01\x09\x0a\0")?;
> assert_eq!(format!("{:?}", non_printables), "\"\\x01\\x09\\x0a\"");
> - let non_ascii = CStr::from_bytes_with_nul(b"d\xe9j\xe0 vu\0").unwrap();
> + let non_ascii = CStr::from_bytes_with_nul(b"d\xe9j\xe0 vu\0")?;
> assert_eq!(format!("{:?}", non_ascii), "\"d\\xe9j\\xe0 vu\"");
> - let good_bytes = CStr::from_bytes_with_nul(b"\xf0\x9f\xa6\x80\0").unwrap();
> + let good_bytes = CStr::from_bytes_with_nul(b"\xf0\x9f\xa6\x80\0")?;
> assert_eq!(format!("{:?}", good_bytes), "\"\\xf0\\x9f\\xa6\\x80\"");
> + Ok(())
> }
>
> #[test]
> - fn test_bstr_display() {
> + fn test_bstr_display() -> Result {
> 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_\\_\'_\"_");
> @@ -683,10 +673,11 @@ fn test_bstr_display() {
> 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");
> + Ok(())
> }
>
> #[test]
> - fn test_bstr_debug() {
> + fn test_bstr_debug() -> Result {
> 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_\\_\'_\"_");
> @@ -697,6 +688,7 @@ fn test_bstr_debug() {
> 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\"");
> + Ok(())
> }
> }
>
> --
> 2.49.0
>
>