rust-for-linux.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Andreas Hindborg <a.hindborg@kernel.org>
To: "Alice Ryhl" <aliceryhl@google.com>
Cc: "Tamir Duberstein" <tamird@gmail.com>,
	"Gary Guo" <gary@garyguo.net>,
	"Danilo Krummrich" <dakr@kernel.org>,
	"Miguel Ojeda" <ojeda@kernel.org>,
	"Alex Gaynor" <alex.gaynor@gmail.com>,
	"Boqun Feng" <boqun.feng@gmail.com>,
	"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
	"Benno Lossin" <benno.lossin@proton.me>,
	"Trevor Gross" <tmgross@umich.edu>,
	"Matthew Wilcox" <willy@infradead.org>,
	"Bjorn Helgaas" <bhelgaas@google.com>,
	"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	"Rafael J. Wysocki" <rafael@kernel.org>,
	"FUJITA Tomonori" <fujita.tomonori@gmail.com>,
	"Rob Herring (Arm)" <robh@kernel.org>,
	"Maíra Canal" <mcanal@igalia.com>,
	"Asahi Lina" <lina@asahilina.net>,
	rust-for-linux@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-pci@vger.kernel.org
Subject: Re: [PATCH v19 1/3] rust: types: add `ForeignOwnable::PointedTo`
Date: Thu, 01 May 2025 10:07:49 +0200	[thread overview]
Message-ID: <87y0vg21pm.fsf@kernel.org> (raw)
In-Reply-To: <CAH5fLghKLZR7i6YQk8cQrvfOr11xEKia5LHtj1fn8dD3Stv0dQ@mail.gmail.com> (Alice Ryhl's message of "Thu, 01 May 2025 09:17:27 +0200")

"Alice Ryhl" <aliceryhl@google.com> writes:

> On Wed, Apr 30, 2025 at 8:57 PM Tamir Duberstein <tamird@gmail.com> wrote:
>>
>> On Wed, Apr 30, 2025 at 11:31 AM Gary Guo <gary@garyguo.net> wrote:
>> >
>> > On Wed, 23 Apr 2025 09:54:37 -0400
>> > Tamir Duberstein <tamird@gmail.com> wrote:
>> > > -impl<T: 'static, A> ForeignOwnable for Box<T, A>
>> > > +// SAFETY: The `into_foreign` function returns a pointer that is well-aligned.
>> > > +unsafe impl<T: 'static, A> ForeignOwnable for Box<T, A>
>> > >  where
>> > >      A: Allocator,
>> > >  {
>> > > +    type PointedTo = T;
>> >
>> > I don't think this is the correct solution for this. The returned
>> > pointer is supposed to opaque, and exposing this type may encourage
>> > this is to be wrongly used.
>>
>> Can you give an example?
>
> This came up when we discussed this patch in the meeting yesterday:
> https://lore.kernel.org/all/20250227-configfs-v5-1-c40e8dc3b9cd@kernel.org/
>
> This is incorrect use of the trait. The pointer is supposed to be
> opaque, and you can't dereference it. See my reply to that patch as
> well:
> https://lore.kernel.org/all/CAH5fLggDwPBzMO2Z48oMjDm4qgoNM0NQs_63TxmVEGy+gtMpOA@mail.gmail.com/


For reference, the outcome of the discussion yesterday:

 - The use of `ForeignOwnable` in the configfs series is not correct. The pointer
   must be opaque. I will drop the use of `ForeignOwnable` and adapt
   `Arc` methods `into_raw`/`from_raw` instead. I had a plan to make the
   code generic over the pointer type with a bound on `ForeignOwnable`.
   A new trait is required for that now.

 - There may be a use case for a trait that allows passing ownership of
   an object to C, similar to `ForeignOwnable` but with a non-opaque
   pointer. Trait methods would be `into_raw`, `from_raw`, `borrow`.

 - The solution for alignment adopted in this (xarray) series is not
   ideal. However, given the timeline we will proceed merging the series
   as is, and then change the solution to the one outlined by Gary in
   the next cycle.

@Gary you mentioned an implementation of the solution you outlined is
already posted to the list. I can't seem to find it, can you point to
it?

Best regards,
Andreas Hindborg



  reply	other threads:[~2025-05-01  8:08 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-04-23 13:54 [PATCH v19 0/3] rust: xarray: Add a minimal abstraction for XArray Tamir Duberstein
2025-04-23 13:54 ` [PATCH v19 1/3] rust: types: add `ForeignOwnable::PointedTo` Tamir Duberstein
2025-04-30 18:31   ` Gary Guo
2025-04-30 18:57     ` Tamir Duberstein
2025-05-01  7:17       ` Alice Ryhl
2025-05-01  8:07         ` Andreas Hindborg [this message]
2025-04-23 13:54 ` [PATCH v19 2/3] rust: xarray: Add an abstraction for XArray Tamir Duberstein
2025-04-23 13:54 ` [PATCH v19 3/3] MAINTAINERS: add entry for Rust XArray API Tamir Duberstein
2025-04-26 19:48 ` [PATCH v19 0/3] rust: xarray: Add a minimal abstraction for XArray Andreas Hindborg
2025-04-27 14:39   ` Danilo Krummrich
2025-04-27 15:57     ` Andreas Hindborg
2025-04-28  7:14       ` Andreas Hindborg

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=87y0vg21pm.fsf@kernel.org \
    --to=a.hindborg@kernel.org \
    --cc=alex.gaynor@gmail.com \
    --cc=aliceryhl@google.com \
    --cc=benno.lossin@proton.me \
    --cc=bhelgaas@google.com \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun.feng@gmail.com \
    --cc=dakr@kernel.org \
    --cc=fujita.tomonori@gmail.com \
    --cc=gary@garyguo.net \
    --cc=gregkh@linuxfoundation.org \
    --cc=lina@asahilina.net \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=mcanal@igalia.com \
    --cc=ojeda@kernel.org \
    --cc=rafael@kernel.org \
    --cc=robh@kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=tamird@gmail.com \
    --cc=tmgross@umich.edu \
    --cc=willy@infradead.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).