On Mon, Jul 15, 2024 at 5:14 PM Michal Rostecki <vadorovsky@xxxxxxxxx> 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?)
