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
