All of lore.kernel.org
 help / color / mirror / Atom feed
From: Danilo Krummrich <dakr@kernel.org>
To: Alice Ryhl <aliceryhl@google.com>
Cc: "Miguel Ojeda" <ojeda@kernel.org>,
	"Alex Gaynor" <alex.gaynor@gmail.com>,
	"Boqun Feng" <boqun.feng@gmail.com>,
	"Gary Guo" <gary@garyguo.net>,
	"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
	"Benno Lossin" <benno.lossin@proton.me>,
	"Andreas Hindborg" <a.hindborg@kernel.org>,
	"Trevor Gross" <tmgross@umich.edu>,
	rust-for-linux@vger.kernel.org, linux-kernel@vger.kernel.org,
	patches@lists.linux.dev
Subject: Re: [PATCH] rust: add C FFI types to the prelude
Date: Mon, 14 Apr 2025 13:58:49 +0200	[thread overview]
Message-ID: <Z_z4eaYwADrHTVLW@cassiopeiae> (raw)
In-Reply-To: <CAH5fLgiF0kZdOYaQi18_LTNx6Onq-5PBomA=bwS0m0z+C0wtvg@mail.gmail.com>

On Mon, Apr 14, 2025 at 10:46:46AM +0200, Alice Ryhl wrote:
> On Sun, Apr 13, 2025 at 2:57 AM Miguel Ojeda <ojeda@kernel.org> wrote:
> >
> > Rust kernel code is supposed to use the custom mapping of C FFI types,
> > i.e. those from the `ffi` crate, rather than the ones coming from `core`.
> >
> > Thus, to minimize mistakes and to simplify the code everywhere, just
> > provide them in the `kernel` prelude and ask in the Coding Guidelines
> > to use them directly, i.e. as a single segment path.
> >
> > After this lands, we can start cleaning up the existing users.
> >
> > Ideally, we would use something like Clippy's `disallowed-types` to
> > prevent the use of the `core` ones, but that one sees through aliases.
> >
> > Link: https://lore.kernel.org/rust-for-linux/CANiq72kc4gzfieD-FjuWfELRDXXD2vLgPv4wqk3nt4pjdPQ=qg@mail.gmail.com/
> > Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
> 
> Yes please!
> 
> >  Documentation/rust/coding-guidelines.rst | 17 +++++++++++++++++
> >  rust/kernel/prelude.rs                   |  5 +++++
> >  2 files changed, 22 insertions(+)
> >
> > diff --git a/Documentation/rust/coding-guidelines.rst b/Documentation/rust/coding-guidelines.rst
> > index 27f2a7bb5a4a..d0bf0b3a058a 100644
> > --- a/Documentation/rust/coding-guidelines.rst
> > +++ b/Documentation/rust/coding-guidelines.rst
> > @@ -191,6 +191,23 @@ or:
> >         /// [`struct mutex`]: srctree/include/linux/mutex.h
> >
> >
> > +C FFI types
> > +-----------
> > +
> > +Rust kernel code does not use the C FFI types (such as ``c_char``) from
> > +``core::ffi::*``. Instead, a custom mapping that matches properly the C types
> > +used in the kernel is provided in the prelude, i.e. ``kernel::prelude::*``.
> > +
> > +These types (aliases) should generally be referred directly by their identifier,
> > +i.e. as a single segment path. For instance:
> > +
> > +.. code-block:: rust
> > +
> > +       fn f(p: *const c_char) -> c_int {
> > +           // ...
> > +       }
> 
> I wonder if it would make more sense to rephrase this section to first
> say that rfl has type aliases for the C integer types called c_int and
> so on, then mention that they are available in the prelude, and then
> at the end of the section have a note that we don't use the type
> aliases from core::ffi. I think focusing on how to use C integer
> types, rather than technical details about how they are defined, is
> more relevant for a reader who is just looking for coding guidelines.

I think that's a good suggestion. With that,

	Reviewed-by: Danilo Krummrich <dakr@kernel.org>

  reply	other threads:[~2025-04-14 11:58 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-04-13  0:56 [PATCH] rust: add C FFI types to the prelude Miguel Ojeda
2025-04-14  8:46 ` Alice Ryhl
2025-04-14 11:58   ` Danilo Krummrich [this message]
2025-04-14 13:22   ` Miguel Ojeda
2025-04-14 14:13     ` Alice Ryhl
2025-05-22 20:05       ` Miguel Ojeda
2025-05-22 20:10         ` Alice Ryhl
2025-05-25 21:12 ` Miguel Ojeda

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Z_z4eaYwADrHTVLW@cassiopeiae \
    --to=dakr@kernel.org \
    --cc=a.hindborg@kernel.org \
    --cc=alex.gaynor@gmail.com \
    --cc=aliceryhl@google.com \
    --cc=benno.lossin@proton.me \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun.feng@gmail.com \
    --cc=gary@garyguo.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=ojeda@kernel.org \
    --cc=patches@lists.linux.dev \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=tmgross@umich.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.