[PATCH] rust: kunit: use crate-level mapping for `c_void`

[PATCH] rust: kunit: use crate-level mapping for `c_void`

[Jesung Yang]

Use `kernel::ffi::c_void` instead of `core::ffi::c_void` for consistency
and to centralize abstraction.

Since `kernel::ffi::c_void` is a transparent wrapper around
`core::ffi::c_void`, both are functionally equivalent. However, using
`kernel::ffi::c_void` improves consistency across the kernel's Rust code
and provides a unified reference point in case the definition ever needs
to change, even if such a change is unlikely.

Signed-off-by: Jesung Yang <y.j3ms.n@gmail.com>
---
 rust/kernel/kunit.rs | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/rust/kernel/kunit.rs b/rust/kernel/kunit.rs
index 81833a687b75..bd6fc712dd79 100644
--- a/rust/kernel/kunit.rs
+++ b/rust/kernel/kunit.rs
@@ -6,7 +6,8 @@
 //!
 //! Reference: <https://docs.kernel.org/dev-tools/kunit/index.html>
 
-use core::{ffi::c_void, fmt};
+use core::fmt;
+use kernel::ffi::c_void;
 
 /// Prints a KUnit error-level message.
 ///
-- 
2.39.5

Re: [PATCH] rust: kunit: use crate-level mapping for `c_void`

[Miguel Ojeda]

On Mon, May 26, 2025 at 6:26 PM Jesung Yang <y.j3ms.n@gmail.com> wrote:
>
> Since `kernel::ffi::c_void` is a transparent wrapper around
> `core::ffi::c_void`, both are functionally equivalent. However, using

Hmm... It is not a transparent wrapper, but a reexport, right? (it is
not even a type alias, like the others in the `ffi` crate).

Other than that, the change looks fine -- thanks for the patch!

(By the way, in general, please provide the `--base` flag to
`format-patch` when possible, since that makes later on applying
commits much easier. And it usually doesn't hurt to have a "Link:" tag
to the discussion in Zulip.)

Cheers,
Miguel

Re: [PATCH] rust: kunit: use crate-level mapping for `c_void`

[Benno Lossin]

On Mon May 26, 2025 at 6:24 PM CEST, Jesung Yang wrote:
> Use `kernel::ffi::c_void` instead of `core::ffi::c_void` for consistency
> and to centralize abstraction.
>
> Since `kernel::ffi::c_void` is a transparent wrapper around
> `core::ffi::c_void`, both are functionally equivalent. However, using
> `kernel::ffi::c_void` improves consistency across the kernel's Rust code
> and provides a unified reference point in case the definition ever needs
> to change, even if such a change is unlikely.
>
> Signed-off-by: Jesung Yang <y.j3ms.n@gmail.com>
> ---
>  rust/kernel/kunit.rs | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/rust/kernel/kunit.rs b/rust/kernel/kunit.rs
> index 81833a687b75..bd6fc712dd79 100644
> --- a/rust/kernel/kunit.rs
> +++ b/rust/kernel/kunit.rs
> @@ -6,7 +6,8 @@
>  //!
>  //! Reference: <https://docs.kernel.org/dev-tools/kunit/index.html>
>  
> -use core::{ffi::c_void, fmt};
> +use core::fmt;
> +use kernel::ffi::c_void;

We don't need to explicitly import it, as `c_void` is present in the
prelude since 3d5bef5d47c3 ("rust: add C FFI types to the prelude").

With the import removed:

Reviewed-by: Benno Lossin <lossin@kernel.org>

---
Cheers,
Benno

Re: [PATCH] rust: kunit: use crate-level mapping for `c_void`

[Miguel Ojeda]

On Tue, May 27, 2025 at 2:06 PM Benno Lossin <lossin@kernel.org> wrote:
>
> We don't need to explicitly import it, as `c_void` is present in the
> prelude since 3d5bef5d47c3 ("rust: add C FFI types to the prelude").

Hmm... But the prelude isn't there yet in this patch, no? i.e. our
prelude is (so far) not a "real prelude" that gets injected
automatically. So I guess you mean importing the prelude instead.

(It is imported in the KUnit series anyway, so it will llikely be
there either way)

Thanks!

Cheers,
Miguel

Re: [PATCH] rust: kunit: use crate-level mapping for `c_void`

[Benno Lossin]

On Tue May 27, 2025 at 3:51 PM CEST, Miguel Ojeda wrote:
> On Tue, May 27, 2025 at 2:06 PM Benno Lossin <lossin@kernel.org> wrote:
>>
>> We don't need to explicitly import it, as `c_void` is present in the
>> prelude since 3d5bef5d47c3 ("rust: add C FFI types to the prelude").
>
> Hmm... But the prelude isn't there yet in this patch, no? i.e. our
> prelude is (so far) not a "real prelude" that gets injected
> automatically. So I guess you mean importing the prelude instead.

Ah right it's only auto-imported in the doctests. Forgot that, would be
nice if it could be :)

> (It is imported in the KUnit series anyway, so it will llikely be
> there either way)

Oh yeah, it's in rust-next already :)

---
Cheers,
Benno

Re: [PATCH] rust: kunit: use crate-level mapping for `c_void`

[Jesung Yang]

Hi,

On Tue, May 27, 2025 at 9:06 PM Benno Lossin <lossin@kernel.org> wrote:
>
> We don't need to explicitly import it, as `c_void` is present in the
> prelude since 3d5bef5d47c3 ("rust: add C FFI types to the prelude").

The base commit of my patch is f4daa80d6be7 ("rust: compile libcore with
edition 2024 for 1.87+"), which unfortunately predates the addition of
`use crate::prelude::*`. As a result, removing `use kernel::ffi::c_void`
causes the build to fail with the following error:

```
error[E0412]: cannot find type `c_void` in this scope
  --> rust/kernel/kunit.rs:22:41
   |
22 |             &args as *const _ as *const c_void,
   |                                         ^^^^^^ not found in this scope
   |
help: consider importing one of these enums
   |
9  + use crate::prelude::c_void;
   |
9  + use core::ffi::c_void;
   |
9  + use ffi::c_void;
   |

error[E0412]: cannot find type `c_void` in this scope
  --> rust/kernel/kunit.rs:38:41
   |
38 |             &args as *const _ as *const c_void,
   |                                         ^^^^^^ not found in this scope
   |
help: consider importing one of these enums
   |
9  + use crate::prelude::c_void;
   |
9  + use core::ffi::c_void;
   |
9  + use ffi::c_void;
   |

error: aborting due to 2 previous errors
```

Starting from commit c4c0574ee33b ("rust: add `kunit_tests` to the
prelude"), we do have `use crate::prelude::*;`, so the explicit import
is no longer necessary in that context.

Thanks for pointing this out!

Best regards,
Jesung