linux-kernel - Re: [PATCH RFC] rust: HrTimerMode replacement with bitfield

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <Gv30E9TnyGo2GTh5Z9xnxBXcr3jYGFBmgG2JwkEsCq0X4kHuaPrH_T6cq3iTSzf70fubE7CbJl_OghSPUurylDVQYVzPnoEntilzNv_CSHI=@protonmail.com>
Date: Tue, 28 Jan 2025 21:23:51 +0000
From: Timothy Garwood <gtimothy-dev@...tonmail.com>
To: Miguel Ojeda <miguel.ojeda.sandonis@...il.com>
Cc: Miguel Ojeda <ojeda@...nel.org>, Alex Gaynor <alex.gaynor@...il.com>, Andreas Hindborg <a.hindborg@...nel.org>, Boqun Feng <boqun.feng@...il.com>, Gary Guo <gary@...yguo.net>, Björn Roy Baron <bjorn3_gh@...tonmail.com>, Benno Lossin <benno.lossin@...ton.me>, Alice Ryhl <aliceryhl@...gle.com>, Trevor Gross <tmgross@...ch.edu>, rust-for-linux@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH RFC] rust: HrTimerMode replacement with bitfield_options macro

> One quick question: what happens with the generated docs of the enum
> and each variant etc.? (since that is part of the savings in lines
> here)

Here it is all lost.
This is something that could be much improved: for now, the macro just 
generates a default documentation that equates to this:

+ // A wrapper around a bindings::hrtimer_mode bitfield that sets bitflags using its relative, pinned, and hard methods.
+ pub struct HrTimerMode(/* private fields */);
+
+ // if true, unsets the bindings::hrtimer_mode_HRTIMER_MODE_ABS bit(s) and sets the bindings::hrtimer_mode_HRTIMER_MODE_REL bit(s).
+ // Otherwise, does the opposite.
+ pub fn relative(self, relative: bool) -> Self
+
+ // if true, unsets the 0 bit(s) and sets the bindings::hrtimer_mode_HRTIMER_MODE_PINNED bit(s).
+ // Otherwise, does the opposite.
+ pub fn pinned(self, pinned: bool) -> Self
+
+ // if true, unsets the bindings::hrtimer_mode_HRTIMER_MODE_SOFT bit(s) and sets the bindings::hrtimer_mode_HRTIMER_MODE_HARD bit(s).
+ // Otherwise, does the opposite.
+ pub fn hard(self, hard: bool) -> Self

I would like to find a way to have better documentation. 
Doing something like the 
+ <#optional_attributes>
+ <visibility> <wrapper_name>(<field_type>);
mentioned in [1] could open the door to passing the documentation for the type.
The method's documentation would need a different solution.

> Nits/tips: when you send the next version, consider sending both as a
> series, i.e. this patch being #2 in the series, and #1 the one the
> "real patch" that introduces the macro. That way they stay linked and
> people browsing their emails and/or Lore will see it together :)

Even if this patch depends on something ([2]) the "real patch" does not depend on?

> Also, the notes on the commit that are not part of the commit should
> go after the SoB and the `---` line, before the diffstat.

I am using b4, I think I just signed the wrong commit? I'll make sure to sign the
code commit instead of the cover-letter commit

> There is also some strange text wrapping going on in the message, e.g.
> on the `Link:` lines.

I forgot one [1] reference for the first link. I'll fix it.

Thank you Miguel,
Timothy