Implement the `Unique` type as a prerequisite for `Box` and `Vec`
introduced in subsequent patches.
`Unique` serves as wrapper around a `NonNull`, but indicates that the
possessor of this wrapper owns the referent.
This type already exists in Rust's core library, but, unfortunately, is
exposed as unstable API and hence shouldn't be used in the kernel.
This implementation of `Unique` is almost identical, but mostly stripped
down to the functionality we need for `Box` and `Vec`. Additionally, all
unstable features are removed and / or replaced by stable ones.
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
rust/kernel/types.rs | 183 +++++++++++++++++++++++++++++++++++++++++++
1 file changed, 183 insertions(+)
diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
index bd189d646adb..7cf89067b5fc 100644
--- a/rust/kernel/types.rs
+++ b/rust/kernel/types.rs
@@ -473,3 +473,186 @@ unsafe impl AsBytes for str {}
// does not have any uninitialized portions either.
unsafe impl<T: AsBytes> AsBytes for [T] {}
unsafe impl<T: AsBytes, const N: usize> AsBytes for [T; N] {}
+
+/// A wrapper around a raw non-null `*mut T` that indicates that the possessor
+/// of this wrapper owns the referent. Useful for building abstractions like
+/// `Box<T>`, `Vec<T>`, `String`, and `HashMap<K, V>`.
+///
+/// Unlike `*mut T`, `Unique<T>` behaves "as if" it were an instance of `T`.
+/// It implements `Send`/`Sync` if `T` is `Send`/`Sync`. It also implies
+/// the kind of strong aliasing guarantees an instance of `T` can expect:
+/// the referent of the pointer should not be modified without a unique path to
+/// its owning Unique.
+///
+/// If you're uncertain of whether it's correct to use `Unique` for your purposes,
+/// consider using `NonNull`, which has weaker semantics.
+///
+/// Unlike `*mut T`, the pointer must always be non-null, even if the pointer
+/// is never dereferenced. This is so that enums may use this forbidden value
+/// as a discriminant -- `Option<Unique<T>>` has the same size as `Unique<T>`.
+/// However the pointer may still dangle if it isn't dereferenced.
+///
+/// Unlike `*mut T`, `Unique<T>` is covariant over `T`. This should always be correct
+/// for any type which upholds Unique's aliasing requirements.
+#[repr(transparent)]
+pub struct Unique<T: ?Sized> {
+ pointer: NonNull<T>,
+ // NOTE: this marker has no consequences for variance, but is necessary
+ // for dropck to understand that we logically own a `T`.
+ //
+ // For details, see:
+ // https://github.com/rust-lang/rfcs/blob/master/text/0769-sound-generic-drop.md#phantom-data
+ _marker: PhantomData<T>,
+}
+
+/// `Unique` pointers are `Send` if `T` is `Send` because the data they
+/// reference is unaliased. Note that this aliasing invariant is
+/// unenforced by the type system; the abstraction using the
+/// `Unique` must enforce it.
+unsafe impl<T: Send + ?Sized> Send for Unique<T> {}
+
+/// `Unique` pointers are `Sync` if `T` is `Sync` because the data they
+/// reference is unaliased. Note that this aliasing invariant is
+/// unenforced by the type system; the abstraction using the
+/// `Unique` must enforce it.
+unsafe impl<T: Sync + ?Sized> Sync for Unique<T> {}
+
+impl<T: Sized> Unique<T> {
+ /// Creates a new `Unique` that is dangling, but well-aligned.
+ ///
+ /// This is useful for initializing types which lazily allocate, like
+ /// `Vec::new` does.
+ ///
+ /// Note that the pointer value may potentially represent a valid pointer to
+ /// a `T`, which means this must not be used as a "not yet initialized"
+ /// sentinel value. Types that lazily allocate must track initialization by
+ /// some other means.
+ #[must_use]
+ #[inline]
+ pub const fn dangling() -> Self {
+ Unique {
+ pointer: NonNull::dangling(),
+ _marker: PhantomData,
+ }
+ }
+}
+
+impl<T: ?Sized> Unique<T> {
+ /// Creates a new `Unique`.
+ ///
+ /// # Safety
+ ///
+ /// `ptr` must be non-null.
+ #[inline]
+ pub const unsafe fn new_unchecked(ptr: *mut T) -> Self {
+ // SAFETY: the caller must guarantee that `ptr` is non-null.
+ unsafe {
+ Unique {
+ pointer: NonNull::new_unchecked(ptr),
+ _marker: PhantomData,
+ }
+ }
+ }
+
+ /// Creates a new `Unique` if `ptr` is non-null.
+ #[allow(clippy::manual_map)]
+ #[inline]
+ pub fn new(ptr: *mut T) -> Option<Self> {
+ if let Some(pointer) = NonNull::new(ptr) {
+ Some(Unique {
+ pointer,
+ _marker: PhantomData,
+ })
+ } else {
+ None
+ }
+ }
+
+ /// Acquires the underlying `*mut` pointer.
+ #[must_use = "`self` will be dropped if the result is not used"]
+ #[inline]
+ pub const fn as_ptr(self) -> *mut T {
+ self.pointer.as_ptr()
+ }
+
+ /// Dereferences the content.
+ ///
+ /// The resulting lifetime is bound to self so this behaves "as if"
+ /// it were actually an instance of T that is getting borrowed. If a longer
+ /// (unbound) lifetime is needed, use `&*my_ptr.as_ptr()`.
+ ///
+ /// # Safety
+ ///
+ /// Safety requirements for this function are inherited from [NonNull::as_ref].
+ ///
+ #[must_use]
+ #[inline]
+ pub const unsafe fn as_ref(&self) -> &T {
+ // SAFETY: the caller must guarantee that `self` meets all the
+ // requirements for a reference.
+ unsafe { self.pointer.as_ref() }
+ }
+
+ /// Mutably dereferences the content.
+ ///
+ /// The resulting lifetime is bound to self so this behaves "as if"
+ /// it were actually an instance of T that is getting borrowed. If a longer
+ /// (unbound) lifetime is needed, use `&mut *my_ptr.as_ptr()`.
+ ///
+ /// # Safety
+ ///
+ /// Safety requirements for this function are inherited from [NonNull::as_mut].
+ #[must_use]
+ #[inline]
+ pub unsafe fn as_mut(&mut self) -> &mut T {
+ // SAFETY: the caller must guarantee that `self` meets all the
+ // requirements for a mutable reference.
+ unsafe { self.pointer.as_mut() }
+ }
+
+ /// Casts to a pointer of another type.
+ #[must_use = "`self` will be dropped if the result is not used"]
+ #[inline]
+ pub fn cast<U>(self) -> Unique<U> {
+ Unique::from(self.pointer.cast())
+ }
+
+ /// Acquires the underlying `*mut` pointer.
+ #[must_use = "`self` will be dropped if the result is not used"]
+ #[inline]
+ pub const fn as_non_null(self) -> NonNull<T> {
+ self.pointer
+ }
+}
+
+impl<T: ?Sized> Clone for Unique<T> {
+ #[inline]
+ fn clone(&self) -> Self {
+ *self
+ }
+}
+
+impl<T: ?Sized> Copy for Unique<T> {}
+
+impl<T: ?Sized> From<&mut T> for Unique<T> {
+ /// Converts a `&mut T` to a `Unique<T>`.
+ ///
+ /// This conversion is infallible since references cannot be null.
+ #[inline]
+ fn from(reference: &mut T) -> Self {
+ Self::from(NonNull::from(reference))
+ }
+}
+
+impl<T: ?Sized> From<NonNull<T>> for Unique<T> {
+ /// Converts a `NonNull<T>` to a `Unique<T>`.
+ ///
+ /// This conversion is infallible since `NonNull` cannot be null.
+ #[inline]
+ fn from(pointer: NonNull<T>) -> Self {
+ Unique {
+ pointer,
+ _marker: PhantomData,
+ }
+ }
+}
--
2.45.2On 05.08.24 17:19, Danilo Krummrich wrote:
> Implement the `Unique` type as a prerequisite for `Box` and `Vec`
> introduced in subsequent patches.
>
> `Unique` serves as wrapper around a `NonNull`, but indicates that the
> possessor of this wrapper owns the referent.
>
> This type already exists in Rust's core library, but, unfortunately, is
> exposed as unstable API and hence shouldn't be used in the kernel.
>
> This implementation of `Unique` is almost identical, but mostly stripped
> down to the functionality we need for `Box` and `Vec`. Additionally, all
> unstable features are removed and / or replaced by stable ones.
>
> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> ---
> rust/kernel/types.rs | 183 +++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 183 insertions(+)
>
> diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
> index bd189d646adb..7cf89067b5fc 100644
> --- a/rust/kernel/types.rs
> +++ b/rust/kernel/types.rs
> @@ -473,3 +473,186 @@ unsafe impl AsBytes for str {}
> // does not have any uninitialized portions either.
> unsafe impl<T: AsBytes> AsBytes for [T] {}
> unsafe impl<T: AsBytes, const N: usize> AsBytes for [T; N] {}
> +
> +/// A wrapper around a raw non-null `*mut T` that indicates that the possessor
> +/// of this wrapper owns the referent. Useful for building abstractions like
> +/// `Box<T>`, `Vec<T>`, `String`, and `HashMap<K, V>`.
> +///
> +/// Unlike `*mut T`, `Unique<T>` behaves "as if" it were an instance of `T`.
> +/// It implements `Send`/`Sync` if `T` is `Send`/`Sync`. It also implies
> +/// the kind of strong aliasing guarantees an instance of `T` can expect:
> +/// the referent of the pointer should not be modified without a unique path to
> +/// its owning Unique.
> +///
> +/// If you're uncertain of whether it's correct to use `Unique` for your purposes,
> +/// consider using `NonNull`, which has weaker semantics.
> +///
> +/// Unlike `*mut T`, the pointer must always be non-null, even if the pointer
> +/// is never dereferenced. This is so that enums may use this forbidden value
> +/// as a discriminant -- `Option<Unique<T>>` has the same size as `Unique<T>`.
> +/// However the pointer may still dangle if it isn't dereferenced.
> +///
> +/// Unlike `*mut T`, `Unique<T>` is covariant over `T`. This should always be correct
> +/// for any type which upholds Unique's aliasing requirements.
> +#[repr(transparent)]
> +pub struct Unique<T: ?Sized> {
> + pointer: NonNull<T>,
> + // NOTE: this marker has no consequences for variance, but is necessary
> + // for dropck to understand that we logically own a `T`.
> + //
> + // For details, see:
> + // https://github.com/rust-lang/rfcs/blob/master/text/0769-sound-generic-drop.md#phantom-data
> + _marker: PhantomData<T>,
> +}
> +
> +/// `Unique` pointers are `Send` if `T` is `Send` because the data they
> +/// reference is unaliased. Note that this aliasing invariant is
> +/// unenforced by the type system; the abstraction using the
> +/// `Unique` must enforce it.
> +unsafe impl<T: Send + ?Sized> Send for Unique<T> {}
> +
> +/// `Unique` pointers are `Sync` if `T` is `Sync` because the data they
> +/// reference is unaliased. Note that this aliasing invariant is
> +/// unenforced by the type system; the abstraction using the
> +/// `Unique` must enforce it.
> +unsafe impl<T: Sync + ?Sized> Sync for Unique<T> {}
> +
> +impl<T: Sized> Unique<T> {
> + /// Creates a new `Unique` that is dangling, but well-aligned.
> + ///
> + /// This is useful for initializing types which lazily allocate, like
> + /// `Vec::new` does.
> + ///
> + /// Note that the pointer value may potentially represent a valid pointer to
> + /// a `T`, which means this must not be used as a "not yet initialized"
> + /// sentinel value. Types that lazily allocate must track initialization by
> + /// some other means.
> + #[must_use]
> + #[inline]
> + pub const fn dangling() -> Self {
> + Unique {
> + pointer: NonNull::dangling(),
> + _marker: PhantomData,
> + }
> + }
I think I already asked this, but the code until this point is copied
from the rust stdlib and nowhere cited, does that work with the
licensing?
I also think that the code above could use some improvements:
- add an `# Invariants` section with appropriate invariants (what are
they supposed to be?)
- Do we really want this type to be public and exported from the kernel
crate? I think it would be better if it were crate-private.
- What do we gain from having this type? As I learned recently, the
`Unique` type from `core` doesn't actually put the `noalias` onto
`Box` and `Vec`. The functions are mostly delegations to `NonNull`, so
if the only advantages are that `Send` and `Sync` are already
implemented, then I think we should drop this.
> +}
> +
> +impl<T: ?Sized> Unique<T> {
> + /// Creates a new `Unique`.
> + ///
> + /// # Safety
> + ///
> + /// `ptr` must be non-null.
> + #[inline]
> + pub const unsafe fn new_unchecked(ptr: *mut T) -> Self {
> + // SAFETY: the caller must guarantee that `ptr` is non-null.
> + unsafe {
The only unsafe operation in the body is `new_unchecked` only that one
should be wrapped in `unsafe {}`.
> + Unique {
> + pointer: NonNull::new_unchecked(ptr),
> + _marker: PhantomData,
> + }
> + }
> + }
> +
> + /// Creates a new `Unique` if `ptr` is non-null.
> + #[allow(clippy::manual_map)]
> + #[inline]
> + pub fn new(ptr: *mut T) -> Option<Self> {
> + if let Some(pointer) = NonNull::new(ptr) {
> + Some(Unique {
> + pointer,
> + _marker: PhantomData,
> + })
> + } else {
> + None
> + }
Why is this so verbose? You even needed to disable the clippy lint!
Can't this just be?:
Some(Unique {
pointer: NonNull::new(ptr)?,
_marker: PhantomData,
})
or maybe even
NonNull::new(ptr).map(Unique::from)
> + }
> +
> + /// Acquires the underlying `*mut` pointer.
> + #[must_use = "`self` will be dropped if the result is not used"]
This seems like an odd thing, there is no `Drop` impl that drops the
pointee...
---
Cheers,
Benno
On Tue, Aug 06, 2024 at 05:22:21PM +0000, Benno Lossin wrote:
> On 05.08.24 17:19, Danilo Krummrich wrote:
> > Implement the `Unique` type as a prerequisite for `Box` and `Vec`
> > introduced in subsequent patches.
> >
> > `Unique` serves as wrapper around a `NonNull`, but indicates that the
> > possessor of this wrapper owns the referent.
> >
> > This type already exists in Rust's core library, but, unfortunately, is
> > exposed as unstable API and hence shouldn't be used in the kernel.
> >
> > This implementation of `Unique` is almost identical, but mostly stripped
> > down to the functionality we need for `Box` and `Vec`. Additionally, all
> > unstable features are removed and / or replaced by stable ones.
> >
> > Reviewed-by: Alice Ryhl <aliceryhl@google.com>
> > Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> > ---
> > rust/kernel/types.rs | 183 +++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 183 insertions(+)
> >
> > diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
> > index bd189d646adb..7cf89067b5fc 100644
> > --- a/rust/kernel/types.rs
> > +++ b/rust/kernel/types.rs
> > @@ -473,3 +473,186 @@ unsafe impl AsBytes for str {}
> > // does not have any uninitialized portions either.
> > unsafe impl<T: AsBytes> AsBytes for [T] {}
> > unsafe impl<T: AsBytes, const N: usize> AsBytes for [T; N] {}
> > +
> > +/// A wrapper around a raw non-null `*mut T` that indicates that the possessor
> > +/// of this wrapper owns the referent. Useful for building abstractions like
> > +/// `Box<T>`, `Vec<T>`, `String`, and `HashMap<K, V>`.
> > +///
> > +/// Unlike `*mut T`, `Unique<T>` behaves "as if" it were an instance of `T`.
> > +/// It implements `Send`/`Sync` if `T` is `Send`/`Sync`. It also implies
> > +/// the kind of strong aliasing guarantees an instance of `T` can expect:
> > +/// the referent of the pointer should not be modified without a unique path to
> > +/// its owning Unique.
> > +///
> > +/// If you're uncertain of whether it's correct to use `Unique` for your purposes,
> > +/// consider using `NonNull`, which has weaker semantics.
> > +///
> > +/// Unlike `*mut T`, the pointer must always be non-null, even if the pointer
> > +/// is never dereferenced. This is so that enums may use this forbidden value
> > +/// as a discriminant -- `Option<Unique<T>>` has the same size as `Unique<T>`.
> > +/// However the pointer may still dangle if it isn't dereferenced.
> > +///
> > +/// Unlike `*mut T`, `Unique<T>` is covariant over `T`. This should always be correct
> > +/// for any type which upholds Unique's aliasing requirements.
> > +#[repr(transparent)]
> > +pub struct Unique<T: ?Sized> {
> > + pointer: NonNull<T>,
> > + // NOTE: this marker has no consequences for variance, but is necessary
> > + // for dropck to understand that we logically own a `T`.
> > + //
> > + // For details, see:
> > + // https://github.com/rust-lang/rfcs/blob/master/text/0769-sound-generic-drop.md#phantom-data
> > + _marker: PhantomData<T>,
> > +}
> > +
> > +/// `Unique` pointers are `Send` if `T` is `Send` because the data they
> > +/// reference is unaliased. Note that this aliasing invariant is
> > +/// unenforced by the type system; the abstraction using the
> > +/// `Unique` must enforce it.
> > +unsafe impl<T: Send + ?Sized> Send for Unique<T> {}
> > +
> > +/// `Unique` pointers are `Sync` if `T` is `Sync` because the data they
> > +/// reference is unaliased. Note that this aliasing invariant is
> > +/// unenforced by the type system; the abstraction using the
> > +/// `Unique` must enforce it.
> > +unsafe impl<T: Sync + ?Sized> Sync for Unique<T> {}
> > +
> > +impl<T: Sized> Unique<T> {
> > + /// Creates a new `Unique` that is dangling, but well-aligned.
> > + ///
> > + /// This is useful for initializing types which lazily allocate, like
> > + /// `Vec::new` does.
> > + ///
> > + /// Note that the pointer value may potentially represent a valid pointer to
> > + /// a `T`, which means this must not be used as a "not yet initialized"
> > + /// sentinel value. Types that lazily allocate must track initialization by
> > + /// some other means.
> > + #[must_use]
> > + #[inline]
> > + pub const fn dangling() -> Self {
> > + Unique {
> > + pointer: NonNull::dangling(),
> > + _marker: PhantomData,
> > + }
> > + }
>
> I think I already asked this, but the code until this point is copied
> from the rust stdlib and nowhere cited, does that work with the
> licensing?
>
> I also think that the code above could use some improvements:
> - add an `# Invariants` section with appropriate invariants (what are
> they supposed to be?)
> - Do we really want this type to be public and exported from the kernel
> crate? I think it would be better if it were crate-private.
> - What do we gain from having this type? As I learned recently, the
> `Unique` type from `core` doesn't actually put the `noalias` onto
> `Box` and `Vec`. The functions are mostly delegations to `NonNull`, so
> if the only advantages are that `Send` and `Sync` are already
> implemented, then I think we should drop this.
I originally introduced it for the reasons described in [1], but mainly to make
clear that the owner of this thing also owns the memory behind the pointer and
the `Send` and `Sync` stuff you already mentioned.
If no one else has objections we can also just drop it. Personally, I'm fine
either way.
[1] https://docs.rs/rust-libcore/latest/core/ptr/struct.Unique.html
>
> > +}
> > +
> > +impl<T: ?Sized> Unique<T> {
> > + /// Creates a new `Unique`.
> > + ///
> > + /// # Safety
> > + ///
> > + /// `ptr` must be non-null.
> > + #[inline]
> > + pub const unsafe fn new_unchecked(ptr: *mut T) -> Self {
> > + // SAFETY: the caller must guarantee that `ptr` is non-null.
> > + unsafe {
>
> The only unsafe operation in the body is `new_unchecked` only that one
> should be wrapped in `unsafe {}`.
>
> > + Unique {
> > + pointer: NonNull::new_unchecked(ptr),
> > + _marker: PhantomData,
> > + }
> > + }
> > + }
> > +
> > + /// Creates a new `Unique` if `ptr` is non-null.
> > + #[allow(clippy::manual_map)]
> > + #[inline]
> > + pub fn new(ptr: *mut T) -> Option<Self> {
> > + if let Some(pointer) = NonNull::new(ptr) {
> > + Some(Unique {
> > + pointer,
> > + _marker: PhantomData,
> > + })
> > + } else {
> > + None
> > + }
>
> Why is this so verbose? You even needed to disable the clippy lint!
> Can't this just be?:
>
> Some(Unique {
> pointer: NonNull::new(ptr)?,
> _marker: PhantomData,
> })
>
> or maybe even
>
> NonNull::new(ptr).map(Unique::from)
>
>
> > + }
> > +
> > + /// Acquires the underlying `*mut` pointer.
> > + #[must_use = "`self` will be dropped if the result is not used"]
>
> This seems like an odd thing, there is no `Drop` impl that drops the
> pointee...
>
> ---
> Cheers,
> Benno
>
On 07.08.24 01:12, Danilo Krummrich wrote:
> On Tue, Aug 06, 2024 at 05:22:21PM +0000, Benno Lossin wrote:
>> On 05.08.24 17:19, Danilo Krummrich wrote:
>>> +impl<T: Sized> Unique<T> {
>>> + /// Creates a new `Unique` that is dangling, but well-aligned.
>>> + ///
>>> + /// This is useful for initializing types which lazily allocate, like
>>> + /// `Vec::new` does.
>>> + ///
>>> + /// Note that the pointer value may potentially represent a valid pointer to
>>> + /// a `T`, which means this must not be used as a "not yet initialized"
>>> + /// sentinel value. Types that lazily allocate must track initialization by
>>> + /// some other means.
>>> + #[must_use]
>>> + #[inline]
>>> + pub const fn dangling() -> Self {
>>> + Unique {
>>> + pointer: NonNull::dangling(),
>>> + _marker: PhantomData,
>>> + }
>>> + }
>>
>> I think I already asked this, but the code until this point is copied
>> from the rust stdlib and nowhere cited, does that work with the
>> licensing?
>>
>> I also think that the code above could use some improvements:
>> - add an `# Invariants` section with appropriate invariants (what are
>> they supposed to be?)
>> - Do we really want this type to be public and exported from the kernel
>> crate? I think it would be better if it were crate-private.
>> - What do we gain from having this type? As I learned recently, the
>> `Unique` type from `core` doesn't actually put the `noalias` onto
>> `Box` and `Vec`. The functions are mostly delegations to `NonNull`, so
>> if the only advantages are that `Send` and `Sync` are already
>> implemented, then I think we should drop this.
>
> I originally introduced it for the reasons described in [1], but mainly to make
> clear that the owner of this thing also owns the memory behind the pointer and
> the `Send` and `Sync` stuff you already mentioned.
I would prefer if we make that explicit, since it is rather error-prone
when creating new pointer types (and one should have to think about
thread safety).
---
Cheers,
Benno
> If no one else has objections we can also just drop it. Personally, I'm fine
> either way.
>
> [1] https://docs.rs/rust-libcore/latest/core/ptr/struct.Unique.html
On Wed, Aug 07, 2024 at 07:27:43AM +0000, Benno Lossin wrote:
> On 07.08.24 01:12, Danilo Krummrich wrote:
> > On Tue, Aug 06, 2024 at 05:22:21PM +0000, Benno Lossin wrote:
> >> On 05.08.24 17:19, Danilo Krummrich wrote:
> >>> +impl<T: Sized> Unique<T> {
> >>> + /// Creates a new `Unique` that is dangling, but well-aligned.
> >>> + ///
> >>> + /// This is useful for initializing types which lazily allocate, like
> >>> + /// `Vec::new` does.
> >>> + ///
> >>> + /// Note that the pointer value may potentially represent a valid pointer to
> >>> + /// a `T`, which means this must not be used as a "not yet initialized"
> >>> + /// sentinel value. Types that lazily allocate must track initialization by
> >>> + /// some other means.
> >>> + #[must_use]
> >>> + #[inline]
> >>> + pub const fn dangling() -> Self {
> >>> + Unique {
> >>> + pointer: NonNull::dangling(),
> >>> + _marker: PhantomData,
> >>> + }
> >>> + }
> >>
> >> I think I already asked this, but the code until this point is copied
> >> from the rust stdlib and nowhere cited, does that work with the
> >> licensing?
> >>
> >> I also think that the code above could use some improvements:
> >> - add an `# Invariants` section with appropriate invariants (what are
> >> they supposed to be?)
> >> - Do we really want this type to be public and exported from the kernel
> >> crate? I think it would be better if it were crate-private.
> >> - What do we gain from having this type? As I learned recently, the
> >> `Unique` type from `core` doesn't actually put the `noalias` onto
> >> `Box` and `Vec`. The functions are mostly delegations to `NonNull`, so
> >> if the only advantages are that `Send` and `Sync` are already
> >> implemented, then I think we should drop this.
> >
> > I originally introduced it for the reasons described in [1], but mainly to make
> > clear that the owner of this thing also owns the memory behind the pointer and
> > the `Send` and `Sync` stuff you already mentioned.
>
> I would prefer if we make that explicit, since it is rather error-prone
> when creating new pointer types (and one should have to think about
> thread safety).
Again, fine for me. If no one else has objections I'll just drop `Unique`.
>
> ---
> Cheers,
> Benno
>
> > If no one else has objections we can also just drop it. Personally, I'm fine
> > either way.
> >
> > [1] https://docs.rs/rust-libcore/latest/core/ptr/struct.Unique.html
>
On Tue, Aug 6, 2024 at 7:22 PM Benno Lossin <benno.lossin@proton.me> wrote: > > I think I already asked this, but the code until this point is copied > from the rust stdlib and nowhere cited, does that work with the > licensing? No, it doesn't. They should be put into its own file with the right license and a paragraph about it, e.g. the other `std_vendor.rs` files we have (if it is just about `Unique`, it could go in the existing one at the `kernel` level). Cheers, Miguel
On Tue, Aug 06, 2024 at 07:28:45PM +0200, Miguel Ojeda wrote: > On Tue, Aug 6, 2024 at 7:22 PM Benno Lossin <benno.lossin@proton.me> wrote: > > > > I think I already asked this, but the code until this point is copied > > from the rust stdlib and nowhere cited, does that work with the > > licensing? > > No, it doesn't. They should be put into its own file with the right > license and a paragraph about it, e.g. the other `std_vendor.rs` files > we have (if it is just about `Unique`, it could go in the existing one > at the `kernel` level). Thanks for catching this, I once thought about it and then forgot about it entirely. I didn't know about std_vendor.rs, that's pretty convenient. I will move it over in case we don't drop `Unique` entirely. > > Cheers, > Miguel >
© 2016 - 2025 Red Hat, Inc.