linux-kernel - Re: [PATCH] rust: macros: improve `#[vtable]` documentation

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <64542ca3-180a-483d-9e86-bf03f0533254@proton.me>
Date:   Thu, 12 Oct 2023 13:51:39 +0000
From:   Benno Lossin <benno.lossin@...ton.me>
To:     Wedson Almeida Filho <wedsonaf@...il.com>
Cc:     Miguel Ojeda <ojeda@...nel.org>,
        Alex Gaynor <alex.gaynor@...il.com>,
        Boqun Feng <boqun.feng@...il.com>, Gary Guo <gary@...yguo.net>,
        Björn Roy Baron <bjorn3_gh@...tonmail.com>,
        Alice Ryhl <aliceryhl@...gle.com>,
        Andreas Hindborg <nmi@...aspace.dk>,
        rust-for-linux@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] rust: macros: improve `#[vtable]` documentation

On 12.10.23 15:48, Wedson Almeida Filho wrote:
> On Thu, 12 Oct 2023 at 10:22, Benno Lossin <benno.lossin@...ton.me> wrote:
>> +
>> +/// Error message for calling a default function of a [`#[vtable]`](macros::vtable) trait.
>> +pub const VTABLE_DEFAULT_ERROR: &str =
>> +    "This function must not be called, see the #[vtable] documentation.";
>> diff --git a/rust/macros/lib.rs b/rust/macros/lib.rs
>> index c42105c2ff96..dab9a1080b82 100644
>> --- a/rust/macros/lib.rs
>> +++ b/rust/macros/lib.rs
>> @@ -87,27 +87,41 @@ pub fn module(ts: TokenStream) -> TokenStream {
>>   /// implementation could just return `Error::EINVAL`); Linux typically use C
>>   /// `NULL` pointers to represent these functions.
>>   ///
>> -/// This attribute is intended to close the gap. Traits can be declared and
>> -/// implemented with the `#[vtable]` attribute, and a `HAS_*` associated constant
>> -/// will be generated for each method in the trait, indicating if the implementor
>> -/// has overridden a method.
>> +/// This attribute closes that gap. A trait can be annotated with the `#[vtable]` attribute.
>> +/// Implementers of the trait will then also have to annotate the trait with `#[vtable]`. This
>> +/// attribute generates a `HAS_*` associated constant bool for each method in the trait that is set
>> +/// to true if the implementer has overridden the associated method.
>> +///
>> +/// If you want to make a function optional, you must provide a default implementation. But this
> 
> We should  standardise how we write our documentation. In the `rust`
> branch, we avoided using the imperative mood like you have here; the
> rationale was that the documentation was describing how things
> are/work, so we shouldn't be giving orders to readers (they may be
> authors of traits, implementers of some vtable trait, or neither, just
> someone learning about things, etc.).
> 
> In the paragraph above, you'll find an example: "Implementers of the
> trait will then also have to...".
> 
> For the specific case above, I'd suggest: 'For a function to be
> optional, it must have a default implementation.', or using the
> passive voice, 'To make a function optional, a default implementation
> must be provided'.

I agree, I also think we should just fix this now, so I will send a v2.

-- 
Cheers,
Benno