Re: [PATCH v4 1/3] rust: revocable: update write invariant and fix safety comments

Marcelo Moreira
2025-06-14T23:12:03Z

Em sáb., 14 de jun. de 2025 às 15:05, Benno Lossin <lossin@kernel.org> escreveu:
>
> On Thu Jun 12, 2025 at 9:22 PM CEST, Marcelo Moreira wrote:
> > Em qui., 12 de jun. de 2025 às 06:02, Benno Lossin <lossin@kernel.org> escreveu:
> >> On Tue Jun 3, 2025 at 1:26 AM CEST, Marcelo Moreira wrote:
> >> > + // and because this `PinnedDrop` context (having `&mut Self`) guarantees exclusive access,
> >> > + // ensuring no other thread can concurrently access or revoke `data`.
> >> > + // This ensures `data` is valid for `drop_in_place`.
> >>
> >> This last sentence is not needed. Instead, we should mention that since
> >> this is a `drop` function, we're only called once, so it's fine to call
> >> drop (since that also is only allowed to be called once).
> >>
> >
> > ok, I thought of something like:
> >
> > // SAFETY: `self.data` is valid for writes because of `Self`'s type invariants.
> > // The `&mut Self` context guarantees exclusive access and a single
> > call to `drop`,
>
> Let's mention that no thread can access `data` *because* of the
> exclusive reference.
>
> > // making `self.data` valid for `drop_in_place`.
>
> The part about drop should be separate. We usually use bullet points for
> that.
>

Hmm, ok.
I think I understand now.

what about:

// SAFETY: `self.data` is valid for writes because of `Self`'s type invariants.
// The `&mut Self` context guarantees exclusive access, meaning no other
// thread can concurrently access `data`.
//
// - `drop_in_place` is valid to call because `drop` is only called once.
unsafe { drop_in_place(p.data.get()) };

I'm not sure if the line break in the comment is okay...