Message ID | 20230614115328.2825961-1-aliceryhl@google.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:994d:0:b0:3d9:f83d:47d9 with SMTP id k13csp1187403vqr; Wed, 14 Jun 2023 04:59:31 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ5PyeAlU7NPq54K2Z2nlSqaInqxwL32lBxk2Y0RIEmhFipJt22p5NBaf0soeVNlG0Yc8yrg X-Received: by 2002:a17:907:1c23:b0:965:7fba:6bcf with SMTP id nc35-20020a1709071c2300b009657fba6bcfmr14491051ejc.67.1686743971323; Wed, 14 Jun 2023 04:59:31 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1686743971; cv=none; d=google.com; s=arc-20160816; b=WtOl7vQqZMAw1FzfQGP4Q7PvviL38LvE4ehMAFLOBstfOEq3woXHfLPBkSidlsUNaD PU8SwfNSmVjMCR9ebIEKI7qKoC0J2EpkIMQ7DT1Cxp+15oWuuL2ScLIvsSu53fvM9Dpy PCgTOijJRMzvAFXFN9kbuGmkwJWm5SIyYL5L2ArYrFbZ5cAx0AP3ys2VqGSHn4jgWbvN 7FHtE+h9LtvH/BVVqCwZM1Y7+f1BWeXyzv4Ta8vYED/ik/KB92eC96YITE/CuN9He3PS B/uv2v36tbBCltHXetQnGXcKDMGo/A+ivnmgcwwqGadBEL7J90X9wa0yN+x0RLCKb7CL +3zw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:cc:to:from:subject:message-id:mime-version:date :dkim-signature; bh=iQcKjMXMsnWztaoMR9yA4alqbcESKo/Ty0nyLCc3lHY=; b=E9fzp3sE7KdbZU+rGqIGlxJDZvQKC65mbRT9959I2WrpTeaHXVO+vEpjmz617BMIuS hmfB81fCgtyG2wP7WgOCLYH5vtLif2N+aCxBm6teKwFwE/ZCS0GEUSfbItZopDwksDz1 X2ZhNdSwX2w/hdA+vNcHnFW4Hn90Envvgd3Kiq4YMyYMV9baD4EhImokmqwvMpqVqXBZ E2AkWHoxqnmQdTbpUy59ZeosPhAtZNWdh1jy5Okvw9G/IngM4XZEVpWuSdS/z4SKFuI9 F43dKcdAc1r7KhFhjeChho9ZQIbNTJz62Gg6KEz8h1IGgbpw2cuFcUqVWJZjxZlIx9oT 7M7A== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@google.com header.s=20221208 header.b="P/0dT32m"; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=google.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id y23-20020a17090668d700b0097890ca35e8si8138112ejr.862.2023.06.14.04.59.06; Wed, 14 Jun 2023 04:59:31 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@google.com header.s=20221208 header.b="P/0dT32m"; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=google.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S244305AbjFNLxo (ORCPT <rfc822;n2h9z4@gmail.com> + 99 others); Wed, 14 Jun 2023 07:53:44 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:34444 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S236151AbjFNLxm (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Wed, 14 Jun 2023 07:53:42 -0400 Received: from mail-yb1-xb4a.google.com (mail-yb1-xb4a.google.com [IPv6:2607:f8b0:4864:20::b4a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 2A2FD1BF9 for <linux-kernel@vger.kernel.org>; Wed, 14 Jun 2023 04:53:37 -0700 (PDT) Received: by mail-yb1-xb4a.google.com with SMTP id 3f1490d57ef6-bc8ea14f4eeso757904276.0 for <linux-kernel@vger.kernel.org>; Wed, 14 Jun 2023 04:53:37 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20221208; t=1686743616; x=1689335616; h=cc:to:from:subject:message-id:mime-version:date:from:to:cc:subject :date:message-id:reply-to; bh=iQcKjMXMsnWztaoMR9yA4alqbcESKo/Ty0nyLCc3lHY=; b=P/0dT32mEmx5CRm6KyIkY8XYLtJCudbxoZS8fwkFdmZ7FMZ4gNRw1+wgP465gUIN42 LdIpWmYzaXg3CI1XlaE4maNokY3KosI1rhodoBW1i9esHOuyKU6hAdVgZxmvkeK5ZDe9 aLJ9E7GES+I4AlHLDcNtJNj4K0AllRunwIQ9tqSlsSCb8yOCdqpgZgtpSsfxZURxkCOW +tQPNlr1bkJ2jdG3tZqf0IWPKabg/xWJ0mH6IspxYJyWXft9mOT1ou3m9ewsZ3VAUNwC Exrhm9dNNTMrUzhFcy0uEdc+f9CEHHQo3at96Cp0neLyBwDwfv6PJsN0aOxfCVncW+0k acsg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1686743616; x=1689335616; h=cc:to:from:subject:message-id:mime-version:date:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=iQcKjMXMsnWztaoMR9yA4alqbcESKo/Ty0nyLCc3lHY=; b=bkuBlsuf2swu/UDYwYIjjswnKQ/9sOGaP41/3a2zS3H7yi8e/ZBXDdZFCt2k+LY/w6 S9PyTs0ZEzj+1T3LtzVEVZ/xroTnUk5Y1zsP0Z2GdISbBIV+R0CIghhPlx/NlNUZKy3B D0OKHSV1Ndo1Tnmb07X0qXOuch4tamr9q1KRzYrGs9gDeH9asmbY3Vw4iHcU4apGfzM3 WMjVUCptmE6Mx0fkzvnuJc+fK+hUb5sE5jPJr7EQkhv+bWRsu8kwNAgWiV3x9FcVRP7R FOLsMcJCUmUmLaKA8mby0a4KDXNkZxFuLdM76Q/QnmJmuxKnQ/mS+h1BKp+2y6YKcX1G X2dA== X-Gm-Message-State: AC+VfDzD5NaLm898cTlDwJBBkRugPvwnsp6wrMDOcPsQmvIMP/ZinKhg fL6hAfEaJ1tGnxyEOsFNOjZBf25msT5hRPE= X-Received: from aliceryhl.c.googlers.com ([fda3:e722:ac3:cc00:31:98fb:c0a8:6c8]) (user=aliceryhl job=sendgmr) by 2002:a25:d854:0:b0:bc4:8939:e1f5 with SMTP id p81-20020a25d854000000b00bc48939e1f5mr909934ybg.4.1686743616324; Wed, 14 Jun 2023 04:53:36 -0700 (PDT) Date: Wed, 14 Jun 2023 11:53:28 +0000 Mime-Version: 1.0 X-Mailer: git-send-email 2.41.0.162.gfafddb0af9-goog Message-ID: <20230614115328.2825961-1-aliceryhl@google.com> Subject: [PATCH] rust: make `UnsafeCell` the outer type in `Opaque` From: Alice Ryhl <aliceryhl@google.com> To: rust-for-linux@vger.kernel.org Cc: Miguel Ojeda <ojeda@kernel.org>, Wedson Almeida Filho <wedsonaf@gmail.com>, Alex Gaynor <alex.gaynor@gmail.com>, Boqun Feng <boqun.feng@gmail.com>, Gary Guo <gary@garyguo.net>, " =?utf-8?q?Bj=C3=B6rn_Roy_Baron?= " <bjorn3_gh@protonmail.com>, Benno Lossin <benno.lossin@proton.me>, Alice Ryhl <aliceryhl@google.com>, linux-kernel@vger.kernel.org, patches@lists.linux.dev Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-9.6 required=5.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE, SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE,URIBL_BLOCKED, USER_IN_DEF_DKIM_WL autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1768679246350468965?= X-GMAIL-MSGID: =?utf-8?q?1768679246350468965?= |
Series |
rust: make `UnsafeCell` the outer type in `Opaque`
|
|
Commit Message
Alice Ryhl
June 14, 2023, 11:53 a.m. UTC
When combining `UnsafeCell` with `MaybeUninit`, it is idiomatic to use
`UnsafeCell` as the outer type. Intuitively, this is because a
`MaybeUninit<T>` might not contain a `T`, but we always want the effect
of the `UnsafeCell`, even if the inner value is uninitialized.
Now, strictly speaking, this doesn't really make a difference. The
compiler will always apply the `UnsafeCell` effect even if the inner
value is uninitialized. But I think we should follow the convention
here.
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
rust/kernel/types.rs | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
base-commit: d2e3115d717197cb2bc020dd1f06b06538474ac3
Comments
On 14.06.23 13:53, Alice Ryhl wrote: > When combining `UnsafeCell` with `MaybeUninit`, it is idiomatic to use > `UnsafeCell` as the outer type. Intuitively, this is because a > `MaybeUninit<T>` might not contain a `T`, but we always want the effect > of the `UnsafeCell`, even if the inner value is uninitialized. > > Now, strictly speaking, this doesn't really make a difference. The > compiler will always apply the `UnsafeCell` effect even if the inner > value is uninitialized. But I think we should follow the convention > here. > > Signed-off-by: Alice Ryhl <aliceryhl@google.com> Small comment below, but I think it is fine the way it is. Reviewed-by: Benno Lossin <benno.lossin@proton.me> > --- > rust/kernel/types.rs | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs > index 1e5380b16ed5..fb41635f1e1f 100644 > --- a/rust/kernel/types.rs > +++ b/rust/kernel/types.rs > @@ -224,17 +224,17 @@ fn drop(&mut self) { > /// > /// This is meant to be used with FFI objects that are never interpreted by Rust code. > #[repr(transparent)] > -pub struct Opaque<T>(MaybeUninit<UnsafeCell<T>>); > +pub struct Opaque<T>(UnsafeCell<MaybeUninit<T>>); > > impl<T> Opaque<T> { > /// Creates a new opaque value. > pub const fn new(value: T) -> Self { > - Self(MaybeUninit::new(UnsafeCell::new(value))) > + Self(UnsafeCell::new(MaybeUninit::new(value))) > } > > /// Creates an uninitialised value. > pub const fn uninit() -> Self { > - Self(MaybeUninit::uninit()) > + Self(UnsafeCell::new(MaybeUninit::uninit())) > } > > /// Creates a pin-initializer from the given initializer closure. > @@ -258,7 +258,7 @@ pub fn ffi_init(init_func: impl FnOnce(*mut T)) -> impl PinInit<Self> { > > /// Returns a raw pointer to the opaque data. > pub fn get(&self) -> *mut T { > - UnsafeCell::raw_get(self.0.as_ptr()) > + UnsafeCell::get(&self.0).cast::<T>() Is there a reason you don't do `self.0.get().cast::<T>()`?
On 6/14/23 16:27, Benno Lossin wrote:>> @@ -258,7 +258,7 @@ pub fn ffi_init(init_func: impl FnOnce(*mut T)) -> impl PinInit<Self> { >> >> /// Returns a raw pointer to the opaque data. >> pub fn get(&self) -> *mut T { >> - UnsafeCell::raw_get(self.0.as_ptr()) >> + UnsafeCell::get(&self.0).cast::<T>() > > Is there a reason you don't do `self.0.get().cast::<T>()`? > Not really. I just modified what was already there.
On 6/14/23 08:53, Alice Ryhl wrote: > When combining `UnsafeCell` with `MaybeUninit`, it is idiomatic to use > `UnsafeCell` as the outer type. Intuitively, this is because a > `MaybeUninit<T>` might not contain a `T`, but we always want the effect > of the `UnsafeCell`, even if the inner value is uninitialized. > > Now, strictly speaking, this doesn't really make a difference. The > compiler will always apply the `UnsafeCell` effect even if the inner > value is uninitialized. But I think we should follow the convention > here. > > Signed-off-by: Alice Ryhl <aliceryhl@google.com> > --- > [...] Reviewed-by: Martin Rodriguez Reboredo <yakoyoku@gmail.com>
On Wed, 14 Jun 2023 11:53:28 +0000 Alice Ryhl <aliceryhl@google.com> wrote: > When combining `UnsafeCell` with `MaybeUninit`, it is idiomatic to use > `UnsafeCell` as the outer type. Intuitively, this is because a > `MaybeUninit<T>` might not contain a `T`, but we always want the effect > of the `UnsafeCell`, even if the inner value is uninitialized. > > Now, strictly speaking, this doesn't really make a difference. The > compiler will always apply the `UnsafeCell` effect even if the inner > value is uninitialized. But I think we should follow the convention > here. > > Signed-off-by: Alice Ryhl <aliceryhl@google.com> Reviewed-by: Gary Guo <gary@garyguo.net> > --- > rust/kernel/types.rs | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs > index 1e5380b16ed5..fb41635f1e1f 100644 > --- a/rust/kernel/types.rs > +++ b/rust/kernel/types.rs > @@ -224,17 +224,17 @@ fn drop(&mut self) { > /// > /// This is meant to be used with FFI objects that are never interpreted by Rust code. > #[repr(transparent)] > -pub struct Opaque<T>(MaybeUninit<UnsafeCell<T>>); > +pub struct Opaque<T>(UnsafeCell<MaybeUninit<T>>); > > impl<T> Opaque<T> { > /// Creates a new opaque value. > pub const fn new(value: T) -> Self { > - Self(MaybeUninit::new(UnsafeCell::new(value))) > + Self(UnsafeCell::new(MaybeUninit::new(value))) > } > > /// Creates an uninitialised value. > pub const fn uninit() -> Self { > - Self(MaybeUninit::uninit()) > + Self(UnsafeCell::new(MaybeUninit::uninit())) > } > > /// Creates a pin-initializer from the given initializer closure. > @@ -258,7 +258,7 @@ pub fn ffi_init(init_func: impl FnOnce(*mut T)) -> impl PinInit<Self> { > > /// Returns a raw pointer to the opaque data. > pub fn get(&self) -> *mut T { > - UnsafeCell::raw_get(self.0.as_ptr()) > + UnsafeCell::get(&self.0).cast::<T>() > } > > /// Gets the value behind `this`. > @@ -266,7 +266,7 @@ pub fn get(&self) -> *mut T { > /// This function is useful to get access to the value without creating intermediate > /// references. > pub const fn raw_get(this: *const Self) -> *mut T { > - UnsafeCell::raw_get(this.cast::<UnsafeCell<T>>()) > + UnsafeCell::raw_get(this.cast::<UnsafeCell<MaybeUninit<T>>>()).cast::<T>() This can just be `this.cast_mut().cast()` since all types involved are transparent. > } > } > > > base-commit: d2e3115d717197cb2bc020dd1f06b06538474ac3
> > /// Gets the value behind `this`. > > @@ -266,7 +266,7 @@ pub fn get(&self) -> *mut T { > > /// This function is useful to get access to the value without creating intermediate > > /// references. > > pub const fn raw_get(this: *const Self) -> *mut T { > > - UnsafeCell::raw_get(this.cast::<UnsafeCell<T>>()) > > + UnsafeCell::raw_get(this.cast::<UnsafeCell<MaybeUninit<T>>>()).cast::<T>() > > > This can just be `this.cast_mut().cast()` since all types involved are > transparent. I would advise against that, see [1]. It would be bad for people to assume that one is always allowed to do that. I also like it explicit here. [1]: https://github.com/rust-lang/unsafe-code-guidelines/issues/281 -- Cheers, Benno
On Wed, Jun 14, 2023 at 1:53 PM Alice Ryhl <aliceryhl@google.com> wrote: > > When combining `UnsafeCell` with `MaybeUninit`, it is idiomatic to use > `UnsafeCell` as the outer type. Intuitively, this is because a > `MaybeUninit<T>` might not contain a `T`, but we always want the effect > of the `UnsafeCell`, even if the inner value is uninitialized. > > Now, strictly speaking, this doesn't really make a difference. The > compiler will always apply the `UnsafeCell` effect even if the inner > value is uninitialized. But I think we should follow the convention > here. > > Signed-off-by: Alice Ryhl <aliceryhl@google.com> Applied to `rust-next`, thanks everyone! Cheers, Miguel
diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs index 1e5380b16ed5..fb41635f1e1f 100644 --- a/rust/kernel/types.rs +++ b/rust/kernel/types.rs @@ -224,17 +224,17 @@ fn drop(&mut self) { /// /// This is meant to be used with FFI objects that are never interpreted by Rust code. #[repr(transparent)] -pub struct Opaque<T>(MaybeUninit<UnsafeCell<T>>); +pub struct Opaque<T>(UnsafeCell<MaybeUninit<T>>); impl<T> Opaque<T> { /// Creates a new opaque value. pub const fn new(value: T) -> Self { - Self(MaybeUninit::new(UnsafeCell::new(value))) + Self(UnsafeCell::new(MaybeUninit::new(value))) } /// Creates an uninitialised value. pub const fn uninit() -> Self { - Self(MaybeUninit::uninit()) + Self(UnsafeCell::new(MaybeUninit::uninit())) } /// Creates a pin-initializer from the given initializer closure. @@ -258,7 +258,7 @@ pub fn ffi_init(init_func: impl FnOnce(*mut T)) -> impl PinInit<Self> { /// Returns a raw pointer to the opaque data. pub fn get(&self) -> *mut T { - UnsafeCell::raw_get(self.0.as_ptr()) + UnsafeCell::get(&self.0).cast::<T>() } /// Gets the value behind `this`. @@ -266,7 +266,7 @@ pub fn get(&self) -> *mut T { /// This function is useful to get access to the value without creating intermediate /// references. pub const fn raw_get(this: *const Self) -> *mut T { - UnsafeCell::raw_get(this.cast::<UnsafeCell<T>>()) + UnsafeCell::raw_get(this.cast::<UnsafeCell<MaybeUninit<T>>>()).cast::<T>() } }