[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.j3...@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

[Miguel Ojeda]

On Mon, May 26, 2025 at 6:26 PM Jesung Yang <y.j3...@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

[Benno Lossin]

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 <los...@kernel.org>

---
Cheers,
Benno

[Miguel Ojeda]

On Tue, May 27, 2025 at 2:06 PM Benno Lossin <los...@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

[Jesung Yang]

Hi,

On Tue, May 27, 2025 at 9:06 PM Benno Lossin <los...@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

[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 <los...@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

[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 straightforward re-export of

`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.j3...@gmail.com>

Link: https://rust-for-linux.zulipchat.com/#narrow/channel/288089/topic/x/near/520452733
---
So in sum, I believe it's reasonable to keep the diff unchanged... but
I'm happy to adjust if you'd prefer a different approach.
---
Changes in v2:
- Add "Link" tag to the related discussion on Zulip
- Reword the commit message to clarify `kernel::ffi::c_void` is a re-export
- Link to v1: https://lore.kernel.org/rust-for-linux/20250526162429.1...@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.
///

base-commit: f4daa80d6be7d3c55ca72a8e560afc4e21f886aa
--
2.39.5

[Miguel Ojeda]

On Wed, May 28, 2025 at 5:55 PM Jesung Yang <y.j3...@gmail.com> wrote:
>
> So in sum, I believe it's reasonable to keep the diff unchanged... but
> I'm happy to adjust if you'd prefer a different approach.

I see this is based on rust-next -- if you rebase the patch on top of
the latest one, you will see the prelude there :)

Thanks!

Cheers,
Miguel

[JS Y]

On Thu, May 29, 2025 at 1:31 AM Miguel Ojeda
<miguel.oje...@gmail.com> wrote:
>
> I see this is based on rust-next -- if you rebase the patch on top of
> the latest one, you will see the prelude there :)

Thanks for the heads up! I'll rebase and send out v3 accordingly.

Best regards,
Jesung

[Jesung Yang]

Remove `use core::ffi::c_void`, which shadows `kernel::ffi::c_void`
brought in via `use crate::prelude::*`, to maintain consistency and
centralize the abstraction.

Since `kernel::ffi::c_void` is a straightforward re-export of
`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.

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

Signed-off-by: Jesung Yang <y.j3...@gmail.com>
Link: https://rust-for-linux.zulipchat.com/#narrow/channel/288089/topic/x/near/520452733
---

Changes in v3:
- Rebase on a3b2347343e0
- Remove the explicit import of `kernel::ffi::c_void`
- Reword the commit message accordingly
- Link to v2: https://lore.kernel.org/rust-for-linux/20250528155147.2...@gmail.com/

Changes in v2:
- Add "Link" tag to the related discussion on Zulip
- Reword the commit message to clarify `kernel::ffi::c_void` is a re-export
- Link to v1: https://lore.kernel.org/rust-for-linux/20250526162429.1...@gmail.com/
---

rust/kernel/kunit.rs | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/rust/kernel/kunit.rs b/rust/kernel/kunit.rs
index 4b8cdcb21e77..603330f247c7 100644
--- a/rust/kernel/kunit.rs
+++ b/rust/kernel/kunit.rs
@@ -7,7 +7,7 @@
//! Reference: <https://docs.kernel.org/dev-tools/kunit/index.html>

use crate::prelude::*;

-use core::{ffi::c_void, fmt};
+use core::fmt;

/// Prints a KUnit error-level message.
///

base-commit: a3b2347343e077e81d3c169f32c9b2cb1364f4cc
--
2.39.5

[Miguel Ojeda]

On Wed, May 28, 2025 at 7:51 PM Jesung Yang <y.j3...@gmail.com> wrote:
>
> Remove `use core::ffi::c_void`, which shadows `kernel::ffi::c_void`
> brought in via `use crate::prelude::*`, to maintain consistency and
> centralize the abstraction.
>
> Since `kernel::ffi::c_void` is a straightforward re-export of
> `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.
>
> Reviewed-by: Benno Lossin <los...@kernel.org>
> Signed-off-by: Jesung Yang <y.j3...@gmail.com>
> Link: https://rust-for-linux.zulipchat.com/#narrow/channel/288089/topic/x/near/520452733

Looks good to me, thanks! If KUnit picks this next cycle:

Acked-by: Miguel Ojeda <oj...@kernel.org>

Otherwise, I am happy to take it too.

Cheers,
Miguel

[Miguel Ojeda]

On Wed, May 28, 2025 at 7:51 PM Jesung Yang <y.j3...@gmail.com> wrote:
>

> Remove `use core::ffi::c_void`, which shadows `kernel::ffi::c_void`
> brought in via `use crate::prelude::*`, to maintain consistency and
> centralize the abstraction.
>
> Since `kernel::ffi::c_void` is a straightforward re-export of
> `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.
>
> Reviewed-by: Benno Lossin <los...@kernel.org>
> Signed-off-by: Jesung Yang <y.j3...@gmail.com>
> Link: https://rust-for-linux.zulipchat.com/#narrow/channel/288089/topic/x/near/520452733

Applied to `rust-next` -- thanks everyone!

Cheers,
Miguel