On Thu, Jul 03, 2025 at 02:55:30PM -0400, Tamir Duberstein wrote:
On Thu, Jul 3, 2025 at 11:08 AM Benno Lossin lossin@kernel.org wrote:
On Thu Jul 3, 2025 at 3:55 PM CEST, Tamir Duberstein wrote:
On Thu, Jul 3, 2025 at 5:32 AM Benno Lossin lossin@kernel.org wrote:
On Tue Jul 1, 2025 at 6:49 PM CEST, Tamir Duberstein wrote:
Introduce a `fmt!` macro which wraps all arguments in `kernel::fmt::Adapter` and a `kernel::fmt::Display` trait. This enables formatting of foreign types (like `core::ffi::CStr`) that do not implement `core::fmt::Display` due to concerns around lossy conversions which do not apply in the kernel.
Replace all direct calls to `format_args!` with `fmt!`.
Replace all implementations of `core::fmt::Display` with implementations of `kernel::fmt::Display`.
Suggested-by: Alice Ryhl aliceryhl@google.com Link: https://rust-for-linux.zulipchat.com/#narrow/channel/288089-General/topic/Cu... Acked-by: Greg Kroah-Hartman gregkh@linuxfoundation.org Reviewed-by: Alice Ryhl aliceryhl@google.com Signed-off-by: Tamir Duberstein tamird@gmail.com
drivers/block/rnull.rs | 2 +- drivers/gpu/nova-core/gpu.rs | 4 +- rust/kernel/block/mq.rs | 2 +- rust/kernel/device.rs | 2 +- rust/kernel/fmt.rs | 89 +++++++++++++++++++++++++++++++++++++++ rust/kernel/kunit.rs | 6 +-- rust/kernel/lib.rs | 1 + rust/kernel/prelude.rs | 3 +- rust/kernel/print.rs | 4 +- rust/kernel/seq_file.rs | 2 +- rust/kernel/str.rs | 22 ++++------ rust/macros/fmt.rs | 99 ++++++++++++++++++++++++++++++++++++++++++++ rust/macros/lib.rs | 19 +++++++++ rust/macros/quote.rs | 7 ++++ scripts/rustdoc_test_gen.rs | 2 +- 15 files changed, 236 insertions(+), 28 deletions(-)
This would be a lot easier to review if he proc-macro and the call replacement were different patches.
Also the `kernel/fmt.rs` file should be a different commit.
Can you help me understand why? The changes you ask to be separated would all be in different files, so why would separate commits make it easier to review?
It takes less time to go through the entire patch and give a RB. I can take smaller time chunks and don't have to get back into the entire context of the patch when I don't have 30-60min available.
Ah, I see what you mean. Yeah, the requirement to RB the entire patch does mean there's a benefit to smaller patches.
I often tell kernel newbies:
Lots of small patches which are obviously correct.
A small patch tends to be more obviously correct than a big patch. The commit message is more focused and helpful because it refers to a small chunk of code. Because the commit message is more focused, it can answer questions reviewers might ask, before they ask them. If i can spend 60 seconds looking at a patch and decide it looks correct, i'm more likely to actually look at it and give a reviewed by. If i need to find 10 minutes, it is going to get put off for a later time. Many reviewers just have a few minutes here, a few there, slotted into time between other tasks, while drinking coffee, etc.
Andrew