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

["Benno Lossin" <lossin@kernel.org>]

On Mon Jun 16, 2025 at 2:36 AM CEST, Marcelo Moreira wrote:
> Em dom., 15 de jun. de 2025 às 05:38, Miguel Ojeda
> <miguel.ojeda.sandonis@gmail.com> escreveu:
>>
>> On Sun, Jun 15, 2025 at 1:12 AM Marcelo Moreira
>> <marcelomoreira1905@gmail.com> wrote:
>> >
>> > 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...
>>
>> Benno likely meant to use bullet points for each one, rather than a
>> bullet point after a paragraph.
>>
>> If you do e.g.
>>
>>     git grep -A5 '// SAFETY:$' -- rust/kernel
>>
>> you will find examples of that.
>
> Cool! Thanks for the command Miguel =D it helped a lot.
>
> I'm ready to send like this:
>
> // 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`.
       ^ spurious space here,

This argument is actually an extension of the previous one.

> // - `drop_in_place` is valid to call because `drop` is only called once.

This phrasing feels a bit weird to me, but I can't put my finger on what
exactly...

> unsafe { drop_in_place(p.data.get()) };

How about:

// - `self.data` is valid for writes because of `Self`'s type invariants:
//   `&mut Self` guarantees exclusive access, thus no other thread can concurrently access `data`.
// - this function is a drop function, thus this code is at most executed once.

---
Cheers,
Benno