Re: [PATCH v8 2/7] rust: time: Introduce Delta type
From: FUJITA Tomonori <fujita.tomonori@gmail.com>
Date: 2025-01-22 07:37:38
Also in:
lkml, rust-for-linux
On Sat, 18 Jan 2025 13:19:21 +0100 Miguel Ojeda [off-list ref] wrote:
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).
Yes, please create such. I'll add more docs but I'm sure that there will be room for improvement.
On Thu, Jan 16, 2025 at 5:42 AM FUJITA Tomonori [off-list ref] wrote:quoted
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 ..."?
Oops, will fix.
quoted
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.
You want to add the above explanation to all the Delta::from_[millis|micro|secs], right?
quoted
+/// 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?
Okay, I'll add.
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).
Is there any existing source code I can refer to? I'm not sure how "module-level docs" should be written.
quoted
+ /// 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.
I'll try to improve.