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. ///
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
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
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
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
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
linux-kselftest-mirror@lists.linaro.org
-
Benno Lossin -
Jesung Yang -
Miguel Ojeda