netdev - Re: [PATCH v8 4/7] rust: time: Add wrapper for fsleep function

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: <CANiq72=xwhu21YJ+HEXhF1Uk_t1tuffphRgF4wAGiTc-JYcJVQ@mail.gmail.com>
Date: Wed, 22 Jan 2025 11:21:51 +0100
From: Miguel Ojeda <miguel.ojeda.sandonis@...il.com>
To: FUJITA Tomonori <fujita.tomonori@...il.com>
Cc: linux-kernel@...r.kernel.org, rust-for-linux@...r.kernel.org, 
	netdev@...r.kernel.org, andrew@...n.ch, hkallweit1@...il.com, 
	tmgross@...ch.edu, ojeda@...nel.org, alex.gaynor@...il.com, gary@...yguo.net, 
	bjorn3_gh@...tonmail.com, benno.lossin@...ton.me, a.hindborg@...sung.com, 
	aliceryhl@...gle.com, anna-maria@...utronix.de, frederic@...nel.org, 
	tglx@...utronix.de, arnd@...db.de, jstultz@...gle.com, sboyd@...nel.org, 
	mingo@...hat.com, peterz@...radead.org, juri.lelli@...hat.com, 
	vincent.guittot@...aro.org, dietmar.eggemann@....com, rostedt@...dmis.org, 
	bsegall@...gle.com, mgorman@...e.de, vschneid@...hat.com
Subject: Re: [PATCH v8 4/7] rust: time: Add wrapper for fsleep function

On Wed, Jan 22, 2025 at 7:57 AM FUJITA Tomonori
<fujita.tomonori@...il.com> wrote:
>
> Agreed that "the C side" is better and updated the comment. I copied
> that expression from the existing code; there are many "kernel's" in
> rust/kernel/. "good first issues" for them?

Yeah, will do.

> You prefer "[`fsleep()`]" rather than "[`fsleep`]"? I can't find any
> precedent for the C side functions.

There is no preference yet. It would be nice to be consistent, though.
The option of removing the `()` in all cases may be easier to check
for than the other, though the `()` give a bit of (possibly redundant)
information to the reader.

> Yeah, simpler is better. After applying the above changes, it ended up
> as follows.

Looks good, thanks!

Not sure if we should say "Equivalent" given it is not exactly the
same, but I am not a native speaker: I think it does not necessarily
need to be exactly the same to be "equivalent", but perhaps "Similar
to" or "Counterpart of" or something like that is better.

> Ah, it might work. The following doesn't work. Seems that we need to
> add another const like MAX_DELTA_NANOS or something. No strong
> preference but I feel the current is simpler.
>
> let delta = match delta.as_nanos() {
>     0..=MAX_DELTA.as_nanos() as i32 => delta,
>     _ => MAX_DELTA,
> };

Yeah, don't worry about it too much :)

[ The language may get `const { ... }` to work there (it does in
nightly) though it wouldn't look good either. I think the `as i32`
would not be needed. ]

By the way, speaking of something related, do we want to make some of
the methods `fn`s be `const`?

Cheers,
Miguel