On Mon, Jul 15, 2024 at 5:14 PM Michal Rostecki vadorovsky@gmail.com wrote:
diff --git a/rust/kernel/kunit.rs b/rust/kernel/kunit.rs index 0ba77276ae7e..c08f9dddaa6f 100644 --- a/rust/kernel/kunit.rs +++ b/rust/kernel/kunit.rs [...] // SAFETY: FFI call without safety requirements. let kunit_test = unsafe { $crate::bindings::kunit_get_current_test() }; @@ -71,11 +71,11 @@ macro_rules! kunit_assert { // // This mimics KUnit's failed assertion format. $crate::kunit::err(format_args!(
" # {}: ASSERTION FAILED at {FILE}:{LINE}\n",
" # {:?}: ASSERTION FAILED at {FILE:?}:{LINE:?}\n", $name )); $crate::kunit::err(format_args!(
" Expected {CONDITION} to be true, but is false\n"
" Expected {CONDITION:?} to be true, but is false\n" ));
These aren't exactly the same: the existing `Display` impl will print the string (hexifying invalid characters), but this will add `" ... "` around it.
In Rust's libraries, string `Path` and `OsStr` both have a `.display()` method that returns a wrapper type that does implement `fmt::Display`, which can then be printed (see [1]). We could do something similar here, via a `CStrExt` trait that goes in the prelude and provides `.display(&self)`.
Rust itself could actually use something here too - if you're up for it, feel free to propose an implementation via ACP (that's just an issue template at [2]). It would probably be pretty similar to the recent `OsStr` one. Of course it will be a while before we can use it in the kernel, but it wouldn't hurt to get the ball rolling.
[1]: https://doc.rust-lang.org/std/path/struct.Path.html#method.display [2]: https://github.com/rust-lang/libs-team/issues
/// Creates a new [`CStr`] from a string literal. /// -/// The string literal should not contain any `NUL` bytes. +/// Usually, defining C-string literals directly should be preffered, but this +/// macro is helpful in situations when C-string literals are hard or +/// impossible to use, for example: +/// +/// - When working with macros, which already return a Rust string literal +/// (e.g. `stringify!`). +/// - When building macros, where we want to take a Rust string literal as an +/// argument (for caller's convenience), but still use it as a C-string +/// internally. +///
s/preffered/prefered
"when C-string literals are hard or impossible to use" doesn't sound quite right - I think it is more common that you're just hiding an implementation detail (string type) from the user of a macro. Maybe something like:
This isn't needed when C-string literals (`c"hello"` syntax) can be used directly, but can be used when a C-string version of a standard string literal is required (often when working with macros).
+/// The string should not contain any `NUL` bytes. /// /// # Examples /// /// ``` +/// # use core::ffi::CStr; /// # use kernel::c_str; -/// # use kernel::str::CStr; -/// const MY_CSTR: &CStr = c_str!("My awesome CStr!"); +/// const MY_CSTR: &CStr = c_str!(stringify!(5)); /// ``` #[macro_export] macro_rules! c_str { ($str:expr) => {{ const S: &str = concat!($str, "\0");
const C: &$crate::str::CStr = match $crate::str::CStr::from_bytes_with_nul(S.as_bytes()) {
const C: &core::ffi::CStr = match core::ffi::CStr::from_bytes_with_nul(S.as_bytes()) { Ok(v) => v, Err(_) => panic!("string contains interior NUL"), };
Thanks for this, will be a nice bit of code cleanup.
- Trevor
(also, v2 and v3 are appearing in different threads on lore (as they should), but they're in the same thread as v1 in my email client - any idea if there is a reason for this?)