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

Marcelo Moreira
2025-06-17T02:49:43Z

Em seg., 16 de jun. de 2025 às 04:15, Benno Lossin <lossin@kernel.org> escreveu:
>
> 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.

Ok! =)

I thought about using "so" instead of the first "thus" so I wouldn't
have to repeat the use of "thus" so many times.

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

Thanks! =)
--
Cheers,
Marcelo Moreira