netdev - Re: [PATCH v8 2/7] rust: time: Introduce Delta type

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: <CANiq72=kuZcLCgsSkKa6MrYCJY9UsWSV9VLvj2TcVOQEf0Cnmg@mail.gmail.com>
Date: Sat, 18 Jan 2025 13:19:21 +0100
From: Miguel Ojeda <miguel.ojeda.sandonis@...il.com>
To: FUJITA Tomonori <fujita.tomonori@...il.com>
Cc: linux-kernel@...r.kernel.org, Andrew Lunn <andrew@...n.ch>, 
	rust-for-linux@...r.kernel.org, netdev@...r.kernel.org, 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 2/7] rust: time: Introduce Delta type

Hi Tomonori,

Since you started getting Reviewed-bys, I don't want to delay this
more (pun unintended :), but a couple quick notes...

I can create "good first issues" for these in our tracker if you
prefer, since these should be easy and can be done later (even if, in
general, I think we should require examples and good docs for new
abstractions).

On Thu, Jan 16, 2025 at 5:42 AM FUJITA Tomonori
<fujita.tomonori@...il.com> wrote:
>
> i64 is used instead of bindings::ktime_t because when the ktime_t
> type is used as timestamp, it represents values from 0 to
> KTIME_MAX, which different from Delta.

Typo: "is different ..."?

> Delta::from_[millis|secs] APIs take i64. When a span of time
> overflows, i64::MAX is used.

This behavior should be part of the docs of the methods below.

> +/// A span of time.
> +#[derive(Copy, Clone, PartialEq, PartialOrd, Eq, Ord, Debug)]
> +pub struct Delta {
> +    nanos: i64,
> +}

Some more docs here in `Delta` would be good, e.g. some questions
readers may have could be: What range of values can it hold? Can they
be negative?

Also some module-level docs would be nice relating all the types (you
mention some of that in the commit message for `Instant`, but it would
be nice to put it as docs, rather than in the commit message).

> +    /// Create a new `Delta` from a number of microseconds.
> +    #[inline]
> +    pub const fn from_micros(micros: i64) -> Self {
> +        Self {
> +            nanos: micros.saturating_mul(NSEC_PER_USEC),
> +        }
> +    }

For each of these, I would mention that they saturate and I would
mention the range of input values that would be kept as-is without
loss.

And it would be nice to add some examples, which you can take the
chance to show how it saturates, and they would double as tests, too.

Thanks!

Cheers,
Miguel