* Re: [PATCH] rust: macros: Fix macro referencing core and kernel crates
@ 2025-03-28 0:49 Benno Lossin
2025-03-28 18:03 ` [PATCH v2 0/1] rust: macros: Fix macro referencing core and kernel crates v2 Igor Korotin
0 siblings, 1 reply; 6+ messages in thread
From: Benno Lossin @ 2025-03-28 0:49 UTC (permalink / raw)
To: Igor Korotin, linux-kernel; +Cc: rust-for-linux, y86-dev
On Wed Mar 26, 2025 at 7:23 PM CET, Igor Korotin wrote:
> To prevent issues where user-defined modules named `core` or `kernel`
> could be picked up instead of the standard ones, update macros to use
> absolute paths, `::core::...` and `::kernel::...`, for core and kernel
> references.
Please split this sentence up into smaller parts. Also "standard ones"
sounds weird to me, `core` and `kernel` are crates and not modules.
> Suggested-by: y86-dev <y86-dev@protonmail.com>
Please use my real name and other mail address instead:
Benno Lossin <benno.lossin@proton.me>
> Closes: https://github.com/Rust-for-Linux/linux/issues/1150
> Signed-off-by: Igor Korotin <igor.korotin@yahoo.com>
> ---
> rust/ffi.rs | 2 +-
> rust/kernel/device.rs | 2 +-
> rust/kernel/device_id.rs | 4 ++--
> rust/kernel/kunit.rs | 8 ++++----
> rust/kernel/static_assert.rs | 4 ++--
> rust/kernel/str.rs | 4 ++--
> rust/macros/lib.rs | 6 +++---
> scripts/rustdoc_test_gen.rs | 8 ++++++--
> 8 files changed, 21 insertions(+), 17 deletions(-)
There are also some occurrences of `kernel::` & `core::` in proc macros,
for example:
- rust/macros/kunit.rs:129
- rust/macros/module.rs:347
- ...
Could you also add those to the patch? Thanks!
(line numbers after applying your patch to rust-next)
---
Cheers,
Benno
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 0/1] rust: macros: Fix macro referencing core and kernel crates v2
2025-03-28 0:49 [PATCH] rust: macros: Fix macro referencing core and kernel crates Benno Lossin
@ 2025-03-28 18:03 ` Igor Korotin
2025-03-28 18:03 ` [PATCH v2 1/1] " Igor Korotin
2025-03-28 21:48 ` [PATCH v2 0/1] " Benno Lossin
0 siblings, 2 replies; 6+ messages in thread
From: Igor Korotin @ 2025-03-28 18:03 UTC (permalink / raw)
To: benno.lossin; +Cc: linux-kernel, rust-for-linux, Igor Korotin
> > To prevent issues where user-defined modules named `core` or `kernel`
> > could be picked up instead of the standard ones, update macros to use
> > absolute paths, `::core::...` and `::kernel::...`, for core and kernel
> > references.
>
> Please split this sentence up into smaller parts. Also "standard ones"
> sounds weird to me, `core` and `kernel` are crates and not modules.
Simplified the commit message.
> > Suggested-by: y86-dev <y86-dev@protonmail.com>
>
> Please use my real name and other mail address instead:
>
> Benno Lossin <benno.lossin@proton.me>
Sure. Tried to find your creds via your github profile/commits without
success.
> > Closes: https://github.com/Rust-for-Linux/linux/issues/1150
> > Signed-off-by: Igor Korotin <igor.korotin@yahoo.com>
> > ---
> > rust/ffi.rs | 2 +-
> > rust/kernel/device.rs | 2 +-
> > rust/kernel/device_id.rs | 4 ++--
> > rust/kernel/kunit.rs | 8 ++++----
> > rust/kernel/static_assert.rs | 4 ++--
> > rust/kernel/str.rs | 4 ++--
> > rust/macros/lib.rs | 6 +++---
> > scripts/rustdoc_test_gen.rs | 8 ++++++--
> > 8 files changed, 21 insertions(+), 17 deletions(-)
>
> There are also some occurrences of `kernel::` & `core::` in proc macros,
> for example:
> - rust/macros/kunit.rs:129
> - rust/macros/module.rs:347
> - ...
>
> Could you also add those to the patch? Thanks!
>
> (line numbers after applying your patch to rust-next)
Agreed. Fixed references in auto-generated code in files
1. rust/macros/kunit.rs
2. rust/macros/module.rs
3. scripts/rustdoc_test_builder.rs
4. scripts/rustdoc_test_gen.rs
--
2.43.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 1/1] rust: macros: Fix macro referencing core and kernel crates v2
2025-03-28 18:03 ` [PATCH v2 0/1] rust: macros: Fix macro referencing core and kernel crates v2 Igor Korotin
@ 2025-03-28 18:03 ` Igor Korotin
2025-03-28 21:50 ` Benno Lossin
2025-03-29 12:28 ` Miguel Ojeda
2025-03-28 21:48 ` [PATCH v2 0/1] " Benno Lossin
1 sibling, 2 replies; 6+ messages in thread
From: Igor Korotin @ 2025-03-28 18:03 UTC (permalink / raw)
To: benno.lossin; +Cc: linux-kernel, rust-for-linux, Igor Korotin
Update macros to always use absolute paths for crates `core` and `kernel`.
This guarantees that macros will not pick up user-defined crates `core`
or `kernel` by accident.
Changes since v1:
- Fixed paths in auto-generated code.
Suggested-by: Benno Lossin <benno.lossin@proton.me>
Closes: https://github.com/Rust-for-Linux/linux/issues/1150
Signed-off-by: Igor Korotin <igor.korotin@yahoo.com>
---
rust/ffi.rs | 2 +-
rust/kernel/device.rs | 2 +-
rust/kernel/device_id.rs | 4 ++--
rust/kernel/kunit.rs | 8 ++++----
rust/kernel/static_assert.rs | 4 ++--
rust/kernel/str.rs | 4 ++--
rust/macros/kunit.rs | 22 +++++++++++-----------
rust/macros/lib.rs | 6 +++---
rust/macros/module.rs | 31 ++++++++++++++++---------------
scripts/rustdoc_test_builder.rs | 6 +++---
scripts/rustdoc_test_gen.rs | 16 ++++++++++------
11 files changed, 55 insertions(+), 50 deletions(-)
diff --git a/rust/ffi.rs b/rust/ffi.rs
index 584f75b49862..d60aad792af4 100644
--- a/rust/ffi.rs
+++ b/rust/ffi.rs
@@ -17,7 +17,7 @@ macro_rules! alias {
// Check size compatibility with `core`.
const _: () = assert!(
- core::mem::size_of::<$name>() == core::mem::size_of::<core::ffi::$name>()
+ ::core::mem::size_of::<$name>() == ::core::mem::size_of::<::core::ffi::$name>()
);
)*}
}
diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs
index db2d9658ba47..2b19a8597158 100644
--- a/rust/kernel/device.rs
+++ b/rust/kernel/device.rs
@@ -214,7 +214,7 @@ unsafe impl Sync for Device {}
macro_rules! dev_printk {
($method:ident, $dev:expr, $($f:tt)*) => {
{
- ($dev).$method(core::format_args!($($f)*));
+ ($dev).$method(::core::format_args!($($f)*));
}
}
}
diff --git a/rust/kernel/device_id.rs b/rust/kernel/device_id.rs
index e5859217a579..0a4eb56d98f2 100644
--- a/rust/kernel/device_id.rs
+++ b/rust/kernel/device_id.rs
@@ -159,7 +159,7 @@ macro_rules! module_device_table {
"_", line!(),
"_", stringify!($table_name))
]
- static $module_table_name: [core::mem::MaybeUninit<u8>; $table_name.raw_ids().size()] =
- unsafe { core::mem::transmute_copy($table_name.raw_ids()) };
+ static $module_table_name: [::core::mem::MaybeUninit<u8>; $table_name.raw_ids().size()] =
+ unsafe { ::core::mem::transmute_copy($table_name.raw_ids()) };
};
}
diff --git a/rust/kernel/kunit.rs b/rust/kernel/kunit.rs
index 1604fb6a5b1b..81833a687b75 100644
--- a/rust/kernel/kunit.rs
+++ b/rust/kernel/kunit.rs
@@ -59,7 +59,7 @@ macro_rules! kunit_assert {
}
static FILE: &'static $crate::str::CStr = $crate::c_str!($file);
- static LINE: i32 = core::line!() as i32 - $diff;
+ static LINE: i32 = ::core::line!() as i32 - $diff;
static CONDITION: &'static $crate::str::CStr = $crate::c_str!(stringify!($condition));
// SAFETY: FFI call without safety requirements.
@@ -130,11 +130,11 @@ unsafe impl Sync for UnaryAssert {}
unsafe {
$crate::bindings::__kunit_do_failed_assertion(
kunit_test,
- core::ptr::addr_of!(LOCATION.0),
+ ::core::ptr::addr_of!(LOCATION.0),
$crate::bindings::kunit_assert_type_KUNIT_ASSERTION,
- core::ptr::addr_of!(ASSERTION.0.assert),
+ ::core::ptr::addr_of!(ASSERTION.0.assert),
Some($crate::bindings::kunit_unary_assert_format),
- core::ptr::null(),
+ ::core::ptr::null(),
);
}
diff --git a/rust/kernel/static_assert.rs b/rust/kernel/static_assert.rs
index 3115ee0ba8e9..f3f8c6eb7360 100644
--- a/rust/kernel/static_assert.rs
+++ b/rust/kernel/static_assert.rs
@@ -16,7 +16,7 @@
///
/// ```
/// static_assert!(42 > 24);
-/// static_assert!(core::mem::size_of::<u8>() == 1);
+/// static_assert!(::core::mem::size_of::<u8>() == 1);
///
/// const X: &[u8] = b"bar";
/// static_assert!(X[1] == b'a');
@@ -29,6 +29,6 @@
#[macro_export]
macro_rules! static_assert {
($condition:expr) => {
- const _: () = core::assert!($condition);
+ const _: () = ::core::assert!($condition);
};
}
diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
index 878111cb77bc..aed7b2617c32 100644
--- a/rust/kernel/str.rs
+++ b/rust/kernel/str.rs
@@ -595,7 +595,7 @@ fn deref(&self) -> &str {
macro_rules! format {
($($f:tt)*) => ({
- &*String::from_fmt(kernel::fmt!($($f)*))
+ &*String::from_fmt(::kernel::fmt!($($f)*))
})
}
@@ -944,5 +944,5 @@ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
/// A convenience alias for [`core::format_args`].
#[macro_export]
macro_rules! fmt {
- ($($f:tt)*) => ( core::format_args!($($f)*) )
+ ($($f:tt)*) => ( ::core::format_args!($($f)*) )
}
diff --git a/rust/macros/kunit.rs b/rust/macros/kunit.rs
index 4f553ecf40c0..61fc25228e72 100644
--- a/rust/macros/kunit.rs
+++ b/rust/macros/kunit.rs
@@ -88,29 +88,29 @@ pub(crate) fn kunit_tests(attr: TokenStream, ts: TokenStream) -> TokenStream {
// Looks like:
//
// ```
- // unsafe extern "C" fn kunit_rust_wrapper_foo(_test: *mut kernel::bindings::kunit) { foo(); }
- // unsafe extern "C" fn kunit_rust_wrapper_bar(_test: *mut kernel::bindings::kunit) { bar(); }
+ // unsafe extern "C" fn kunit_rust_wrapper_foo(_test: *mut ::kernel::bindings::kunit) { foo(); }
+ // unsafe extern "C" fn kunit_rust_wrapper_bar(_test: *mut ::kernel::bindings::kunit) { bar(); }
//
- // static mut TEST_CASES: [kernel::bindings::kunit_case; 3] = [
- // kernel::kunit::kunit_case(kernel::c_str!("foo"), kunit_rust_wrapper_foo),
- // kernel::kunit::kunit_case(kernel::c_str!("bar"), kunit_rust_wrapper_bar),
- // kernel::kunit::kunit_case_null(),
+ // static mut TEST_CASES: [::kernel::bindings::kunit_case; 3] = [
+ // ::kernel::kunit::kunit_case(::kernel::c_str!("foo"), kunit_rust_wrapper_foo),
+ // ::kernel::kunit::kunit_case(::kernel::c_str!("bar"), kunit_rust_wrapper_bar),
+ // ::kernel::kunit::kunit_case_null(),
// ];
//
- // kernel::kunit_unsafe_test_suite!(kunit_test_suit_name, TEST_CASES);
+ // ::kernel::kunit_unsafe_test_suite!(kunit_test_suit_name, TEST_CASES);
// ```
let mut kunit_macros = "".to_owned();
let mut test_cases = "".to_owned();
for test in &tests {
let kunit_wrapper_fn_name = format!("kunit_rust_wrapper_{}", test);
let kunit_wrapper = format!(
- "unsafe extern \"C\" fn {}(_test: *mut kernel::bindings::kunit) {{ {}(); }}",
+ "unsafe extern \"C\" fn {}(_test: *mut ::kernel::bindings::kunit) {{ {}(); }}",
kunit_wrapper_fn_name, test
);
writeln!(kunit_macros, "{kunit_wrapper}").unwrap();
writeln!(
test_cases,
- " kernel::kunit::kunit_case(kernel::c_str!(\"{}\"), {}),",
+ " ::kernel::kunit::kunit_case(::kernel::c_str!(\"{}\"), {}),",
test, kunit_wrapper_fn_name
)
.unwrap();
@@ -119,14 +119,14 @@ pub(crate) fn kunit_tests(attr: TokenStream, ts: TokenStream) -> TokenStream {
writeln!(kunit_macros).unwrap();
writeln!(
kunit_macros,
- "static mut TEST_CASES: [kernel::bindings::kunit_case; {}] = [\n{test_cases} kernel::kunit::kunit_case_null(),\n];",
+ "static mut TEST_CASES: [::kernel::bindings::kunit_case; {}] = [\n{test_cases} ::kernel::kunit::kunit_case_null(),\n];",
tests.len() + 1
)
.unwrap();
writeln!(
kunit_macros,
- "kernel::kunit_unsafe_test_suite!({attr}, TEST_CASES);"
+ "::kernel::kunit_unsafe_test_suite!({attr}, TEST_CASES);"
)
.unwrap();
diff --git a/rust/macros/lib.rs b/rust/macros/lib.rs
index 9acaa68c974e..cb429eceff71 100644
--- a/rust/macros/lib.rs
+++ b/rust/macros/lib.rs
@@ -283,7 +283,7 @@ pub fn concat_idents(ts: TokenStream) -> TokenStream {
/// # const binder_driver_return_protocol_BR_FAILED_REPLY: u32 = 14;
/// macro_rules! pub_no_prefix {
/// ($prefix:ident, $($newname:ident),+) => {
-/// kernel::macros::paste! {
+/// ::kernel::macros::paste! {
/// $(pub(crate) const $newname: u32 = [<$prefix $newname>];)+
/// }
/// };
@@ -340,7 +340,7 @@ pub fn concat_idents(ts: TokenStream) -> TokenStream {
/// # const binder_driver_return_protocol_BR_FAILED_REPLY: u32 = 14;
/// macro_rules! pub_no_prefix {
/// ($prefix:ident, $($newname:ident),+) => {
-/// kernel::macros::paste! {
+/// ::kernel::macros::paste! {
/// $(pub(crate) const fn [<$newname:lower:span>]() -> u32 { [<$prefix $newname:span>] })+
/// }
/// };
@@ -375,7 +375,7 @@ pub fn concat_idents(ts: TokenStream) -> TokenStream {
/// ```
/// macro_rules! create_numbered_fn {
/// ($name:literal, $val:literal) => {
-/// kernel::macros::paste! {
+/// ::kernel::macros::paste! {
/// fn [<some_ $name _fn $val>]() -> u32 { $val }
/// }
/// };
diff --git a/rust/macros/module.rs b/rust/macros/module.rs
index 8ab4e1f3eb45..615cefd4160a 100644
--- a/rust/macros/module.rs
+++ b/rust/macros/module.rs
@@ -224,20 +224,20 @@ pub(crate) fn module(ts: TokenStream) -> TokenStream {
// SAFETY: `__this_module` is constructed by the kernel at load time and will not be
// freed until the module is unloaded.
#[cfg(MODULE)]
- static THIS_MODULE: kernel::ThisModule = unsafe {{
+ static THIS_MODULE: ::kernel::ThisModule = unsafe {{
extern \"C\" {{
- static __this_module: kernel::types::Opaque<kernel::bindings::module>;
+ static __this_module: ::kernel::types::Opaque<::kernel::bindings::module>;
}}
- kernel::ThisModule::from_ptr(__this_module.get())
+ ::kernel::ThisModule::from_ptr(__this_module.get())
}};
#[cfg(not(MODULE))]
- static THIS_MODULE: kernel::ThisModule = unsafe {{
- kernel::ThisModule::from_ptr(core::ptr::null_mut())
+ static THIS_MODULE: ::kernel::ThisModule = unsafe {{
+ ::kernel::ThisModule::from_ptr(::core::ptr::null_mut())
}};
- impl kernel::ModuleMetadata for {type_} {{
- const NAME: &'static kernel::str::CStr = kernel::c_str!(\"{name}\");
+ impl ::kernel::ModuleMetadata for {type_} {{
+ const NAME: &'static ::kernel::str::CStr = ::kernel::c_str!(\"{name}\");
}}
// Double nested modules, since then nobody can access the public items inside.
@@ -255,8 +255,8 @@ mod __module_init {{
#[used]
static __IS_RUST_MODULE: () = ();
- static mut __MOD: core::mem::MaybeUninit<{type_}> =
- core::mem::MaybeUninit::uninit();
+ static mut __MOD: ::core::mem::MaybeUninit<{type_}> =
+ ::core::mem::MaybeUninit::uninit();
// Loadable modules need to export the `{{init,cleanup}}_module` identifiers.
/// # Safety
@@ -267,7 +267,7 @@ mod __module_init {{
#[doc(hidden)]
#[no_mangle]
#[link_section = \".init.text\"]
- pub unsafe extern \"C\" fn init_module() -> kernel::ffi::c_int {{
+ pub unsafe extern \"C\" fn init_module() -> ::kernel::ffi::c_int {{
// SAFETY: This function is inaccessible to the outside due to the double
// module wrapping it. It is called exactly once by the C side via its
// unique name.
@@ -306,11 +306,12 @@ mod __module_init {{
#[doc(hidden)]
#[link_section = \"{initcall_section}\"]
#[used]
- pub static __{name}_initcall: extern \"C\" fn() -> kernel::ffi::c_int = __{name}_init;
+ pub static __{name}_initcall: extern \"C\" fn() -> ::kernel::ffi::c_int =
+ __{name}_init;
#[cfg(not(MODULE))]
#[cfg(CONFIG_HAVE_ARCH_PREL32_RELOCATIONS)]
- core::arch::global_asm!(
+ ::core::arch::global_asm!(
r#\".section \"{initcall_section}\", \"a\"
__{name}_initcall:
.long __{name}_init - .
@@ -321,7 +322,7 @@ mod __module_init {{
#[cfg(not(MODULE))]
#[doc(hidden)]
#[no_mangle]
- pub extern \"C\" fn __{name}_init() -> kernel::ffi::c_int {{
+ pub extern \"C\" fn __{name}_init() -> ::kernel::ffi::c_int {{
// SAFETY: This function is inaccessible to the outside due to the double
// module wrapping it. It is called exactly once by the C side via its
// placement above in the initcall section.
@@ -344,9 +345,9 @@ mod __module_init {{
/// # Safety
///
/// This function must only be called once.
- unsafe fn __init() -> kernel::ffi::c_int {{
+ unsafe fn __init() -> ::kernel::ffi::c_int {{
let initer =
- <{type_} as kernel::InPlaceModule>::init(&super::super::THIS_MODULE);
+ <{type_} as ::kernel::InPlaceModule>::init(&super::super::THIS_MODULE);
// SAFETY: No data race, since `__MOD` can only be accessed by this module
// and there only `__init` and `__exit` access it. These functions are only
// called once and `__exit` cannot be called before or during `__init`.
diff --git a/scripts/rustdoc_test_builder.rs b/scripts/rustdoc_test_builder.rs
index e5894652f12c..81233fb56b66 100644
--- a/scripts/rustdoc_test_builder.rs
+++ b/scripts/rustdoc_test_builder.rs
@@ -28,7 +28,7 @@ fn main() {
//
// ```
// fn main() { #[allow(non_snake_case)] fn _doctest_main_rust_kernel_file_rs_28_0() {
- // fn main() { #[allow(non_snake_case)] fn _doctest_main_rust_kernel_file_rs_37_0() -> Result<(), impl core::fmt::Debug> {
+ // fn main() { #[allow(non_snake_case)] fn _doctest_main_rust_kernel_file_rs_37_0() -> Result<(), impl ::core::fmt::Debug> {
// ```
//
// It should be unlikely that doctest code matches such lines (when code is formatted properly).
@@ -49,8 +49,8 @@ fn main() {
// Qualify `Result` to avoid the collision with our own `Result` coming from the prelude.
let body = body.replace(
- &format!("{rustdoc_function_name}() -> Result<(), impl core::fmt::Debug> {{"),
- &format!("{rustdoc_function_name}() -> core::result::Result<(), impl core::fmt::Debug> {{"),
+ &format!("{rustdoc_function_name}() -> Result<(), impl ::core::fmt::Debug> {{"),
+ &format!("{rustdoc_function_name}() -> ::core::result::Result<(), impl ::core::fmt::Debug> {{"),
);
// For tests that get generated with `Result`, like above, `rustdoc` generates an `unwrap()` on
diff --git a/scripts/rustdoc_test_gen.rs b/scripts/rustdoc_test_gen.rs
index 036635fb1621..f8d053f75270 100644
--- a/scripts/rustdoc_test_gen.rs
+++ b/scripts/rustdoc_test_gen.rs
@@ -167,12 +167,14 @@ fn main() {
rust_tests,
r#"/// Generated `{name}` KUnit test case from a Rust documentation test.
#[no_mangle]
-pub extern "C" fn {kunit_name}(__kunit_test: *mut kernel::bindings::kunit) {{
+pub extern "C" fn {kunit_name}(__kunit_test: *mut ::kernel::bindings::kunit) {{
/// Overrides the usual [`assert!`] macro with one that calls KUnit instead.
#[allow(unused)]
macro_rules! assert {{
($cond:expr $(,)?) => {{{{
- kernel::kunit_assert!("{kunit_name}", "{real_path}", __DOCTEST_ANCHOR - {line}, $cond);
+ ::kernel::kunit_assert!(
+ "{kunit_name}", "{real_path}", __DOCTEST_ANCHOR - {line}, $cond
+ );
}}}}
}}
@@ -180,13 +182,15 @@ macro_rules! assert {{
#[allow(unused)]
macro_rules! assert_eq {{
($left:expr, $right:expr $(,)?) => {{{{
- kernel::kunit_assert_eq!("{kunit_name}", "{real_path}", __DOCTEST_ANCHOR - {line}, $left, $right);
+ ::kernel::kunit_assert_eq!(
+ "{kunit_name}", "{real_path}", __DOCTEST_ANCHOR - {line}, $left, $right
+ );
}}}}
}}
// Many tests need the prelude, so provide it by default.
#[allow(unused)]
- use kernel::prelude::*;
+ use ::kernel::prelude::*;
// Unconditionally print the location of the original doctest (i.e. rather than the location in
// the generated file) so that developers can easily map the test back to the source code.
@@ -197,11 +201,11 @@ macro_rules! assert_eq {{
// This follows the syntax for declaring test metadata in the proposed KTAP v2 spec, which may
// be used for the proposed KUnit test attributes API. Thus hopefully this will make migration
// easier later on.
- kernel::kunit::info(format_args!(" # {kunit_name}.location: {real_path}:{line}\n"));
+ ::kernel::kunit::info(format_args!(" # {kunit_name}.location: {real_path}:{line}\n"));
/// The anchor where the test code body starts.
#[allow(unused)]
- static __DOCTEST_ANCHOR: i32 = core::line!() as i32 + {body_offset} + 1;
+ static __DOCTEST_ANCHOR: i32 = ::core::line!() as i32 + {body_offset} + 1;
{{
{body}
main();
--
2.43.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 0/1] rust: macros: Fix macro referencing core and kernel crates v2
2025-03-28 18:03 ` [PATCH v2 0/1] rust: macros: Fix macro referencing core and kernel crates v2 Igor Korotin
2025-03-28 18:03 ` [PATCH v2 1/1] " Igor Korotin
@ 2025-03-28 21:48 ` Benno Lossin
1 sibling, 0 replies; 6+ messages in thread
From: Benno Lossin @ 2025-03-28 21:48 UTC (permalink / raw)
To: Igor Korotin; +Cc: linux-kernel, rust-for-linux
This email seems to be a cover letter for the next version of your
patch. In that case, you should send it as its own thread to the list
(and not in reply to my previous email).
On Fri Mar 28, 2025 at 7:03 PM CET, Igor Korotin wrote:
>> > To prevent issues where user-defined modules named `core` or `kernel`
>> > could be picked up instead of the standard ones, update macros to use
>> > absolute paths, `::core::...` and `::kernel::...`, for core and kernel
>> > references.
>>
>> Please split this sentence up into smaller parts. Also "standard ones"
>> sounds weird to me, `core` and `kernel` are crates and not modules.
>
> Simplified the commit message.
You don't need to reply if you're fine with the suggestion :)
>> > Suggested-by: y86-dev <y86-dev@protonmail.com>
>>
>> Please use my real name and other mail address instead:
>>
>> Benno Lossin <benno.lossin@proton.me>
>
> Sure. Tried to find your creds via your github profile/commits without
> success.
>
>> > Closes: https://github.com/Rust-for-Linux/linux/issues/1150
>> > Signed-off-by: Igor Korotin <igor.korotin@yahoo.com>
>> > ---
>> > rust/ffi.rs | 2 +-
>> > rust/kernel/device.rs | 2 +-
>> > rust/kernel/device_id.rs | 4 ++--
>> > rust/kernel/kunit.rs | 8 ++++----
>> > rust/kernel/static_assert.rs | 4 ++--
>> > rust/kernel/str.rs | 4 ++--
>> > rust/macros/lib.rs | 6 +++---
>> > scripts/rustdoc_test_gen.rs | 8 ++++++--
>> > 8 files changed, 21 insertions(+), 17 deletions(-)
>>
>> There are also some occurrences of `kernel::` & `core::` in proc macros,
>> for example:
>> - rust/macros/kunit.rs:129
>> - rust/macros/module.rs:347
>> - ...
>>
>> Could you also add those to the patch? Thanks!
>>
>> (line numbers after applying your patch to rust-next)
>
> Agreed. Fixed references in auto-generated code in files
> 1. rust/macros/kunit.rs
> 2. rust/macros/module.rs
> 3. scripts/rustdoc_test_builder.rs
> 4. scripts/rustdoc_test_gen.rs
Yeah those last two are also good finds.
---
Cheers,
Benno
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/1] rust: macros: Fix macro referencing core and kernel crates v2
2025-03-28 18:03 ` [PATCH v2 1/1] " Igor Korotin
@ 2025-03-28 21:50 ` Benno Lossin
2025-03-29 12:28 ` Miguel Ojeda
1 sibling, 0 replies; 6+ messages in thread
From: Benno Lossin @ 2025-03-28 21:50 UTC (permalink / raw)
To: Igor Korotin; +Cc: linux-kernel, rust-for-linux
On Fri Mar 28, 2025 at 7:03 PM CET, Igor Korotin wrote:
> Update macros to always use absolute paths for crates `core` and `kernel`.
> This guarantees that macros will not pick up user-defined crates `core`
> or `kernel` by accident.
The crates are not user-defineable. Your initial message was correct in
that the user can define *modules* that have precedence over the crates
`core` and `kernel`.
> Changes since v1:
> - Fixed paths in auto-generated code.
The changelog shouldn't be in the commit message. Instead place it below
the `---`, but before the change stats.
> Suggested-by: Benno Lossin <benno.lossin@proton.me>
> Closes: https://github.com/Rust-for-Linux/linux/issues/1150
> Signed-off-by: Igor Korotin <igor.korotin@yahoo.com>
> ---
(put the changelog here)
---
Cheers,
Benno
> rust/ffi.rs | 2 +-
> rust/kernel/device.rs | 2 +-
> rust/kernel/device_id.rs | 4 ++--
> rust/kernel/kunit.rs | 8 ++++----
> rust/kernel/static_assert.rs | 4 ++--
> rust/kernel/str.rs | 4 ++--
> rust/macros/kunit.rs | 22 +++++++++++-----------
> rust/macros/lib.rs | 6 +++---
> rust/macros/module.rs | 31 ++++++++++++++++---------------
> scripts/rustdoc_test_builder.rs | 6 +++---
> scripts/rustdoc_test_gen.rs | 16 ++++++++++------
> 11 files changed, 55 insertions(+), 50 deletions(-)
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/1] rust: macros: Fix macro referencing core and kernel crates v2
2025-03-28 18:03 ` [PATCH v2 1/1] " Igor Korotin
2025-03-28 21:50 ` Benno Lossin
@ 2025-03-29 12:28 ` Miguel Ojeda
1 sibling, 0 replies; 6+ messages in thread
From: Miguel Ojeda @ 2025-03-29 12:28 UTC (permalink / raw)
To: Igor Korotin; +Cc: benno.lossin, linux-kernel, rust-for-linux
On Fri, Mar 28, 2025 at 7:05 PM Igor Korotin <igor.korotin@yahoo.com> wrote:
>
> Update macros to always use absolute paths for crates `core` and `kernel`.
> This guarantees that macros will not pick up user-defined crates `core`
> or `kernel` by accident.
>
> Changes since v1:
> - Fixed paths in auto-generated code.
>
> Suggested-by: Benno Lossin <benno.lossin@proton.me>
> Closes: https://github.com/Rust-for-Linux/linux/issues/1150
> Signed-off-by: Igor Korotin <igor.korotin@yahoo.com>
Thanks for the patch!
A few procedural notes:
- The title has a v2 at the end -- please do not add versions in the
title of the commit, the "[PATCH v2...] is enough for that :)
- The "Changes since v1" should be part of the non-commit-log part,
i.e. after the `---` line below.
- If you send a single patch, then you don't need to add a cover
letter necessarily (typically you just use the place after `---` in
the patch).
- Speaking of the cover letter, it seems like the 0/1 patch looks
like a reply to Benno rather than an actual cover letter, and does not
follow the usual cover letter format. That is quite confusing.
- You should send the patch to the relevant maintainers and
reviewers -- you can use `scripts/get_maintainer.pl` or something like
https://rust-for-linux.com/contributing#submitting-patches.
Please take a look at another v2 to see how people normally do it. For
instance, this recent one looks fairly standard:
https://lore.kernel.org/rust-for-linux/20250325133352.441425-1-andrewjballance@gmail.com/
Thanks!
Cheers,
Miguel
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-03-29 12:28 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-03-28 0:49 [PATCH] rust: macros: Fix macro referencing core and kernel crates Benno Lossin
2025-03-28 18:03 ` [PATCH v2 0/1] rust: macros: Fix macro referencing core and kernel crates v2 Igor Korotin
2025-03-28 18:03 ` [PATCH v2 1/1] " Igor Korotin
2025-03-28 21:50 ` Benno Lossin
2025-03-29 12:28 ` Miguel Ojeda
2025-03-28 21:48 ` [PATCH v2 0/1] " Benno Lossin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®