This type is useful for cases where a value might be shared with C code
but not interpreted by it.
In partiquarly this is added to for data that is shared between a Driver
and a MiscDevice implementation.
Similar to Opaque but guarantees that the value is initialized and the
inner value is dropped when Aliased is dropped.
This was origianally proposed for the IRQ abstractions [0], but also
useful for other cases where Data may be aliased, but is always valid
and automatic drop is desired.
Link: https://lore.kernel.org/rust-for-linux/CAH5fLgiOASgjoYKFz6kWwzLaH07DqP2ph+3YyCDh2+gYqGpABA@mail.gmail.com [0]
Suggested-by: Alice Ryhl <aliceryhl@google.com>
Signed-off-by: Christian Schrefl <chrisi.schrefl@gmail.com>
---
rust/kernel/types.rs | 40 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 40 insertions(+)
diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
index 3aea6af9a0bca70ee42b4bad2fe31a99750cbf11..5640128c9a9055476a0040033946ba6caa6e7076 100644
--- a/rust/kernel/types.rs
+++ b/rust/kernel/types.rs
@@ -528,3 +528,43 @@ pub enum Either<L, R> {
/// [`NotThreadSafe`]: type@NotThreadSafe
#[allow(non_upper_case_globals)]
pub const NotThreadSafe: NotThreadSafe = PhantomData;
+
+/// Stores a value that may be aliased.
+///
+/// This is similar to `Opaque<T>` but is guaranteed to contain valid data and will
+/// Call the Drop implementation of T when dropped.
+#[repr(transparent)]
+pub struct Aliased<T> {
+ value: UnsafeCell<T>,
+ _pin: PhantomPinned,
+}
+
+impl<T> Aliased<T> {
+ /// Creates a new `Aliased` value.
+ pub const fn new(value: T) -> Self {
+ Self {
+ value: UnsafeCell::new(value),
+ _pin: PhantomPinned,
+ }
+ }
+ /// Create an `Aliased` pin-initializer from the given pin-initializer.
+ pub fn try_pin_init<E>(value: impl PinInit<T, E>) -> impl PinInit<Self, E> {
+ // SAFETY:
+ // In case of an error in value the error is returned, otherwise the slot is fully initialized,
+ // since value is initialized and _pin is a Zero sized type.
+ // The pin invariants of value are upheld, since no moving occurs.
+ unsafe { init::pin_init_from_closure(move |slot| value.__pinned_init(Self::raw_get(slot))) }
+ }
+ /// Returns a raw pointer to the opaque data.
+ pub const fn get(&self) -> *mut T {
+ UnsafeCell::get(&self.value).cast::<T>()
+ }
+
+ /// Gets the value behind `this`.
+ ///
+ /// 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<MaybeUninit<T>>>()).cast::<T>()
+ }
+}
--
2.48.1Hi Christian,
[Cc Daniel and Danilo]
Thanks for the patch!
On Sun, Jan 19, 2025 at 11:11:13PM +0100, Christian Schrefl wrote:
> This type is useful for cases where a value might be shared with C code
> but not interpreted by it.
> In partiquarly this is added to for data that is shared between a Driver
> and a MiscDevice implementation.
>
> Similar to Opaque but guarantees that the value is initialized and the
> inner value is dropped when Aliased is dropped.
>
> This was origianally proposed for the IRQ abstractions [0], but also
> useful for other cases where Data may be aliased, but is always valid
> and automatic drop is desired.
>
> Link: https://lore.kernel.org/rust-for-linux/CAH5fLgiOASgjoYKFz6kWwzLaH07DqP2ph+3YyCDh2+gYqGpABA@mail.gmail.com [0]
> Suggested-by: Alice Ryhl <aliceryhl@google.com>
> Signed-off-by: Christian Schrefl <chrisi.schrefl@gmail.com>
> ---
> rust/kernel/types.rs | 40 ++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 40 insertions(+)
>
> diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
> index 3aea6af9a0bca70ee42b4bad2fe31a99750cbf11..5640128c9a9055476a0040033946ba6caa6e7076 100644
> --- a/rust/kernel/types.rs
> +++ b/rust/kernel/types.rs
> @@ -528,3 +528,43 @@ pub enum Either<L, R> {
> /// [`NotThreadSafe`]: type@NotThreadSafe
> #[allow(non_upper_case_globals)]
> pub const NotThreadSafe: NotThreadSafe = PhantomData;
> +
> +/// Stores a value that may be aliased.
> +///
> +/// This is similar to `Opaque<T>` but is guaranteed to contain valid data and will
> +/// Call the Drop implementation of T when dropped.
> +#[repr(transparent)]
> +pub struct Aliased<T> {
As I already mentioned [1], the name `Aliased` is more reflecting the
fact that this wrapper will avoid generating the "noalias" attribute(?)
on the reference/pointer to the type rather than an intuitive idea about
"why or when do I need this". Moreover, I think the argument about the
naming of the counterpart in unstable Rust (UnsafePinned) makes sense to
me [2]: this type alone won't prevent `&mut Aliased` getting `swap`, and
it has to be used with `Pin`.
Therefore, I think we should use a different name, perhaps
`(Always)Shared`, or just use `UnsafePinned`, or, as always, looking
fowards to a better name from anybody ;-)
[1]: https://lore.kernel.org/rust-for-linux/Z407egxOy7oNLpq8@boqun-archlinux/
[2]: https://rust-lang.github.io/rfcs/3467-unsafe-pinned.html#naming
Regards,
Boqun
> + value: UnsafeCell<T>,
> + _pin: PhantomPinned,
> +}
> +
> +impl<T> Aliased<T> {
> + /// Creates a new `Aliased` value.
> + pub const fn new(value: T) -> Self {
> + Self {
> + value: UnsafeCell::new(value),
> + _pin: PhantomPinned,
> + }
> + }
> + /// Create an `Aliased` pin-initializer from the given pin-initializer.
> + pub fn try_pin_init<E>(value: impl PinInit<T, E>) -> impl PinInit<Self, E> {
> + // SAFETY:
> + // In case of an error in value the error is returned, otherwise the slot is fully initialized,
> + // since value is initialized and _pin is a Zero sized type.
> + // The pin invariants of value are upheld, since no moving occurs.
> + unsafe { init::pin_init_from_closure(move |slot| value.__pinned_init(Self::raw_get(slot))) }
> + }
> + /// Returns a raw pointer to the opaque data.
> + pub const fn get(&self) -> *mut T {
> + UnsafeCell::get(&self.value).cast::<T>()
> + }
> +
> + /// Gets the value behind `this`.
> + ///
> + /// 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<MaybeUninit<T>>>()).cast::<T>()
> + }
> +}
>
> --
> 2.48.1
>
On 20.01.25 6:24 PM, Boqun Feng wrote:
> Hi Christian,
>
> [Cc Daniel and Danilo]
>
> Thanks for the patch!
>
> On Sun, Jan 19, 2025 at 11:11:13PM +0100, Christian Schrefl wrote:
>> This type is useful for cases where a value might be shared with C code
>> but not interpreted by it.
>> In partiquarly this is added to for data that is shared between a Driver
>> and a MiscDevice implementation.
>>
>> Similar to Opaque but guarantees that the value is initialized and the
>> inner value is dropped when Aliased is dropped.
>>
>> This was origianally proposed for the IRQ abstractions [0], but also
>> useful for other cases where Data may be aliased, but is always valid
>> and automatic drop is desired.
>>
>> Link: https://lore.kernel.org/rust-for-linux/CAH5fLgiOASgjoYKFz6kWwzLaH07DqP2ph+3YyCDh2+gYqGpABA@mail.gmail.com [0]
>> Suggested-by: Alice Ryhl <aliceryhl@google.com>
>> Signed-off-by: Christian Schrefl <chrisi.schrefl@gmail.com>
>> ---
>> rust/kernel/types.rs | 40 ++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 40 insertions(+)
>>
>> diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
>> index 3aea6af9a0bca70ee42b4bad2fe31a99750cbf11..5640128c9a9055476a0040033946ba6caa6e7076 100644
>> --- a/rust/kernel/types.rs
>> +++ b/rust/kernel/types.rs
>> @@ -528,3 +528,43 @@ pub enum Either<L, R> {
>> /// [`NotThreadSafe`]: type@NotThreadSafe
>> #[allow(non_upper_case_globals)]
>> pub const NotThreadSafe: NotThreadSafe = PhantomData;
>> +
>> +/// Stores a value that may be aliased.
>> +///
>> +/// This is similar to `Opaque<T>` but is guaranteed to contain valid data and will
>> +/// Call the Drop implementation of T when dropped.
>> +#[repr(transparent)]
>> +pub struct Aliased<T> {
>
> As I already mentioned [1], the name `Aliased` is more reflecting the
> fact that this wrapper will avoid generating the "noalias" attribute(?)
> on the reference/pointer to the type rather than an intuitive idea about
> "why or when do I need this". Moreover, I think the argument about the
> naming of the counterpart in unstable Rust (UnsafePinned) makes sense to
> me [2]: this type alone won't prevent `&mut Aliased` getting `swap`, and
> it has to be used with `Pin`.
>
> Therefore, I think we should use a different name, perhaps
> `(Always)Shared`, or just use `UnsafePinned`, or, as always, looking
> fowards to a better name from anybody ;-)
>
> [1]: https://lore.kernel.org/rust-for-linux/Z407egxOy7oNLpq8@boqun-archlinux/
> [2]: https://rust-lang.github.io/rfcs/3467-unsafe-pinned.html#naming
I don't particularly care about the name, I mostly used aliased, because that's
the name that Alice originally used.
`(Always)Shared` seems confusing to me.
I guess we can use `UnsafePinned`, but most people won't know what that means,
also I'm not sure if it's a good Idea to use the same name as a (future)
language type.
If there is a consensus on a good name I'll use that for the next version.
>
> Regards,
> Boqun
>
>> + value: UnsafeCell<T>,
>> + _pin: PhantomPinned,
>> +}
>> +
>> +impl<T> Aliased<T> {
>> + /// Creates a new `Aliased` value.
>> + pub const fn new(value: T) -> Self {
>> + Self {
>> + value: UnsafeCell::new(value),
>> + _pin: PhantomPinned,
>> + }
>> + }
>> + /// Create an `Aliased` pin-initializer from the given pin-initializer.
>> + pub fn try_pin_init<E>(value: impl PinInit<T, E>) -> impl PinInit<Self, E> {
>> + // SAFETY:
>> + // In case of an error in value the error is returned, otherwise the slot is fully initialized,
>> + // since value is initialized and _pin is a Zero sized type.
>> + // The pin invariants of value are upheld, since no moving occurs.
>> + unsafe { init::pin_init_from_closure(move |slot| value.__pinned_init(Self::raw_get(slot))) }
>> + }
>> + /// Returns a raw pointer to the opaque data.
>> + pub const fn get(&self) -> *mut T {
>> + UnsafeCell::get(&self.value).cast::<T>()
>> + }
>> +
>> + /// Gets the value behind `this`.
>> + ///
>> + /// 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<MaybeUninit<T>>>()).cast::<T>()
>> + }
>> +}
>>
>> --
>> 2.48.1
>>
On Thu, Jan 23, 2025 at 11:21:23AM +0100, Christian Schrefl wrote:
[...]
> >> +
> >> +/// Stores a value that may be aliased.
> >> +///
> >> +/// This is similar to `Opaque<T>` but is guaranteed to contain valid data and will
> >> +/// Call the Drop implementation of T when dropped.
> >> +#[repr(transparent)]
> >> +pub struct Aliased<T> {
> >
> > As I already mentioned [1], the name `Aliased` is more reflecting the
> > fact that this wrapper will avoid generating the "noalias" attribute(?)
> > on the reference/pointer to the type rather than an intuitive idea about
> > "why or when do I need this". Moreover, I think the argument about the
> > naming of the counterpart in unstable Rust (UnsafePinned) makes sense to
> > me [2]: this type alone won't prevent `&mut Aliased` getting `swap`, and
> > it has to be used with `Pin`.
> >
> > Therefore, I think we should use a different name, perhaps
> > `(Always)Shared`, or just use `UnsafePinned`, or, as always, looking
> > fowards to a better name from anybody ;-)
> >
> > [1]: https://lore.kernel.org/rust-for-linux/Z407egxOy7oNLpq8@boqun-archlinux/
> > [2]: https://rust-lang.github.io/rfcs/3467-unsafe-pinned.html#naming
>
> I don't particularly care about the name, I mostly used aliased, because that's
> the name that Alice originally used.
>
> `(Always)Shared` seems confusing to me.
>
> I guess we can use `UnsafePinned`, but most people won't know what that means,
Hmm.. but doesn't `Aliased` have the same effect, i.e. most people won't
know what that means? Moreover, people who already knows `UnsafePinned`
will still take some time to realize "`Aliased` is actually
`UnsafePinned`".
> also I'm not sure if it's a good Idea to use the same name as a (future)
> language type.
The benefit is that we won't re-invent the wheel since `UnsafePinned`
already does what `Aliased` does here. If we don't have a good name, we
should use the one that most people are already using. Honestly, at this
point, I think we should just use the unstable feature unsafe_pinned.
Regards,
Boqun
>
> If there is a consensus on a good name I'll use that for the next version.
>
> >
> > Regards,
> > Boqun
> >
[...]
Hi Boqun
On 23.01.25 6:56 PM, Boqun Feng wrote:
> On Thu, Jan 23, 2025 at 11:21:23AM +0100, Christian Schrefl wrote:
> [...]
>>>> +
>>>> +/// Stores a value that may be aliased.
>>>> +///
>>>> +/// This is similar to `Opaque<T>` but is guaranteed to contain valid data and will
>>>> +/// Call the Drop implementation of T when dropped.
>>>> +#[repr(transparent)]
>>>> +pub struct Aliased<T> {
>>>
>>> As I already mentioned [1], the name `Aliased` is more reflecting the
>>> fact that this wrapper will avoid generating the "noalias" attribute(?)
>>> on the reference/pointer to the type rather than an intuitive idea about
>>> "why or when do I need this". Moreover, I think the argument about the
>>> naming of the counterpart in unstable Rust (UnsafePinned) makes sense to
>>> me [2]: this type alone won't prevent `&mut Aliased` getting `swap`, and
>>> it has to be used with `Pin`.
>>>
>>> Therefore, I think we should use a different name, perhaps
>>> `(Always)Shared`, or just use `UnsafePinned`, or, as always, looking
>>> fowards to a better name from anybody ;-)
>>>
>>> [1]: https://lore.kernel.org/rust-for-linux/Z407egxOy7oNLpq8@boqun-archlinux/
>>> [2]: https://rust-lang.github.io/rfcs/3467-unsafe-pinned.html#naming
>>
>> I don't particularly care about the name, I mostly used aliased, because that's
>> the name that Alice originally used.
>>
>> `(Always)Shared` seems confusing to me.
>>
>> I guess we can use `UnsafePinned`, but most people won't know what that means,
>
> Hmm.. but doesn't `Aliased` have the same effect, i.e. most people won't
> know what that means? Moreover, people who already knows `UnsafePinned`
> will still take some time to realize "`Aliased` is actually
> `UnsafePinned`
I guess I'll name it `UnsafePinned` then.
>
>> also I'm not sure if it's a good Idea to use the same name as a (future)
>> language type.
>
> The benefit is that we won't re-invent the wheel since `UnsafePinned`
> already does what `Aliased` does here. If we don't have a good name, we
> should use the one that most people are already using. Honestly, at this
> point, I think we should just use the unstable feature unsafe_pinned.
I think that's not implemented in rustc yet.
At least no implementation is linked in the tracking issue:
https://github.com/rust-lang/rust/issues/125735
Once that's implemented we can use a `cfg` to disable this
implementation on new versions that provide it.
(Assuming we use the same API)
> [...]
Cheers
Christian
On Thu, Jan 23, 2025 at 07:04:40PM +0100, Christian Schrefl wrote:
> Hi Boqun
>
> On 23.01.25 6:56 PM, Boqun Feng wrote:
> > On Thu, Jan 23, 2025 at 11:21:23AM +0100, Christian Schrefl wrote:
> > [...]
> >>>> +
> >>>> +/// Stores a value that may be aliased.
> >>>> +///
> >>>> +/// This is similar to `Opaque<T>` but is guaranteed to contain valid data and will
> >>>> +/// Call the Drop implementation of T when dropped.
> >>>> +#[repr(transparent)]
> >>>> +pub struct Aliased<T> {
> >>>
> >>> As I already mentioned [1], the name `Aliased` is more reflecting the
> >>> fact that this wrapper will avoid generating the "noalias" attribute(?)
> >>> on the reference/pointer to the type rather than an intuitive idea about
> >>> "why or when do I need this". Moreover, I think the argument about the
> >>> naming of the counterpart in unstable Rust (UnsafePinned) makes sense to
> >>> me [2]: this type alone won't prevent `&mut Aliased` getting `swap`, and
> >>> it has to be used with `Pin`.
> >>>
> >>> Therefore, I think we should use a different name, perhaps
> >>> `(Always)Shared`, or just use `UnsafePinned`, or, as always, looking
> >>> fowards to a better name from anybody ;-)
> >>>
> >>> [1]: https://lore.kernel.org/rust-for-linux/Z407egxOy7oNLpq8@boqun-archlinux/
> >>> [2]: https://rust-lang.github.io/rfcs/3467-unsafe-pinned.html#naming
> >>
> >> I don't particularly care about the name, I mostly used aliased, because that's
> >> the name that Alice originally used.
> >>
> >> `(Always)Shared` seems confusing to me.
> >>
> >> I guess we can use `UnsafePinned`, but most people won't know what that means,
> >
> > Hmm.. but doesn't `Aliased` have the same effect, i.e. most people won't
> > know what that means? Moreover, people who already knows `UnsafePinned`
> > will still take some time to realize "`Aliased` is actually
> > `UnsafePinned`
>
> I guess I'll name it `UnsafePinned` then.
>
> >
> >> also I'm not sure if it's a good Idea to use the same name as a (future)
> >> language type.
> >
> > The benefit is that we won't re-invent the wheel since `UnsafePinned`
> > already does what `Aliased` does here. If we don't have a good name, we
> > should use the one that most people are already using. Honestly, at this
> > point, I think we should just use the unstable feature unsafe_pinned.
>
> I think that's not implemented in rustc yet.
> At least no implementation is linked in the tracking issue:
> https://github.com/rust-lang/rust/issues/125735
>
> Once that's implemented we can use a `cfg` to disable this
> implementation on new versions that provide it.
> (Assuming we use the same API)
>
Sounds good to me, thanks!
Regards,
Boqun
> > [...]
>
> Cheers
> Christian
On 23.01.25 7:25 PM, Boqun Feng wrote: > On Thu, Jan 23, 2025 at 07:04:40PM +0100, Christian Schrefl wrote: >> Hi Boqun >> >> On 23.01.25 6:56 PM, Boqun Feng wrote: >>> On Thu, Jan 23, 2025 at 11:21:23AM +0100, Christian Schrefl wrote: >>> [...] >>>>> [1]: https://lore.kernel.org/rust-for-linux/Z407egxOy7oNLpq8@boqun-archlinux/ >>>>> [2]: https://rust-lang.github.io/rfcs/3467-unsafe-pinned.html#naming >>>> >>>> I don't particularly care about the name, I mostly used aliased, because that's >>>> the name that Alice originally used. >>>> >>>> `(Always)Shared` seems confusing to me. >>>> >>>> I guess we can use `UnsafePinned`, but most people won't know what that means, >>> >>> Hmm.. but doesn't `Aliased` have the same effect, i.e. most people won't >>> know what that means? Moreover, people who already knows `UnsafePinned` >>> will still take some time to realize "`Aliased` is actually >>> `UnsafePinned` >> >> I guess I'll name it `UnsafePinned` then. >> >>> >>>> also I'm not sure if it's a good Idea to use the same name as a (future) >>>> language type. >>> >>> The benefit is that we won't re-invent the wheel since `UnsafePinned` >>> already does what `Aliased` does here. If we don't have a good name, we >>> should use the one that most people are already using. Honestly, at this >>> point, I think we should just use the unstable feature unsafe_pinned. >> >> I think that's not implemented in rustc yet. >> At least no implementation is linked in the tracking issue: >> https://github.com/rust-lang/rust/issues/125735 >> >> Once that's implemented we can use a `cfg` to disable this >> implementation on new versions that provide it. >> (Assuming we use the same API) >> > > Sounds good to me, thanks! From reading the RFC It seems that `UnsafePinned<T>` should contain a `T` directly instead of `UnsafeCell<T>` so I should probably also change my version it to match. Cheers Christian
On Thu, Jan 23, 2025 at 09:18:33PM +0100, Christian Schrefl wrote: > On 23.01.25 7:25 PM, Boqun Feng wrote: > > On Thu, Jan 23, 2025 at 07:04:40PM +0100, Christian Schrefl wrote: > >> Hi Boqun > >> > >> On 23.01.25 6:56 PM, Boqun Feng wrote: > >>> On Thu, Jan 23, 2025 at 11:21:23AM +0100, Christian Schrefl wrote: > >>> [...] > >>>>> [1]: https://lore.kernel.org/rust-for-linux/Z407egxOy7oNLpq8@boqun-archlinux/ > >>>>> [2]: https://rust-lang.github.io/rfcs/3467-unsafe-pinned.html#naming > >>>> > >>>> I don't particularly care about the name, I mostly used aliased, because that's > >>>> the name that Alice originally used. > >>>> > >>>> `(Always)Shared` seems confusing to me. > >>>> > >>>> I guess we can use `UnsafePinned`, but most people won't know what that means, > >>> > >>> Hmm.. but doesn't `Aliased` have the same effect, i.e. most people won't > >>> know what that means? Moreover, people who already knows `UnsafePinned` > >>> will still take some time to realize "`Aliased` is actually > >>> `UnsafePinned` > >> > >> I guess I'll name it `UnsafePinned` then. > >> > >>> > >>>> also I'm not sure if it's a good Idea to use the same name as a (future) > >>>> language type. > >>> > >>> The benefit is that we won't re-invent the wheel since `UnsafePinned` > >>> already does what `Aliased` does here. If we don't have a good name, we > >>> should use the one that most people are already using. Honestly, at this > >>> point, I think we should just use the unstable feature unsafe_pinned. > >> > >> I think that's not implemented in rustc yet. > >> At least no implementation is linked in the tracking issue: > >> https://github.com/rust-lang/rust/issues/125735 > >> > >> Once that's implemented we can use a `cfg` to disable this > >> implementation on new versions that provide it. > >> (Assuming we use the same API) > >> > > > > Sounds good to me, thanks! > > From reading the RFC It seems that `UnsafePinned<T>` should contain a `T` > directly instead of `UnsafeCell<T>` so I should probably also change my > version it to match. > Per https://github.com/rust-lang/rust/issues/125735#issuecomment-2299183374 , I think you still want to use `UnsafeCell<T>`. Regards, Boqun > Cheers > Christian >
On 23.01.25 9:24 PM, Boqun Feng wrote: > On Thu, Jan 23, 2025 at 09:18:33PM +0100, Christian Schrefl wrote: >> On 23.01.25 7:25 PM, Boqun Feng wrote: >>> On Thu, Jan 23, 2025 at 07:04:40PM +0100, Christian Schrefl wrote: >>>> Hi Boqun >>>> >>>> On 23.01.25 6:56 PM, Boqun Feng wrote: >>>>> On Thu, Jan 23, 2025 at 11:21:23AM +0100, Christian Schrefl wrote: >>>>> [...] >>>>>>> [1]: https://lore.kernel.org/rust-for-linux/Z407egxOy7oNLpq8@boqun-archlinux/ >>>>>>> [2]: https://rust-lang.github.io/rfcs/3467-unsafe-pinned.html#naming >>>>>> >>>>>> I don't particularly care about the name, I mostly used aliased, because that's >>>>>> the name that Alice originally used. >>>>>> >>>>>> `(Always)Shared` seems confusing to me. >>>>>> >>>>>> I guess we can use `UnsafePinned`, but most people won't know what that means, >>>>> >>>>> Hmm.. but doesn't `Aliased` have the same effect, i.e. most people won't >>>>> know what that means? Moreover, people who already knows `UnsafePinned` >>>>> will still take some time to realize "`Aliased` is actually >>>>> `UnsafePinned` >>>> >>>> I guess I'll name it `UnsafePinned` then. >>>> >>>>> >>>>>> also I'm not sure if it's a good Idea to use the same name as a (future) >>>>>> language type. >>>>> >>>>> The benefit is that we won't re-invent the wheel since `UnsafePinned` >>>>> already does what `Aliased` does here. If we don't have a good name, we >>>>> should use the one that most people are already using. Honestly, at this >>>>> point, I think we should just use the unstable feature unsafe_pinned. >>>> >>>> I think that's not implemented in rustc yet. >>>> At least no implementation is linked in the tracking issue: >>>> https://github.com/rust-lang/rust/issues/125735 >>>> >>>> Once that's implemented we can use a `cfg` to disable this >>>> implementation on new versions that provide it. >>>> (Assuming we use the same API) >>>> >>> >>> Sounds good to me, thanks! >> >> From reading the RFC It seems that `UnsafePinned<T>` should contain a `T` >> directly instead of `UnsafeCell<T>` so I should probably also change my >> version it to match. >> > > Per > > https://github.com/rust-lang/rust/issues/125735#issuecomment-2299183374 > > , I think you still want to use `UnsafeCell<T>`. Ah thanks for the quick response, I'll keep the `UnsafeCell<T>` then. > > Regards, > Boqun > >> Cheers >> Christian >>
Hi Christian,
Thanks for the series! A few quick comments on the documentation...
On Sun, Jan 19, 2025 at 11:11 PM Christian Schrefl
<chrisi.schrefl@gmail.com> wrote:
>
> This type is useful for cases where a value might be shared with C code
> but not interpreted by it.
> In partiquarly this is added to for data that is shared between a Driver
> and a MiscDevice implementation.
Typo: partiquarly
Also, normally you would leave an empty line between paragraphs, if
that is what you intended.
> Similar to Opaque but guarantees that the value is initialized and the
> inner value is dropped when Aliased is dropped.
>
> This was origianally proposed for the IRQ abstractions [0], but also
Typo: origianally
I usually recommend using `checkpatch.pl` with `--codespell` (it may
have caught these).
> +/// Stores a value that may be aliased.
> +///
> +/// This is similar to `Opaque<T>` but is guaranteed to contain valid data and will
Would an intra-doc link be possible? Please use then wherever reasonable.
> +/// Call the Drop implementation of T when dropped.
"Call" -> "call"?
Also, please format `Drop` and `T` and add the intra-doc link for the former.
Could we have an example or two for the type? More generally, you have
some nice comments in the commit message, so it is best to put some of
that in the actual documentation, which is what most people will read,
so that they don't need to search for the commit that introduced it.
> + }
> + /// Create an `Aliased` pin-initializer from the given pin-initializer.
Newline between methods/functions/etc.
> + pub fn try_pin_init<E>(value: impl PinInit<T, E>) -> impl PinInit<Self, E> {
> + // SAFETY:
> + // In case of an error in value the error is returned, otherwise the slot is fully initialized,
> + // since value is initialized and _pin is a Zero sized type.
> + // The pin invariants of value are upheld, since no moving occurs.
For lists of requirements, we typically use bullets, e.g. ` - `. I
think this applies to another patch too.
Also, please format comments with Markdown as well (no need for
intra-doc links, of course).
> + }
> + /// Returns a raw pointer to the opaque data.
Missing newline?
Cheers,
Miguel
On 20.01.25 12:04 AM, Miguel Ojeda wrote:
> Hi Christian,
>
> Thanks for the series! A few quick comments on the documentation...
>
> On Sun, Jan 19, 2025 at 11:11 PM Christian Schrefl
> <chrisi.schrefl@gmail.com> wrote:
>>
>> This type is useful for cases where a value might be shared with C code
>> but not interpreted by it.
>> In partiquarly this is added to for data that is shared between a Driver
>> and a MiscDevice implementation.
>
> Typo: partiquarly
Fixed the typos and missing newlines `--codespell` did not catch them.
> Also, normally you would leave an empty line between paragraphs, if
> that is what you intended.
>
>> Similar to Opaque but guarantees that the value is initialized and the
>> inner value is dropped when Aliased is dropped.
>>
>> This was origianally proposed for the IRQ abstractions [0], but also
>
> Typo: origianally
>
> I usually recommend using `checkpatch.pl` with `--codespell` (it may
> have caught these).
>> +/// Stores a value that may be aliased.
>> +///
>> +/// This is similar to `Opaque<T>` but is guaranteed to contain valid data and will
>
> Would an intra-doc link be possible? Please use then wherever reasonable.
Fixed
>
>> +/// Call the Drop implementation of T when dropped.
>
> "Call" -> "call"?
>
> Also, please format `Drop` and `T` and add the intra-doc link for the former.
>
> Could we have an example or two for the type? More generally, you have
> some nice comments in the commit message, so it is best to put some of
> that in the actual documentation, which is what most people will read,
> so that they don't need to search for the commit that introduced it.
I added some more to the documentation and also mentioned it in the
`Opaque` docs.
>
>> + }
>> + /// Create an `Aliased` pin-initializer from the given pin-initializer.
>
> Newline between methods/functions/etc.
>
>> + pub fn try_pin_init<E>(value: impl PinInit<T, E>) -> impl PinInit<Self, E> {
>> + // SAFETY:
>> + // In case of an error in value the error is returned, otherwise the slot is fully initialized,
>> + // since value is initialized and _pin is a Zero sized type.
>> + // The pin invariants of value are upheld, since no moving occurs.
>
> For lists of requirements, we typically use bullets, e.g. ` - `. I
> think this applies to another patch too.
Changed it, also fixed the wrapping.
>
> Also, please format comments with Markdown as well (no need for
> intra-doc links, of course).
>
>> + }
>> + /// Returns a raw pointer to the opaque data.
Changed this to contained data.
>
> Missing newline?
>
> Cheers,
> Miguel
In case you want to take a look the current version of the patch is available at:
https://github.com/onestacked/linux/commits/b4/rust_miscdevice_registrationdata/
Cheers
Christian
© 2016 - 2026 Red Hat, Inc.