From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from vps0.lunn.ch (vps0.lunn.ch [156.67.10.101]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id F3F1B2AF19; Thu, 3 Jul 2025 21:28:48 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=156.67.10.101 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1751578131; cv=none; b=oJNUjWpHI5H3OsUODtQ4BZjorF0w4Dw2SfGwtD4B+TOVp+7QnMSkujFF3r3oWsv0ctLIuqMP9YwBiO/OTfyZxT05kXbEAUwj+4frTLBLg/kkvPP/wgSZpnWLZmLOR/6kdci7QQ20QQwuhbsL8DI3Ll3sbrY252BpcFROAzzlA8g= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1751578131; c=relaxed/simple; bh=7s1vdVb4Jp0AKYi0fqgt4BTeqCIZhFaWic60VjgU5TU=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=GHB5t5XzldEmrSlyUzdCzN5OmzCJIS5T3FdIn0uTbHwi+calt1iaanfpHJVlhnmTyTUkSMxGXU2iMgy/Qy5MbLWP6SXR3RluJU2NlnbQdYsJ81Qk20k0HiPd7wRKxVfTqhD6oWX3YyniAANFhC7+0y3+JtkJO7Nat1kcEUsXAYo= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=lunn.ch; spf=pass smtp.mailfrom=lunn.ch; dkim=pass (1024-bit key) header.d=lunn.ch header.i=@lunn.ch header.b=Nas0qbwd; arc=none smtp.client-ip=156.67.10.101 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=lunn.ch Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=lunn.ch Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=lunn.ch header.i=@lunn.ch header.b="Nas0qbwd" DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lunn.ch; s=20171124; h=In-Reply-To:Content-Transfer-Encoding:Content-Disposition: Content-Type:MIME-Version:References:Message-ID:Subject:Cc:To:From:Date:From: Sender:Reply-To:Subject:Date:Message-ID:To:Cc:MIME-Version:Content-Type: Content-Transfer-Encoding:Content-ID:Content-Description:Content-Disposition: In-Reply-To:References; bh=A83NKRGoevWzL/NcvqijuYTaZ0xDTVIZ9pvu0pc07Jk=; b=Na s0qbwdL6B67K0wS+f7tbNlfneHHctNev/cTniYFhLMkwezOAunGEDatFtpLb4g+PG761JVmPtbDzC rfpydruVCCrDayQ33cjSGl/o6BiA+VGkuX6ILQmO0/nR1G4/Ap/W5RcWrBNuhbzqrUhTrrpdbnIlt YvmVDe7keP8zJfo=; Received: from andrew by vps0.lunn.ch with local (Exim 4.94.2) (envelope-from ) id 1uXRTN-0008hX-2p; Thu, 03 Jul 2025 23:28:01 +0200 Date: Thu, 3 Jul 2025 23:28:01 +0200 From: Andrew Lunn To: Tamir Duberstein Cc: Benno Lossin , Michal Rostecki , Miguel Ojeda , Alex Gaynor , Boqun Feng , Gary Guo , =?iso-8859-1?Q?Bj=F6rn?= Roy Baron , Andreas Hindborg , Alice Ryhl , Trevor Gross , Brendan Higgins , David Gow , Rae Moar , Danilo Krummrich , Maarten Lankhorst , Maxime Ripard , Thomas Zimmermann , David Airlie , Simona Vetter , Greg Kroah-Hartman , "Rafael J. Wysocki" , Luis Chamberlain , Russ Weight , FUJITA Tomonori , Rob Herring , Saravana Kannan , Peter Zijlstra , Ingo Molnar , Will Deacon , Waiman Long , Nathan Chancellor , Nick Desaulniers , Bill Wendling , Justin Stitt , Heiner Kallweit , Russell King , "David S. Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , Bjorn Helgaas , Arnd Bergmann , Jens Axboe , Krzysztof =?utf-8?Q?Wilczy=C5=84ski?= , Dave Ertman , Ira Weiny , Leon Romanovsky , Breno Leitao , Viresh Kumar , Michael Turquette , Stephen Boyd , rust-for-linux@vger.kernel.org, linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org, kunit-dev@googlegroups.com, dri-devel@lists.freedesktop.org, netdev@vger.kernel.org, devicetree@vger.kernel.org, llvm@lists.linux.dev, linux-pci@vger.kernel.org, nouveau@lists.freedesktop.org, linux-block@vger.kernel.org, linux-pm@vger.kernel.org, linux-clk@vger.kernel.org Subject: Re: [PATCH v13 2/5] rust: support formatting of foreign types Message-ID: <34c00dfa-8302-45ee-8d80-58b97a08e52e@lunn.ch> References: <20250701-cstr-core-v13-0-29f7d3eb97a6@gmail.com> <20250701-cstr-core-v13-2-29f7d3eb97a6@gmail.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: On Thu, Jul 03, 2025 at 02:55:30PM -0400, Tamir Duberstein wrote: > On Thu, Jul 3, 2025 at 11:08 AM Benno Lossin 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 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 > > >> > Link: https://rust-for-linux.zulipchat.com/#narrow/channel/288089-General/topic/Custom.20formatting/with/516476467 > > >> > Acked-by: Greg Kroah-Hartman > > >> > Reviewed-by: Alice Ryhl > > >> > Signed-off-by: Tamir Duberstein > > >> > --- > > >> > 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