lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAJ-ks9n7u3WkYmJCVc18c_cKod6DaB7KrA7NXbOuD3E3TYbvpQ@mail.gmail.com>
Date: Sun, 4 May 2025 13:29:19 -0400
From: Tamir Duberstein <tamird@...il.com>
To: Miguel Ojeda <ojeda@...nel.org>
Cc: Brendan Higgins <brendan.higgins@...ux.dev>, David Gow <davidgow@...gle.com>, 
	Alex Gaynor <alex.gaynor@...il.com>, Rae Moar <rmoar@...gle.com>, 
	linux-kselftest@...r.kernel.org, kunit-dev@...glegroups.com, 
	Boqun Feng <boqun.feng@...il.com>, Gary Guo <gary@...yguo.net>, 
	Björn Roy Baron <bjorn3_gh@...tonmail.com>, 
	Benno Lossin <benno.lossin@...ton.me>, Andreas Hindborg <a.hindborg@...nel.org>, 
	Alice Ryhl <aliceryhl@...gle.com>, Trevor Gross <tmgross@...ch.edu>, 
	Danilo Krummrich <dakr@...nel.org>, rust-for-linux@...r.kernel.org, 
	linux-kernel@...r.kernel.org, patches@...ts.linux.dev
Subject: Re: [PATCH 5/7] rust: str: take advantage of the `-> Result` support
 in KUnit `#[test]`'s

On Fri, May 2, 2025 at 5:54 PM Miguel Ojeda <ojeda@...nel.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@...nel.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
>
>

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ