Messages in this thread | ![/](/images/icornerl.gif) | | Date | Mon, 11 Dec 2023 15:34:35 +0000 | Subject | Re: [PATCH v2 4/7] rust: file: add `FileDescriptorReservation` | From | Alice Ryhl <> |
| |
Benno Lossin <benno.lossin@proton.me> writes: > On 12/6/23 12:59, Alice Ryhl wrote: > > + /// Commits the reservation. > > + /// > > + /// The previously reserved file descriptor is bound to `file`. This method consumes the > > + /// [`FileDescriptorReservation`], so it will not be usable after this call. > > + pub fn fd_install(self, file: ARef<File>) { > > + // SAFETY: `self.fd` was previously returned by `get_unused_fd_flags`, and `file.ptr` is > > + // guaranteed to have an owned ref count by its type invariants. > > There is no mention of the requirement that `current` has not changed > (you do mention it on the `_not_send` field, but I think it should also > be in the safety comment here). > > [...] > > +impl Drop for FileDescriptorReservation { > > + fn drop(&mut self) { > > + // SAFETY: `self.fd` was returned by a previous call to `get_unused_fd_flags`. > > Ditto.
I'll update this.
> > +/// Zero-sized type to mark types not [`Send`]. > > +/// > > +/// Add this type as a field to your struct if your type should not be sent to a different task. > > +/// Since [`Send`] is an auto trait, adding a single field that is `!Send` will ensure that the > > +/// whole type is `!Send`. > > +/// > > +/// If a type is `!Send` it is impossible to give control over an instance of the type to another > > +/// task. This is useful when a type stores task-local information for example file descriptors. > > +pub type NotThreadSafe = PhantomData<*mut ()>; > > This should be in its own commit. > > Then you can also change the usages of `PhantomData<*mut ()>` in > `Guard` and `TaskRef`. > > It would be nice to use `NotThreadSafe` as the value instead of > `PhantomData`, since it is a bit confusing... > I think we might be able to also have a constant with the same name > that is just `pub const NotThreadSafe: NotThreadSafe = PhantomData;`.
I was able to get this to work with a `const`, so I will use that.
Alice
| ![\](/images/icornerr.gif) |